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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
