Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/8676 )
Change subject: IMPALA-4168: Adds Oracle-style hint placement for INSERT/UPSERT ...................................................................... Patch Set 6: (3 comments) several more follow-up comments and we should be good. thx! http://gerrit.cloudera.org:8080/#/c/8676/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8676/5//COMMIT_MSG@12 PS5, Line 12: rebuild omit "query rebuild" http://gerrit.cloudera.org:8080/#/c/8676/5/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java File fe/src/main/java/org/apache/impala/analysis/InsertStmt.java: http://gerrit.cloudera.org:8080/#/c/8676/5/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@56 PS5, Line 56: AtStatement argh... I was inconsistent with my suggestion and see it here now. I have Start/End vs. AtStatement/AtQuery. I have a slight preference for the latter since its more precise. I'll let you decide that one. But it should be consistent. http://gerrit.cloudera.org:8080/#/c/8676/5/fe/src/test/java/org/apache/impala/analysis/ParserTest.java File fe/src/test/java/org/apache/impala/analysis/ParserTest.java: http://gerrit.cloudera.org:8080/#/c/8676/5/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@310 PS5, Line 310: actualPlanHints, String... expectedHints) { : List<String> actualHints = Lists.newArrayList(); : for (PlanHint hint: actualPlanHints) actualHints there's some shadowing here (actualHints is a parameter and local variable)-- does it do what you intend? perhaps to make it clearer, call the local one "stringHints". -- To view, visit http://gerrit.cloudera.org:8080/8676 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ied7629d70197a0270cdc0853e00cc021fdb4dc20 Gerrit-Change-Number: 8676 Gerrit-PatchSet: 6 Gerrit-Owner: Kim Jin Chul <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: John Russell <[email protected]> Gerrit-Reviewer: Kim Jin Chul <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Thu, 14 Dec 2017 01:33:06 +0000 Gerrit-HasComments: Yes
