Lars Volker has posted comments on this change. Change subject: IMPALA-4163: Add sortby() query hint ......................................................................
Patch Set 4: (7 comments) Thanks for the review. Please see PS5. http://gerrit.cloudera.org:8080/#/c/5051/4/fe/src/main/java/org/apache/impala/analysis/SelectList.java File fe/src/main/java/org/apache/impala/analysis/SelectList.java: Line 74: if (planHints != null) planHints_ = planHints; > assert != null They could actually be null, since the parser just forwards its results to here. I corrected this in the parser and added the assertions. http://gerrit.cloudera.org:8080/#/c/5051/4/fe/src/main/java/org/apache/impala/analysis/TableRef.java File fe/src/main/java/org/apache/impala/analysis/TableRef.java: Line 274: if (hints != null) this.joinHints_ = hints; > assert that hints != null Done Line 278: if (hints != null) this.tableHints_ = hints; > same Done http://gerrit.cloudera.org:8080/#/c/5051/4/fe/src/main/java/org/apache/impala/planner/Planner.java File fe/src/main/java/org/apache/impala/planner/Planner.java: Line 513: * contain the clustering columns (key columns for Kudu tables), so that partitions can > they won't just "contain" (in an unspecified position), the ordering column Done http://gerrit.cloudera.org:8080/#/c/5051/4/fe/src/main/jflex/sql-scanner.flex File fe/src/main/jflex/sql-scanner.flex: PS4, Line 326: [ ]* > also change to " "* (and wherever else it applies) Done Line 349: %state HINT > rename to EOLHINT then Done Line 447: {CommentedHintEnd} { > move below the corresponding 'begin' rule Done -- To view, visit http://gerrit.cloudera.org:8080/5051 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I37a3ffab99aaa5d5a4fd1ac674b3e8b394a3c4c0 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-HasComments: Yes
