Steve Carlin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22091 )

Change subject: IMPALA-13531: Calcite CTE frontend
......................................................................


Patch Set 14:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/22091/14/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaSequence.java
File 
java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaSequence.java:

http://gerrit.cloudera.org:8080/#/c/22091/14/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaSequence.java@39
PS14, Line 39:   private final List<RelNode> inputs_;
> The problem I think exists is that these aren't normal "inputs".  It's a se
Ok, I'm thinking through this a bit more...

So if you understood what I said in my last comment, basically, I'm saying that 
Calcite's internal rules and programs will be calling "replaceInput" and "copy" 
potentially within the tree but not knowing that ImpalaSequence has a copy of 
it.

I also mentioned that this is special because it's not done at the very end 
when we know we're converting Calcite RelNodes to Impala RelNodes.

And basically, the real difference here is that we are treating CTEConsumer and 
CTEProducer as Calcite RelNodes, but ImpalaSequence really doesn't fit into 
that category because of the way it holds inputs.

So another alternative to fix this, and prolly a better way:  We only need 
ImpalaSequence as a top physical level node.  So I think we should hold off on 
creation of ImpalaSequence until the runImpalaConvertProgram is run.  When it 
hits this point, we know we're done with the Calcite rules and just running our 
final rule.  The replaceInput and copy will never be called on ImpalaSequence.



--
To view, visit http://gerrit.cloudera.org:8080/22091
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id0840c0859d2fe25628d799a18d302cee1eb36e8
Gerrit-Change-Number: 22091
Gerrit-PatchSet: 14
Gerrit-Owner: Michael Smith <[email protected]>
Gerrit-Reviewer: Anonymous Coward (816)
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Steve Carlin <[email protected]>
Gerrit-Comment-Date: Sat, 13 Dec 2025 17:59:41 +0000
Gerrit-HasComments: Yes

Reply via email to