Marcel Kornacker has posted comments on this change. Change subject: IMPALA-4163: Add sortby() query hint ......................................................................
Patch Set 4: (8 comments) 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 as it is, a null param will simply leave planHints_ unchanged, which is counterintuitive. 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 Line 278: if (hints != null) this.tableHints_ = hints; same 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 columns will be the clustering columns, followed the sort-by cols. http://gerrit.cloudera.org:8080/#/c/5051/3/fe/src/main/jflex/sql-scanner.flex File fe/src/main/jflex/sql-scanner.flex: Line 331: HintContent = [ ]* "+" [^\r\n]* > Yes, they are the same. I went with [ ]* because it was like that before. S yes, please do, [ ]* feels a bit indirect 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) Line 349: %state HINT rename to EOLHINT then Line 447: {CommentedHintEnd} { move below the corresponding 'begin' rule -- 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
