Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/16723 )
Change subject: IMPALA-10314: Optimize planning time for simple limits ...................................................................... Patch Set 7: (4 comments) http://gerrit.cloudera.org:8080/#/c/16723/5/fe/src/main/cup/sql-parser.cup File fe/src/main/cup/sql-parser.cup: http://gerrit.cloudera.org:8080/#/c/16723/5/fe/src/main/cup/sql-parser.cup@3115 PS5, Line 3115: // clause. An attempt was made to set this for individual exprs > I guess this is a bit limiting in the it applies only to the whole where cl That was my original attempt too..setting the hint at the predicate level but when I was running into shift/reduce conflicts. I gave it another try today with a few variations and still could not get it to work. I have left a comment in the code about this. For reference, here's one change I tried: expr ::= non_pred_expr:e {: RESULT = e; :} | opt_plan_hints:pred_hints predicate:p {: p.setPredicateHints(pred_hints); RESULT = p; :}; This generated quite a few conflicts..example: Warning : *** Shift/Reduce conflict found in state #253 between opt_plan_hints ::= (*) and case_expr ::= (*) KW_CASE expr case_when_clause_list case_else_clause KW_END and case_expr ::= (*) KW_CASE case_when_clause_list case_else_clause KW_END under symbol KW_CASE Resolved in favor of shifting. Warning : *** Shift/Reduce conflict found in state #253 between opt_plan_hints ::= (*) and cast_expr ::= (*) KW_CAST LPAREN expr KW_AS type_def cast_format_val RPAREN under symbol KW_CAST Resolved in favor of shifting. .. ... http://gerrit.cloudera.org:8080/#/c/16723/5/fe/src/main/java/org/apache/impala/analysis/Predicate.java File fe/src/main/java/org/apache/impala/analysis/Predicate.java: http://gerrit.cloudera.org:8080/#/c/16723/5/fe/src/main/java/org/apache/impala/analysis/Predicate.java@30 PS5, Line 30: { > maybe hasAlwaysTrueHint_ just to make it crystal-clear that it's not actual Changed this and the names of the setter/getter also. http://gerrit.cloudera.org:8080/#/c/16723/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java: http://gerrit.cloudera.org:8080/#/c/16723/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@869 PS5, Line 869: if ((fsHasBlocks && fd.getNumFileBlocks() == 0) > nit: use braces for multi-line if Done http://gerrit.cloudera.org:8080/#/c/16723/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@870 PS5, Line 870: d.getFileLength() < > We already had to deal with a similar issue here I will follow up with the doc writer on this. -- To view, visit http://gerrit.cloudera.org:8080/16723 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9d6a79263bc092e0f3e9a1d72da5618f3cc35574 Gerrit-Change-Number: 16723 Gerrit-PatchSet: 7 Gerrit-Owner: Aman Sinha <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Qifan Chen <[email protected]> Gerrit-Reviewer: Shant Hovsepian <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Fri, 20 Nov 2020 18:47:34 +0000 Gerrit-HasComments: Yes
