Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/22091 )
Change subject: IMPALA-13531: Calcite CTE frontend ...................................................................... Patch Set 12: (8 comments) http://gerrit.cloudera.org:8080/#/c/22091/10/be/src/service/frontend.cc File be/src/service/frontend.cc: http://gerrit.cloudera.org:8080/#/c/22091/10/be/src/service/frontend.cc@93 PS10, Line 93: DEFINE_string(cte_suggester_class, "org.apache.impala.calcite.cte.IdentitySuggester", > I have no preference. Not sure what the "right" thing to do is, but I supp Ack http://gerrit.cloudera.org:8080/#/c/22091/10/fe/src/main/java/org/apache/impala/planner/CTEScanNode.java File fe/src/main/java/org/apache/impala/planner/CTEScanNode.java: http://gerrit.cloudera.org:8080/#/c/22091/10/fe/src/main/java/org/apache/impala/planner/CTEScanNode.java@42 PS10, Line 42: public class CTEScanNode extends PlanNode { > This name doesn't seem appropriate. It functions more like an exchange. So Done http://gerrit.cloudera.org:8080/#/c/22091/10/fe/src/main/java/org/apache/impala/planner/SequenceNode.java File fe/src/main/java/org/apache/impala/planner/SequenceNode.java: http://gerrit.cloudera.org:8080/#/c/22091/10/fe/src/main/java/org/apache/impala/planner/SequenceNode.java@53 PS10, Line 53: assignConjuncts(analyzer); > It might not be necessary. The preconditions is asserting there are none. T Cleaned this up. A bunch was copied from SubplanNode. http://gerrit.cloudera.org:8080/#/c/22091/10/java/calcite-planner/src/main/java/org/apache/impala/calcite/cte/Exception.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/cte/Exception.java: http://gerrit.cloudera.org:8080/#/c/22091/10/java/calcite-planner/src/main/java/org/apache/impala/calcite/cte/Exception.java@21 PS10, Line 21: > Optional: I guess I'm ok with this, but it could be confusing to have the " Done http://gerrit.cloudera.org:8080/#/c/22091/10/java/calcite-planner/src/main/java/org/apache/impala/calcite/cte/SuggesterFactory.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/cte/SuggesterFactory.java: http://gerrit.cloudera.org:8080/#/c/22091/10/java/calcite-planner/src/main/java/org/apache/impala/calcite/cte/SuggesterFactory.java@27 PS10, Line 27: public static final String CTE_THRESHOLD = "impala.cte_threshold"; > Ah, I guess I missed it. I don't have a problem keeping the options togeth Ack http://gerrit.cloudera.org:8080/#/c/22091/10/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaCTEConsumer.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaCTEConsumer.java: http://gerrit.cloudera.org:8080/#/c/22091/10/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaCTEConsumer.java@36 PS10, Line 36: private static final String PREFIX = "cte"; > SJC Placeholder: Check if RelOptMaterialization.qualifiedTableName is used Done http://gerrit.cloudera.org:8080/#/c/22091/10/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaCTEConsumer.java@74 PS10, Line 74: NodeWithExprs plan = NodeCreationUtils.createCTEConsumerPlanNode(context, > So the general mechanism I've tried to use to avoid mutation is to use Pare Done http://gerrit.cloudera.org:8080/#/c/22091/10/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java: http://gerrit.cloudera.org:8080/#/c/22091/10/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java@203 PS10, Line 203: CoreRules.UNION_TO_DISTINCT, > I have a patch that gets rid of this by using a parent/child relationship ( Done -- 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: 12 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, 03 Dec 2025 19:31:51 +0000 Gerrit-HasComments: Yes
