impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Marcel Kornacker (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4163: Add sortby() query hint
Date Fri, 16 Dec 2016 23:34:57 GMT
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 <lv@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message