Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/22870 )
Change subject: IMPALA-14061: Calcite Planner: added Calcite rules ...................................................................... Patch Set 10: (1 comment) @iwanttobepowerful As I mentioned in the one comment, 22990 is a test patch containing many changes, including all my performance issues. You mentioned in a comment that the varchar/string issue is fixed in 22990. I know I made a change in that patch which addresses something to do with varchar and string, so perhaps it is fixed in that patch. But if you still have that patch compiled, there is one thing special about that patch, so I would need you to check the log file to see if it is indeed fixed... That patch is specifically testing a "fallback" mechanism. If a query fails for any reason at compilation time, it will fallback to use the original compiler. So if you are seeing that query succeed, it either means that a) the query was fixed, or b) the query threw an exception in the Calcite compiler, but succeeded using the original compiler. The only way to tell is by looking at the log file. So would you be able to check that? Thanks! http://gerrit.cloudera.org:8080/#/c/22870/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/22870/10/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java@200 PS10, Line 200: private RelNode runJoinProgram(RelNode plan) throws ImpalaException { : : HepProgramBuilder builder = new HepProgramBuilder(); : : builder.addMatchOrder(HepMatchOrder.BOTTOM_UP); : : builder.addRuleCollection(ImmutableList.of( : CoreRules.SEMI_JOIN_PROJECT_TRANSPOSE, : CoreRules.JOIN_PROJECT_BOTH_TRANSPOSE_INCLUDE_OUTER, : CoreRules.PROJECT_MERGE : )); : : // has to be in a separate program or else there is an infinite loop : builder.addRuleInstance(CoreRules.JOIN_PUSH_TRANSITIVE_PREDICATES); : : // Merge the filter nodes into the Join. Also include : // The filter/project transpose in case the Filter : // exists above the Project in the RelNode so it can : // then be merged into the Join. The idea is to place : // joins next to each other if possible for the join : // optimization step. : builder.addRuleCollection(ImmutableList.of( : CoreRules.FILTER_INTO_JOIN, : CoreRules.FILTER_PROJECT_TRANSPOSE : )); : : // Join rules work in a two step process. The first step : // is to merge all adjacent joins into one big "multijoin" : // RelNode (the JOIN_TO_MULTIJOIN rule). Then the : // MULTI_JOIN_OPTIMIZE rule is used to determine the join : // ordering. : builder.addMatchOrder(HepMatchOrder.BOTTOM_UP); : builder.addRuleInstance(CoreRules.JOIN_CONDITION_PUSH); : builder.addRuleInstance(CoreRules.JOIN_TO_MULTI_JOIN); : builder.addRuleInstance(CoreRules.MULTI_JOIN_OPTIMIZE); : : return runProgram(plan, builder.build()); : } > ok! thanks!! That's interesting about the plan...but I would imagine it's more freak luck at this point. Without statistics, the join ordering can be kind of random. I saw in a different comment that you tried patch 22990? Can you take a peek and see if the performance is better there? That patch has most of my pre-commit changes that is not quite ready for code review, but testing how many failures I would see on Jenkins -- To view, visit http://gerrit.cloudera.org:8080/22870 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6671f7ed298a18965ef0b7a5fc10f4912333a52b Gerrit-Change-Number: 22870 Gerrit-PatchSet: 10 Gerrit-Owner: Steve Carlin <scar...@cloudera.com> Gerrit-Reviewer: Aman Sinha <amsi...@cloudera.com> Gerrit-Reviewer: Anonymous Coward (816) Gerrit-Reviewer: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Steve Carlin <scar...@cloudera.com> Gerrit-Comment-Date: Thu, 19 Jun 2025 14:21:01 +0000 Gerrit-HasComments: Yes