This is an automated email from the ASF dual-hosted git repository.

rubenql pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/main by this push:
     new 8659140dcb [CALCITE-6507] Random functions are incorrectly considered 
deterministic
8659140dcb is described below

commit 8659140dcb689f71fecbba6b081cc525c7fc2774
Author: Ruben Quesada Lopez <[email protected]>
AuthorDate: Wed Jul 31 09:36:07 2024 +0100

    [CALCITE-6507] Random functions are incorrectly considered deterministic
---
 .../calcite/sql/fun/SqlRandIntegerFunction.java    |  4 +++
 .../calcite/sql/fun/SqlStdOperatorTable.java       |  1 +
 .../apache/calcite/test/RelMdPredicatesTest.java   | 27 +++++++++++++++
 .../org/apache/calcite/test/RelOptRulesTest.java   | 33 ++++++++++++++++++
 .../org/apache/calcite/test/RelOptRulesTest.xml    | 39 ++++++++++++++++++++++
 5 files changed, 104 insertions(+)

diff --git 
a/core/src/main/java/org/apache/calcite/sql/fun/SqlRandIntegerFunction.java 
b/core/src/main/java/org/apache/calcite/sql/fun/SqlRandIntegerFunction.java
index adf4d40d04..4bd3a3ed4c 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlRandIntegerFunction.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlRandIntegerFunction.java
@@ -45,6 +45,10 @@ public class SqlRandIntegerFunction extends SqlFunction {
 
   //~ Methods ----------------------------------------------------------------
 
+  @Override public boolean isDeterministic() {
+    return false;
+  }
+
   // Plans referencing context variables should never be cached
   @Override public boolean isDynamicFunction() {
     return true;
diff --git 
a/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java 
b/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
index 9d1b1a27ca..dc2489664f 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
@@ -320,6 +320,7 @@ public class SqlStdOperatorTable extends 
ReflectiveSqlOperatorTable {
   public static final SqlBasicFunction RAND = SqlBasicFunction
       .create("RAND", ReturnTypes.DOUBLE,
           OperandTypes.NILADIC.or(OperandTypes.NUMERIC), 
SqlFunctionCategory.NUMERIC)
+      .withDeterministic(false)
       .withDynamic(true);
 
   /**
diff --git 
a/core/src/test/java/org/apache/calcite/test/RelMdPredicatesTest.java 
b/core/src/test/java/org/apache/calcite/test/RelMdPredicatesTest.java
index 9b923a1c2f..4e2721a674 100644
--- a/core/src/test/java/org/apache/calcite/test/RelMdPredicatesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelMdPredicatesTest.java
@@ -20,15 +20,22 @@ import org.apache.calcite.plan.RelOptPredicateList;
 import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.core.JoinRelType;
 import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.fun.SqlLibraryOperators;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
 import org.apache.calcite.tools.FrameworkConfig;
 import org.apache.calcite.tools.RelBuilder;
 
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.CsvFileSource;
 
+import java.util.Arrays;
+
 import static org.apache.calcite.test.Matchers.sortsAs;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 /** Tests for {@link org.apache.calcite.rel.metadata.RelMdPredicates} class. */
 public class RelMdPredicatesTest {
@@ -91,4 +98,24 @@ public class RelMdPredicatesTest {
     assertThat(list.rightInferredPredicates, sortsAs(expectedPredicates));
   }
 
+  /** Test case for
+   * <a 
href="https://issues.apache.org/jira/browse/CALCITE-6507";>[CALCITE-6507]
+   * Random functions are incorrectly considered deterministic</a>. */
+  @Test void testRandomFunctionsAreNotConsideredConstant() {
+    FrameworkConfig config = RelBuilderTest.config().build();
+    for (SqlOperator randomOp : Arrays.asList(SqlStdOperatorTable.RAND,
+        SqlLibraryOperators.RANDOM, SqlStdOperatorTable.RAND_INTEGER)) {
+      RelBuilder b = RelBuilder.create(config);
+      RelNode rel = b
+          .scan("EMP")
+          .project(b.field(0), b.call(randomOp))
+          .sort(1)
+          .build();
+      RelMetadataQuery mq = rel.getCluster().getMetadataQuery();
+      RelOptPredicateList list = mq.getPulledUpPredicates(rel);
+      assertTrue(list.constantMap.isEmpty(),
+          "Operator " + randomOp + " considered constant: " + 
list.constantMap);
+    }
+  }
+
 }
diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java 
b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
index 6e2b97c113..bff3b87778 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -1393,6 +1393,39 @@ class RelOptRulesTest extends RelOptTestBase {
         .check();
   }
 
+  /** Test case for
+   * <a 
href="https://issues.apache.org/jira/browse/CALCITE-6507";>[CALCITE-6507]
+   * Random functions are incorrectly considered deterministic</a>. */
+  @Test void testSortRemoveConstantKeyDoesNotRemoveOrderByRand() {
+    final String sql = "SELECT ename FROM emp ORDER BY RAND()";
+    sql(sql)
+        .withRule(CoreRules.SORT_REMOVE_CONSTANT_KEYS)
+        .checkUnchanged();
+  }
+
+  /** Test case for
+   * <a 
href="https://issues.apache.org/jira/browse/CALCITE-6507";>[CALCITE-6507]
+   * Random functions are incorrectly considered deterministic</a>. */
+  @Test void testSortRemoveConstantKeyDoesNotRemoveOrderByRandInteger() {
+    final String sql = "SELECT ename FROM emp ORDER BY RAND_INTEGER(2)";
+    sql(sql)
+        .withRule(CoreRules.SORT_REMOVE_CONSTANT_KEYS)
+        .checkUnchanged();
+  }
+
+  /** Test case for
+   * <a 
href="https://issues.apache.org/jira/browse/CALCITE-6507";>[CALCITE-6507]
+   * Random functions are incorrectly considered deterministic</a>. */
+  @Test void testSortRemoveConstantKeyDoesNotRemoveOrderByRandom() {
+    final String sql = "SELECT ename FROM emp ORDER BY RANDOM()";
+    sql(sql)
+        .withFactory(f ->
+            f.withOperatorTable(opTab ->
+                SqlValidatorTest.operatorTableFor(SqlLibrary.POSTGRESQL)))
+        .withRule(CoreRules.SORT_REMOVE_CONSTANT_KEYS)
+        .checkUnchanged();
+  }
+
   /** Test case for
    * <a href="https://issues.apache.org/jira/browse/CALCITE-873";>[CALCITE-873]
    * SortRemoveConstantKeysRule should remove NULL literal sort keys
diff --git 
a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml 
b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
index 0a97c388da..45d25266b3 100644
--- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
@@ -14856,6 +14856,45 @@ LogicalProject(C=[$0])
         LogicalProject(DEPTNO=[$7], SAL=[$5])
           LogicalFilter(condition=[=($7, 10)])
             LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testSortRemoveConstantKeyDoesNotRemoveOrderByRand">
+    <Resource name="sql">
+      <![CDATA[SELECT ename FROM emp ORDER BY RAND()]]>
+    </Resource>
+    <Resource name="planBefore">
+      <![CDATA[
+LogicalProject(ENAME=[$0])
+  LogicalSort(sort0=[$1], dir0=[ASC])
+    LogicalProject(ENAME=[$1], EXPR$1=[RAND()])
+      LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testSortRemoveConstantKeyDoesNotRemoveOrderByRandInteger">
+    <Resource name="sql">
+      <![CDATA[SELECT ename FROM emp ORDER BY RAND_INTEGER(2)]]>
+    </Resource>
+    <Resource name="planBefore">
+      <![CDATA[
+LogicalProject(ENAME=[$0])
+  LogicalSort(sort0=[$1], dir0=[ASC])
+    LogicalProject(ENAME=[$1], EXPR$1=[RAND_INTEGER(2)])
+      LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testSortRemoveConstantKeyDoesNotRemoveOrderByRandom">
+    <Resource name="sql">
+      <![CDATA[SELECT ename FROM emp ORDER BY RANDOM()]]>
+    </Resource>
+    <Resource name="planBefore">
+      <![CDATA[
+LogicalProject(ENAME=[$0])
+  LogicalSort(sort0=[$1], dir0=[ASC])
+    LogicalProject(ENAME=[$1], EXPR$1=[RANDOM()])
+      LogicalTableScan(table=[[CATALOG, SALES, EMP]])
 ]]>
     </Resource>
   </TestCase>

Reply via email to