Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24257 )
Change subject: IMPALA-14940: Calcite planner: handle broadcast and shuffle hints. ...................................................................... Patch Set 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/24257/7/java/calcite-planner/src/main/codegen/templates/Parser.jj File java/calcite-planner/src/main/codegen/templates/Parser.jj: http://gerrit.cloudera.org:8080/#/c/24257/7/java/calcite-planner/src/main/codegen/templates/Parser.jj@1973 PS7, Line 1973: <HINT_BEG> AddHint(hints) ( <COMMA> AddHint(hints) )* <COMMENT_END> > Agreed, let's defer it - and the pooling costs less than I assumed. In the I'm not sure what you mean by "supports table-ref hints natively". I looked through the AST and I see the table is represented by an SqlJoin with an SqlIdentifier for the table. So the information is lost before it gets pushed down to the scan? But it did give me an idea (and maybe you had this in mind too?) that the SqlHint can be more than just the type. We can include some join information on the SqlHint (and extend the class) and this can then be propagated at logical RelNode time? Regardless, I filed a Jira to be handled at a later date: IMPALA-15251 http://gerrit.cloudera.org:8080/#/c/24257/7/java/calcite-planner/src/main/codegen/templates/Parser.jj@2094 PS7, Line 2094: String joinTypeString = (String) SqlLiteral.value(joinType); > Sorry for this. Yeah, this was just a bad miss on my part. Thought I had it working for the positive and negative tests. And also missed your extra test. I applied the code you suggested and it seems to work fine, so thanks tons! I sometimes don't add tests to Calcite...I'm sometimes random about this. The ultimate goal is to have the full suite of e2e tests do this. So sometimes I avoid putting tests in calcite.test because I feel it will eventually be redundant testing. But I did wind up adding it this time, I think it's worth it, so thanks again! -- To view, visit http://gerrit.cloudera.org:8080/24257 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie5a932cc09d3d370aa627228d0e226a92fa03168 Gerrit-Change-Number: 24257 Gerrit-PatchSet: 7 Gerrit-Owner: Steve Carlin <[email protected]> Gerrit-Reviewer: Aleksandr Efimov <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Reviewer: Xuebin Su <[email protected]> Gerrit-Comment-Date: Fri, 07 Aug 2026 14:26:47 +0000 Gerrit-HasComments: Yes
