You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "darpan-e6 (via GitHub)" <gi...@apache.org> on 2026/05/27 06:54:09 UTC
[PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
darpan-e6 opened a new pull request, #4970:
URL: https://github.com/apache/calcite/pull/4970
## Jira Link
[CALCITE-7551](https://issues.apache.org/jira/browse/CALCITE-7551)
## Changes Proposed
Several optimizer and `RelBuilder`-level transformations inline a lower-projection expression into multiple positions of an upper expression without checking whether the inlined expression is non-deterministic. When the lower expression is non-deterministic (e.g. `RAND()`), this silently turns one evaluation into many, changing query semantics.
### Reproduction
```sql
SELECT a, a + 1 AS b FROM (SELECT rand() AS a)
```
Before the fix, `SqlToRelConverter` produces:
```
LogicalProject(A=[RAND()], B=[+(RAND(), 1)])
LogicalValues(tuples=[[{ 0 }]])
```
The two `RAND()` calls are evaluated independently, so `B - A` is no longer always `1`. After the fix the inner projection is preserved and `A`, `B` share a single evaluation:
```
LogicalProject(A=[$0], B=[+($0, 1)])
LogicalProject(A=[RAND()])
LogicalValues(tuples=[[{ 0 }]])
```
### Affected code paths and fixes
| Component | Mechanism | Fix |
|---|---|---|
| `RelOptUtil.pushPastProjectUnlessBloat` | `pushShuttle(Project)` substitutes every `RexInputRef` with the underlying projection. Only `RexOver` was guarded; determinism was not. | Count `RexInputRef` uses per bottom slot; return `null` if any non-deterministic bottom expression is referenced more than once. Covers `ProjectMergeRule`, `FilterProjectTransposeRule`'s helper call, and the `RelBuilder` bloat merge. |
| `FilterProjectTransposeRule` | Calls `pushPastProjectUnlessBloat` AND re-emits the original projection above the new filter — so even with the helper guarded, `Project(rand() AS a)` on top of `Filter(rand() > ...)` splits one evaluation into two. | Skip when any projected expression is non-deterministic, parallel to the existing `containsOver` skip. |
| `JoinProjectTransposeRule` | Uses `RexProgramBuilder.mergePrograms` + `mergedProgram.expandLocalRef(condition)`. `expandLocalRef` flattens local refs back into a `RexNode` tree, inlining the underlying expression at every occurrence. | Skip non-deterministic projections, parallel to the existing `containsOver` skip on each side. |
| `SemiJoinProjectTransposeRule` | Same `mergePrograms` + `expandLocalRef` pattern. | Same skip. |
The fix uses `RexUtil.isDeterministic(...)`, which correctly distinguishes operators that may return different values across call sites within a single statement (`RAND` overrides to `false`) from operators that are non-deterministic only across executions (`CURRENT_TIMESTAMP`, marked `isDynamicFunction = true` but still `isDeterministic = true`). The latter is safe to duplicate and is unaffected by this change.
### Tests
- `RelOptRulesTest.testProjectMergeShouldIgnoreNonDeterministic`
- `RelOptRulesTest.testFilterProjectTransposeShouldIgnoreNonDeterministic`
- `RelOptRulesTest.testJoinProjectTransposeShouldIgnoreNonDeterministic`
- `RelOptRulesTest.testSemiJoinProjectTransposeShouldIgnoreNonDeterministic`
- `SqlToRelConverterTest.testRandNotDuplicatedInProjectionMerge`
Each follows the existing `.checkUnchanged()` / `.ok()` fixture pattern (parallel to `testProjectMergeShouldIgnoreOver`) and verifies that the rule (or the `SqlToRelConverter` build path) does not produce a plan with duplicated `RAND()` calls.
### Confirmed unaffected (verified by additional probing during investigation)
`CalcMergeRule` (RexProgram preserves CSE via local refs), `ProjectReduceExpressionsRule`, `ProjectValuesMergeRule`, `ProjectFilterTransposeRule`, `ProjectJoinTransposeRule`, `ProjectCorrelateTransposeRule`, `AggregateProjectMergeRule`, `FilterSetOpTransposeRule` (textual duplication only — each row flows through one branch of a `UNION ALL`, so per-row evaluation count is unchanged), `ProjectToWindowRule`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4970:
URL: https://github.com/apache/calcite/pull/4970#issuecomment-4561006659
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4970) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4970&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4970&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4970&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [93.2% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4970&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4970&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4970)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4970:
URL: https://github.com/apache/calcite/pull/4970#discussion_r3363959753
##########
core/src/main/java/org/apache/calcite/plan/RelOptUtil.java:
##########
@@ -3346,6 +3346,21 @@ public static List<RexNode> pushPastProject(List<? extends RexNode> nodes,
// function? Possibly. But it's invalid SQL, so don't go there.
return null;
}
+ // [CALCITE-7551] Refuse to merge if it would duplicate a
+ // non-deterministic expression (e.g. RAND()).
+ final List<RexNode> bottom = project.getProjects();
+ final int[] refs = new int[bottom.size()];
+ new RexVisitorImpl<Void>(true) {
+ @Override public Void visitInputRef(RexInputRef ref) {
+ refs[ref.getIndex()]++;
+ return null;
+ }
+ }.visitEach(nodes);
+ for (int i = 0; i < refs.length; i++) {
Review Comment:
I think so too. Can you please open a new ticket for this (or check whether there is one already open)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4970:
URL: https://github.com/apache/calcite/pull/4970#issuecomment-4637778294
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4970) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4970&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4970&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4970&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [95.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4970&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4970&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4970)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "darpan-e6 (via GitHub)" <gi...@apache.org>.
darpan-e6 commented on code in PR #4970:
URL: https://github.com/apache/calcite/pull/4970#discussion_r3777704973
##########
core/src/main/java/org/apache/calcite/plan/RelOptUtil.java:
##########
@@ -3346,6 +3346,21 @@ public static List<RexNode> pushPastProject(List<? extends RexNode> nodes,
// function? Possibly. But it's invalid SQL, so don't go there.
return null;
}
+ // [CALCITE-7551] Refuse to merge if it would duplicate a
+ // non-deterministic expression (e.g. RAND()).
+ final List<RexNode> bottom = project.getProjects();
+ final int[] refs = new int[bottom.size()];
+ new RexVisitorImpl<Void>(true) {
+ @Override public Void visitInputRef(RexInputRef ref) {
+ refs[ref.getIndex()]++;
+ return null;
+ }
+ }.visitEach(nodes);
+ for (int i = 0; i < refs.length; i++) {
Review Comment:
Looks like discussion has been going on for a long time. [CALCITE-2823](https://issues.apache.org/jira/browse/CALCITE-2823)
Should we go ahead and create a new ticket to decide the identifier for all 5 cases that @julianhyde is mentioning in the last comment of this ticket or should we use this existing ticket only?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4970:
URL: https://github.com/apache/calcite/pull/4970#issuecomment-4561355440
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4970) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4970&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4970&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4970&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [95.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4970&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4970&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4970)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "darpan-e6 (via GitHub)" <gi...@apache.org>.
darpan-e6 commented on code in PR #4970:
URL: https://github.com/apache/calcite/pull/4970#discussion_r3315311244
##########
core/src/main/java/org/apache/calcite/plan/RelOptUtil.java:
##########
@@ -3346,6 +3346,21 @@ public static List<RexNode> pushPastProject(List<? extends RexNode> nodes,
// function? Possibly. But it's invalid SQL, so don't go there.
return null;
}
+ // [CALCITE-7551] Refuse to merge if it would duplicate a
+ // non-deterministic expression (e.g. RAND()).
+ final List<RexNode> bottom = project.getProjects();
+ final int[] refs = new int[bottom.size()];
+ new RexVisitorImpl<Void>(true) {
+ @Override public Void visitInputRef(RexInputRef ref) {
+ refs[ref.getIndex()]++;
+ return null;
+ }
+ }.visitEach(nodes);
+ for (int i = 0; i < refs.length; i++) {
Review Comment:
As far as I understand, CURRENT_TIMESTAMP is actually different than non-deterministic functions like RAND().
1. Non-deterministic function: it may return different values for every evaluations.
1.1 Returns false to `isDeterministic()`.
1.2 Returns false to `isDynamicFunction()`.
2. Dynamic Function: It will return same value at every call site within one statement; can differ across executions
2.1 Returns true to `isDeterministic()`.
2.2 Returns true to `isDynamicFunction()`.
And this path only blocks when we get `isDeterministic()` method response as false. Dynamic functions like `CURRENT_TIMESTAMP` can be duplicated and actually it is safe to duplicate them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on code in PR #4970:
URL: https://github.com/apache/calcite/pull/4970#discussion_r3361817676
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -1533,6 +1533,87 @@ private void checkSemiOrAntiJoinProjectTranspose(JoinRelType type) {
.check();
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-7551">[CALCITE-7551]
+ * Project/Filter/Join transpose and merge rules can duplicate
+ * non-deterministic expressions</a>. JoinProjectTransposeRule must
+ * not pull a project containing a non-deterministic expression above
+ * the join, because it inlines the expression into the new join
+ * condition via {@code mergedProgram.expandLocalRef}. */
+ @Test void testJoinProjectTransposeShouldIgnoreNonDeterministic() {
+ final Function<RelBuilder, RelNode> relFn = b -> b
+ .scan("EMP")
+ .project(b.field("EMPNO"),
+ b.alias(b.call(SqlStdOperatorTable.RAND), "r"))
+ .scan("DEPT")
+ .join(JoinRelType.INNER,
+ b.and(
+ b.greaterThan(b.field(2, 0, "r"), b.literal(0.0)),
+ b.lessThan(b.field(2, 0, "r"), b.literal(1.0))))
+ .build();
+ relFn(relFn).withRule(CoreRules.JOIN_PROJECT_LEFT_TRANSPOSE).checkUnchanged();
+ }
+
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-7551">[CALCITE-7551]
+ * Project/Filter/Join transpose and merge rules can duplicate
+ * non-deterministic expressions</a>. The transpose is still allowed when
+ * the join condition only references deterministic projected columns,
+ * even if the project also computes a non-deterministic column (here
+ * {@code r} is RAND() but the join is on DEPTNO). */
+ @Test void testJoinProjectTransposeWithUnrelatedNonDeterministic() {
+ final Function<RelBuilder, RelNode> relFn = b -> b
Review Comment:
Would it be possible to convert all the tests to SQL? Using builders increases the reading complexity and makes the test intent less obvious.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on code in PR #4970:
URL: https://github.com/apache/calcite/pull/4970#discussion_r3314895841
##########
core/src/main/java/org/apache/calcite/plan/RelOptUtil.java:
##########
@@ -3346,6 +3346,21 @@ public static List<RexNode> pushPastProject(List<? extends RexNode> nodes,
// function? Possibly. But it's invalid SQL, so don't go there.
return null;
}
+ // [CALCITE-7551] Refuse to merge if it would duplicate a
+ // non-deterministic expression (e.g. RAND()).
+ final List<RexNode> bottom = project.getProjects();
+ final int[] refs = new int[bottom.size()];
+ new RexVisitorImpl<Void>(true) {
+ @Override public Void visitInputRef(RexInputRef ref) {
+ refs[ref.getIndex()]++;
+ return null;
+ }
+ }.visitEach(nodes);
+ for (int i = 0; i < refs.length; i++) {
Review Comment:
Agreed. We should complete the fine-grained judgment of the deterministic function first before completing this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4970:
URL: https://github.com/apache/calcite/pull/4970#discussion_r3312892470
##########
core/src/main/java/org/apache/calcite/plan/RelOptUtil.java:
##########
@@ -3346,6 +3346,21 @@ public static List<RexNode> pushPastProject(List<? extends RexNode> nodes,
// function? Possibly. But it's invalid SQL, so don't go there.
return null;
}
+ // [CALCITE-7551] Refuse to merge if it would duplicate a
+ // non-deterministic expression (e.g. RAND()).
+ final List<RexNode> bottom = project.getProjects();
+ final int[] refs = new int[bottom.size()];
+ new RexVisitorImpl<Void>(true) {
+ @Override public Void visitInputRef(RexInputRef ref) {
+ refs[ref.getIndex()]++;
+ return null;
+ }
+ }.visitEach(nodes);
+ for (int i = 0; i < refs.length; i++) {
Review Comment:
As discussed in Jira, this is a bit too conservative, since it will not distinguish CURRENT_TIMESTAMP from RAND. But fixing that may be in the scope of a separate PR - we need really two separate notions of nondeterminism.
##########
core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java:
##########
@@ -6133,4 +6133,16 @@ void checkUserDefinedOrderByOver(NullCollation nullCollation) {
+ "FROM emp JOIN dept using (deptno)";
sql(sql).withConformance(SqlConformanceEnum.PRESTO).ok();
}
+
+ /** Test case of
Review Comment:
why is SqlToRel connected to these rules?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "darpan-e6 (via GitHub)" <gi...@apache.org>.
darpan-e6 commented on code in PR #4970:
URL: https://github.com/apache/calcite/pull/4970#discussion_r3338934037
##########
core/src/main/java/org/apache/calcite/plan/RelOptUtil.java:
##########
@@ -3346,6 +3346,21 @@ public static List<RexNode> pushPastProject(List<? extends RexNode> nodes,
// function? Possibly. But it's invalid SQL, so don't go there.
return null;
}
+ // [CALCITE-7551] Refuse to merge if it would duplicate a
+ // non-deterministic expression (e.g. RAND()).
+ final List<RexNode> bottom = project.getProjects();
+ final int[] refs = new int[bottom.size()];
+ new RexVisitorImpl<Void>(true) {
+ @Override public Void visitInputRef(RexInputRef ref) {
+ refs[ref.getIndex()]++;
+ return null;
+ }
+ }.visitEach(nodes);
+ for (int i = 0; i < refs.length; i++) {
Review Comment:
You're right, that comment mixed up the two flags. The "same value at every call site within one statement" property I attached to `isDynamicFunction()` is not in its JavaDoc; I generalized from `CURRENT_TIMESTAMP`'s behavior to `isDynamicFunction` as a whole, which is wrong, `isDynamicFunction` is overloaded (e.g. `SqlRandIntegerFunction` also returns `true`), and its actual contract is just plan-cache invalidation.
Scoping back to this PR: the bug only affects operators that can return *different values across call sites within a single statement*, i.e. operators with `isDeterministic() == false`, like `RAND` and `RAND_INTEGER`. The other operators that override `isDynamicFunction()` to `true` (`SqlAbstractTimeFunction`, `SqlCurrentDateFunction`, `SqlBaseContextVariable`) are per-statement-stable: duplicating them in the plan is observationally a no-op, because every occurrence resolves to the same value within an execution. So they are not affected by this bug and don't need to be blocked.
The guard in the fix uses `RexUtil.isDeterministic(...)`, which is exactly the right discriminator for the affected set. I'll update the PR description to drop the misleading framing of `isDynamicFunction()`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "darpan-e6 (via GitHub)" <gi...@apache.org>.
darpan-e6 commented on code in PR #4970:
URL: https://github.com/apache/calcite/pull/4970#discussion_r3366864174
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -1533,6 +1533,87 @@ private void checkSemiOrAntiJoinProjectTranspose(JoinRelType type) {
.check();
}
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-7551">[CALCITE-7551]
+ * Project/Filter/Join transpose and merge rules can duplicate
+ * non-deterministic expressions</a>. JoinProjectTransposeRule must
+ * not pull a project containing a non-deterministic expression above
+ * the join, because it inlines the expression into the new join
+ * condition via {@code mergedProgram.expandLocalRef}. */
+ @Test void testJoinProjectTransposeShouldIgnoreNonDeterministic() {
+ final Function<RelBuilder, RelNode> relFn = b -> b
+ .scan("EMP")
+ .project(b.field("EMPNO"),
+ b.alias(b.call(SqlStdOperatorTable.RAND), "r"))
+ .scan("DEPT")
+ .join(JoinRelType.INNER,
+ b.and(
+ b.greaterThan(b.field(2, 0, "r"), b.literal(0.0)),
+ b.lessThan(b.field(2, 0, "r"), b.literal(1.0))))
+ .build();
+ relFn(relFn).withRule(CoreRules.JOIN_PROJECT_LEFT_TRANSPOSE).checkUnchanged();
+ }
+
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-7551">[CALCITE-7551]
+ * Project/Filter/Join transpose and merge rules can duplicate
+ * non-deterministic expressions</a>. The transpose is still allowed when
+ * the join condition only references deterministic projected columns,
+ * even if the project also computes a non-deterministic column (here
+ * {@code r} is RAND() but the join is on DEPTNO). */
+ @Test void testJoinProjectTransposeWithUnrelatedNonDeterministic() {
+ final Function<RelBuilder, RelNode> relFn = b -> b
Review Comment:
Thanks for the review, did the changes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on code in PR #4970:
URL: https://github.com/apache/calcite/pull/4970#discussion_r3315018605
##########
core/src/main/java/org/apache/calcite/rel/rules/FilterProjectTransposeRule.java:
##########
@@ -165,6 +165,13 @@ protected FilterProjectTransposeRule(
// it can be pushed down. For now we don't support this.
return;
}
+ // Pushing the filter below the project would split a single
+ // non-deterministic evaluation (e.g. RAND()) into two: one consumed by
+ // the new filter condition, and the original still produced by the
+ // project above. Refuse to transpose in that case.
+ if (!project.getProjects().stream().allMatch(RexUtil::isDeterministic)) {
Review Comment:
Blocks transpose (because RAND() exists in project)? eg:
```
SELECT * FROM (
SELECT RAND() as r, col1 as b FROM emp
)
WHERE col1 > 0
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4970:
URL: https://github.com/apache/calcite/pull/4970#discussion_r3343692019
##########
core/src/main/java/org/apache/calcite/plan/RelOptUtil.java:
##########
@@ -3346,6 +3346,21 @@ public static List<RexNode> pushPastProject(List<? extends RexNode> nodes,
// function? Possibly. But it's invalid SQL, so don't go there.
return null;
}
+ // [CALCITE-7551] Refuse to merge if it would duplicate a
+ // non-deterministic expression (e.g. RAND()).
+ final List<RexNode> bottom = project.getProjects();
+ final int[] refs = new int[bottom.size()];
+ new RexVisitorImpl<Void>(true) {
+ @Override public Void visitInputRef(RexInputRef ref) {
+ refs[ref.getIndex()]++;
+ return null;
+ }
+ }.visitEach(nodes);
+ for (int i = 0; i < refs.length; i++) {
Review Comment:
That is fine, but I think we have no method to discriminate between RAND and CURRENT_TIMESTAMP. Maybe we need to introduce such a tool?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "darpan-e6 (via GitHub)" <gi...@apache.org>.
darpan-e6 commented on code in PR #4970:
URL: https://github.com/apache/calcite/pull/4970#discussion_r3347457470
##########
core/src/main/java/org/apache/calcite/plan/RelOptUtil.java:
##########
@@ -3346,6 +3346,21 @@ public static List<RexNode> pushPastProject(List<? extends RexNode> nodes,
// function? Possibly. But it's invalid SQL, so don't go there.
return null;
}
+ // [CALCITE-7551] Refuse to merge if it would duplicate a
+ // non-deterministic expression (e.g. RAND()).
+ final List<RexNode> bottom = project.getProjects();
+ final int[] refs = new int[bottom.size()];
+ new RexVisitorImpl<Void>(true) {
+ @Override public Void visitInputRef(RexInputRef ref) {
+ refs[ref.getIndex()]++;
+ return null;
+ }
+ }.visitEach(nodes);
+ for (int i = 0; i < refs.length; i++) {
Review Comment:
Yes, makes sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4970:
URL: https://github.com/apache/calcite/pull/4970#issuecomment-5724815418
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "darpan-e6 (via GitHub)" <gi...@apache.org>.
darpan-e6 commented on code in PR #4970:
URL: https://github.com/apache/calcite/pull/4970#discussion_r3315343197
##########
core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java:
##########
@@ -6133,4 +6133,16 @@ void checkUserDefinedOrderByOver(NullCollation nullCollation) {
+ "FROM emp JOIN dept using (deptno)";
sql(sql).withConformance(SqlConformanceEnum.PRESTO).ok();
}
+
+ /** Test case of
Review Comment:
Because `SqlToRelConverter` builds its `RelNode` tree through `RelBuilder`, and `RelBuilder` eagerly merges adjacent projects at construction time using the same helper that the planner rules use.
Concretely, `RelBuilder.project_` method uses the method `RelOptUtil.pushPastProjectUnlessBloat(nodeList, project, config.bloat())` and this is the method in which we are putting the fix, so I thought of putting a test here as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4970:
URL: https://github.com/apache/calcite/pull/4970#issuecomment-5126487041
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4970:
URL: https://github.com/apache/calcite/pull/4970#discussion_r3777842073
##########
core/src/main/java/org/apache/calcite/plan/RelOptUtil.java:
##########
@@ -3346,6 +3346,21 @@ public static List<RexNode> pushPastProject(List<? extends RexNode> nodes,
// function? Possibly. But it's invalid SQL, so don't go there.
return null;
}
+ // [CALCITE-7551] Refuse to merge if it would duplicate a
+ // non-deterministic expression (e.g. RAND()).
+ final List<RexNode> bottom = project.getProjects();
+ final int[] refs = new int[bottom.size()];
+ new RexVisitorImpl<Void>(true) {
+ @Override public Void visitInputRef(RexInputRef ref) {
+ refs[ref.getIndex()]++;
+ return null;
+ }
+ }.visitEach(nodes);
+ for (int i = 0; i < refs.length; i++) {
Review Comment:
We need a ticket that proposes a solution for handling the different classes of functions. I would file a new one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "darpan-e6 (via GitHub)" <gi...@apache.org>.
darpan-e6 commented on code in PR #4970:
URL: https://github.com/apache/calcite/pull/4970#discussion_r3338934037
##########
core/src/main/java/org/apache/calcite/plan/RelOptUtil.java:
##########
@@ -3346,6 +3346,21 @@ public static List<RexNode> pushPastProject(List<? extends RexNode> nodes,
// function? Possibly. But it's invalid SQL, so don't go there.
return null;
}
+ // [CALCITE-7551] Refuse to merge if it would duplicate a
+ // non-deterministic expression (e.g. RAND()).
+ final List<RexNode> bottom = project.getProjects();
+ final int[] refs = new int[bottom.size()];
+ new RexVisitorImpl<Void>(true) {
+ @Override public Void visitInputRef(RexInputRef ref) {
+ refs[ref.getIndex()]++;
+ return null;
+ }
+ }.visitEach(nodes);
+ for (int i = 0; i < refs.length; i++) {
Review Comment:
You're right, that comment mixed up the two flags. The "same value at every call site within one statement" property I attached to `isDynamicFunction()` is not in its JavaDoc; I generalized from `CURRENT_TIMESTAMP`'s behavior to `isDynamicFunction` as a whole, which is wrong — `isDynamicFunction` is overloaded (e.g. `SqlRandIntegerFunction` also returns `true`), and its actual contract is just plan-cache invalidation.
Scoping back to this PR: the bug only affects operators that can return *different values across call sites within a single statement* — i.e. operators with `isDeterministic() == false`, like `RAND` and `RAND_INTEGER`. The other operators that override `isDynamicFunction()` to `true` (`SqlAbstractTimeFunction`, `SqlCurrentDateFunction`, `SqlBaseContextVariable`) are per-statement-stable: duplicating them in the plan is observationally a no-op, because every occurrence resolves to the same value within an execution. So they are not affected by this bug and don't need to be blocked.
The guard in the fix uses `RexUtil.isDeterministic(...)`, which is exactly the right discriminator for the affected set. I'll update the PR description to drop the misleading framing of `isDynamicFunction()`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "darpan-e6 (via GitHub)" <gi...@apache.org>.
darpan-e6 commented on code in PR #4970:
URL: https://github.com/apache/calcite/pull/4970#discussion_r3489563061
##########
core/src/main/java/org/apache/calcite/plan/RelOptUtil.java:
##########
@@ -3346,6 +3346,21 @@ public static List<RexNode> pushPastProject(List<? extends RexNode> nodes,
// function? Possibly. But it's invalid SQL, so don't go there.
return null;
}
+ // [CALCITE-7551] Refuse to merge if it would duplicate a
+ // non-deterministic expression (e.g. RAND()).
+ final List<RexNode> bottom = project.getProjects();
+ final int[] refs = new int[bottom.size()];
+ new RexVisitorImpl<Void>(true) {
+ @Override public Void visitInputRef(RexInputRef ref) {
+ refs[ref.getIndex()]++;
+ return null;
+ }
+ }.visitEach(nodes);
+ for (int i = 0; i < refs.length; i++) {
Review Comment:
Is it possible to move this forward? @xiedeyantu Please let me know if anything is required from my side.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "darpan-e6 (via GitHub)" <gi...@apache.org>.
darpan-e6 commented on code in PR #4970:
URL: https://github.com/apache/calcite/pull/4970#discussion_r3360260323
##########
core/src/main/java/org/apache/calcite/rel/rules/FilterProjectTransposeRule.java:
##########
@@ -165,6 +165,13 @@ protected FilterProjectTransposeRule(
// it can be pushed down. For now we don't support this.
return;
}
+ // Pushing the filter below the project would split a single
+ // non-deterministic evaluation (e.g. RAND()) into two: one consumed by
+ // the new filter condition, and the original still produced by the
+ // project above. Refuse to transpose in that case.
+ if (!project.getProjects().stream().allMatch(RexUtil::isDeterministic)) {
Review Comment:
Did the changes, can you review again please?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4970:
URL: https://github.com/apache/calcite/pull/4970#discussion_r3350677533
##########
core/src/main/java/org/apache/calcite/plan/RelOptUtil.java:
##########
@@ -3346,6 +3346,21 @@ public static List<RexNode> pushPastProject(List<? extends RexNode> nodes,
// function? Possibly. But it's invalid SQL, so don't go there.
return null;
}
+ // [CALCITE-7551] Refuse to merge if it would duplicate a
+ // non-deterministic expression (e.g. RAND()).
+ final List<RexNode> bottom = project.getProjects();
+ final int[] refs = new int[bottom.size()];
+ new RexVisitorImpl<Void>(true) {
+ @Override public Void visitInputRef(RexInputRef ref) {
+ refs[ref.getIndex()]++;
+ return null;
+ }
+ }.visitEach(nodes);
+ for (int i = 0; i < refs.length; i++) {
Review Comment:
That should probably be a separate JIRA issue and PR. Coming up with a good name for this property of CURRENT_TIMESTAMP may be difficult.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on code in PR #4970:
URL: https://github.com/apache/calcite/pull/4970#discussion_r3361851737
##########
core/src/main/java/org/apache/calcite/plan/RelOptUtil.java:
##########
@@ -3346,6 +3346,21 @@ public static List<RexNode> pushPastProject(List<? extends RexNode> nodes,
// function? Possibly. But it's invalid SQL, so don't go there.
return null;
}
+ // [CALCITE-7551] Refuse to merge if it would duplicate a
+ // non-deterministic expression (e.g. RAND()).
+ final List<RexNode> bottom = project.getProjects();
+ final int[] refs = new int[bottom.size()];
+ new RexVisitorImpl<Void>(true) {
+ @Override public Void visitInputRef(RexInputRef ref) {
+ refs[ref.getIndex()]++;
+ return null;
+ }
+ }.visitEach(nodes);
+ for (int i = 0; i < refs.length; i++) {
Review Comment:
After thinking about it carefully, I believe we should create this JIRA before this PR is merged. However, it shouldn't block the PR from being merged, since the stricter approach does not introduce any incorrect behavior. Am I right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "darpan-e6 (via GitHub)" <gi...@apache.org>.
darpan-e6 commented on PR #4970:
URL: https://github.com/apache/calcite/pull/4970#issuecomment-4629245175
> Can you please also fix this conflict?
@mihaibudiu Fixed the conflict
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on PR #4970:
URL: https://github.com/apache/calcite/pull/4970#issuecomment-4628248109
Can you please also fix this conflict?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4970:
URL: https://github.com/apache/calcite/pull/4970#issuecomment-4829482097
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4970) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4970&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4970&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4970&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [95.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4970&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4970&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4970)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "darpan-e6 (via GitHub)" <gi...@apache.org>.
darpan-e6 commented on code in PR #4970:
URL: https://github.com/apache/calcite/pull/4970#discussion_r3781591803
##########
core/src/main/java/org/apache/calcite/plan/RelOptUtil.java:
##########
@@ -3346,6 +3346,21 @@ public static List<RexNode> pushPastProject(List<? extends RexNode> nodes,
// function? Possibly. But it's invalid SQL, so don't go there.
return null;
}
+ // [CALCITE-7551] Refuse to merge if it would duplicate a
+ // non-deterministic expression (e.g. RAND()).
+ final List<RexNode> bottom = project.getProjects();
+ final int[] refs = new int[bottom.size()];
+ new RexVisitorImpl<Void>(true) {
+ @Override public Void visitInputRef(RexInputRef ref) {
+ refs[ref.getIndex()]++;
+ return null;
+ }
+ }.visitEach(nodes);
+ for (int i = 0; i < refs.length; i++) {
Review Comment:
Got it. Thanks. I will brainstorm through the weekend and will also see what AI agents has to say about it. Will propose something based on that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4970:
URL: https://github.com/apache/calcite/pull/4970#discussion_r3336083481
##########
core/src/main/java/org/apache/calcite/plan/RelOptUtil.java:
##########
@@ -3346,6 +3346,21 @@ public static List<RexNode> pushPastProject(List<? extends RexNode> nodes,
// function? Possibly. But it's invalid SQL, so don't go there.
return null;
}
+ // [CALCITE-7551] Refuse to merge if it would duplicate a
+ // non-deterministic expression (e.g. RAND()).
+ final List<RexNode> bottom = project.getProjects();
+ final int[] refs = new int[bottom.size()];
+ new RexVisitorImpl<Void>(true) {
+ @Override public Void visitInputRef(RexInputRef ref) {
+ refs[ref.getIndex()]++;
+ return null;
+ }
+ }.visitEach(nodes);
+ for (int i = 0; i < refs.length; i++) {
Review Comment:
The JavaDoc of `isDynamicFunction` says:
```
/**
* Returns whether it is unsafe to cache query plans referencing this
* operator; false is assumed by default.
*/
public boolean isDynamicFunction() {
```
which is not exactly what you seem to imply.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "darpan-e6 (via GitHub)" <gi...@apache.org>.
darpan-e6 commented on code in PR #4970:
URL: https://github.com/apache/calcite/pull/4970#discussion_r3315343197
##########
core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java:
##########
@@ -6133,4 +6133,16 @@ void checkUserDefinedOrderByOver(NullCollation nullCollation) {
+ "FROM emp JOIN dept using (deptno)";
sql(sql).withConformance(SqlConformanceEnum.PRESTO).ok();
}
+
+ /** Test case of
Review Comment:
Because `SqlToRelConverter` builds its `RelNode` tree through `RelBuilder`, and `RelBuilder` eagerly merges adjacent projects at construction time using the same helper that the planner rules use.
Concretely, `RelBuilder.project_` method uses the method `RelOptUtil.pushPastProjectUnlessBloat(nodeList, project, config.bloat())` which we are fixing as part of this PR, so I thought of putting a test here as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7551] Project/Filter/Join transpose and merge rules should not duplicate non-deterministic expressions (e.g. RAND()) [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4970:
URL: https://github.com/apache/calcite/pull/4970#issuecomment-4552438291
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4970) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4970&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4970&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4970&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [92.3% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4970&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4970&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4970)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org