Abhishek Rawat has posted comments on this change. ( http://gerrit.cloudera.org:8080/13753 )
Change subject: IMPALA-8673: Add query option to force plan hints for insert queries ...................................................................... Patch Set 2: (3 comments) Thanks for reviewing the code. Please review the latest patch. http://gerrit.cloudera.org:8080/#/c/13753/2/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/13753/2/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@854 PS2, Line 854: planHints_.add(new PlanHint(hint)); > nit: planHints_.add(new PlanHint(hint.trim())); Done http://gerrit.cloudera.org:8080/#/c/13753/2/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@842 PS2, Line 842: if (planHints_.isEmpty() && : ((table_ instanceof FeHBaseTable) || : !(analyzer.getQueryOptions().isSetDefault_hints_insert_statement()))) return; : : // Set up the plan hints from query option DEFAULT_HINTS_INSERT_STATEMENT. : // Default hint is ignored, if the original statement had hints. : if (planHints_.isEmpty() && : analyzer.getQueryOptions().isSetDefault_hints_insert_statement()) { : String defaultHints = : analyzer.getQueryOptions().getDefault_hints_insert_statement(); : for (String hint: defaultHints.trim().split(":")) { : hint = hint.trim(); : planHints_.add(new PlanHint(hint)); : } : } : : if (table_ instanceof FeHBaseTable) { : throw new AnalysisException(String.format("INSERT hints are only supported for " + : "inserting into Hdfs and Kudu tables: %s", getTargetTableName())); : } > I think this can be formatted to be more readable. The checks on the hbase Done http://gerrit.cloudera.org:8080/#/c/13753/2/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java: http://gerrit.cloudera.org:8080/#/c/13753/2/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@2038 PS2, Line 2038: AnalyzesOk(String.format("insert into table functional_kudu.alltypes " + > Mind adding some checks that Very good point. I have added the new checks. -- To view, visit http://gerrit.cloudera.org:8080/13753 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1c3f213402b8e4d1940f96738ad21edf800fa43a Gerrit-Change-Number: 13753 Gerrit-PatchSet: 2 Gerrit-Owner: Abhishek Rawat <ara...@cloudera.com> Gerrit-Reviewer: Abhishek Rawat <ara...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Tue, 02 Jul 2019 19:52:50 +0000 Gerrit-HasComments: Yes
