Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/21575 )
Change subject: IMPALA-13221: Calcite: Enable tpcds and tpch queries ...................................................................... Patch Set 11: (3 comments) http://gerrit.cloudera.org:8080/#/c/21575/10/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/util/ImpalaBaseTableRef.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/util/ImpalaBaseTableRef.java: http://gerrit.cloudera.org:8080/#/c/21575/10/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/util/ImpalaBaseTableRef.java@39 PS10, Line 39: SimplifitedAnalyzer > Nit: spelling, SimplifiedAnalyzer Done http://gerrit.cloudera.org:8080/#/c/21575/10/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ConvertToCNFRules.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ConvertToCNFRules.java: http://gerrit.cloudera.org:8080/#/c/21575/10/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ConvertToCNFRules.java@135 PS10, Line 135: > If this works similarly to Impala's existing code, we can use the max_cnf_e Added a Jira to use the query option. The context needs to be available here which should be doable. In the meantime, changed 100 to 200 which is the default value, as you mentioned. http://gerrit.cloudera.org:8080/#/c/21575/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/21575/10/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java@78 PS10, Line 78: ImpalaPlanRel finalOptimizedPlan = : runImpalaConvertProgram(relBuilder, optimizedPlan); > extract runImpalaConvertProgram to a new class such as ImpalaPlanConvertor Hmmm...not sure about this one (and I love extracting things to new classes!) This Optimizer class is pretty small and isolated. If I created a new class here, I would prolly make it a private class and it would still reside in this file. It's an immutable function, so I kinda feel ok about it here. How strongly do you feel about this? -- To view, visit http://gerrit.cloudera.org:8080/21575 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3107d336ac07ecd89530b640165798ec6a574f41 Gerrit-Change-Number: 21575 Gerrit-PatchSet: 11 Gerrit-Owner: Steve Carlin <[email protected]> Gerrit-Reviewer: Aman Sinha <[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: Thu, 10 Oct 2024 15:37:45 +0000 Gerrit-HasComments: Yes
