Kim Jin Chul 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 8: (4 comments) http://gerrit.cloudera.org:8080/#/c/8676/8/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/8/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@303 PS8, Line 303: String pattern > Can you add a brief comment or example of what this pattern should look lik Done http://gerrit.cloudera.org:8080/#/c/8676/8/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@319 PS8, Line 319: TestInsertStmtHints > nit: "This function" Done http://gerrit.cloudera.org:8080/#/c/8676/8/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@330 PS8, Line 330: ParserErrorOnInsertStmtHints > nit: "It covers..." No need to repeat the function name in the comment. Done http://gerrit.cloudera.org:8080/#/c/8676/8/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java File fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java: http://gerrit.cloudera.org:8080/#/c/8676/8/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java@505 PS8, Line 505: private String InjectInsertHint(String pattern, String hint, : InsertStmt.HintLocation loc) { : final String oracleHint = (loc == InsertStmt.HintLocation.Start) ? hint : ""; : final String defaultHint = (loc == InsertStmt.HintLocation.End) ? hint : ""; : return String.format(pattern, oracleHint, defaultHint); : } > Isn't the same function as in parserTest.java? Plz avoid code duplication. It is moved to the super class(i.e. FrontendTestBase). -- 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: 8 Gerrit-Owner: Kim Jin Chul <jinc...@gmail.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: John Russell <jruss...@cloudera.com> Gerrit-Reviewer: Kim Jin Chul <jinc...@gmail.com> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Fri, 05 Jan 2018 01:32:56 +0000 Gerrit-HasComments: Yes
