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_; So this is a little bit risky, but prolly ok. When this is created, this isn't the last rule run in the plan. If the "copy" method is called for ImpalaConsumer, this list of RelNodes may contain a stale ImpalaConsumer/ImpalaProducer. So if any sub-inputs change after that on the new copied ImpalaConsumer, the stale one here won't be calling "getPlanNode" on the right RelNodes. Given that everything is working so far, I think we're ok. If we weren't, I would expect to see things like an Aggregate RelNode that was not converted to ImpalaAggregateRel. Prolly what is happening is that Calcite is keeping the original ImpalaConsumer and calling "setInput(i)" when the Aggregate gets changed to an ImpalaAggregateRel. But I'm thinking as a safety measure (and protection against any Calcite upgrade), maybe we should throw an exception in ImpalaConsumer.copy() and ImpalaProducer.copy(). If, for some reason, you throw the exception there and the copy is getting called? Then I'm a little surprised at how it is working, and I'll prolly dig deeper to figure out why. -- 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: Wed, 10 Dec 2025 17:03:42 +0000 Gerrit-HasComments: Yes
