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>