Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8400 )

Change subject: IMPALA-4167: Support insert plan hints for CREATE TABLE AS 
SELECT
......................................................................


Patch Set 14:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/8400/14/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java:

http://gerrit.cloudera.org:8080/#/c/8400/14/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java@82
PS14, Line 82:       Preconditions.checkNotNull(queryStmt);
> combine with member assignment:
Done


http://gerrit.cloudera.org:8080/#/c/8400/14/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java:

http://gerrit.cloudera.org:8080/#/c/8400/14/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@1640
PS14, Line 1640:   public void TestCreateTableAsSelectWithHints() throws 
AnalysisException {
> To avoid duplication with AnalyzeStmts#TestInsertHints let's do the followi
I kept 3 tests to cover ok/warning/error case.


http://gerrit.cloudera.org:8080/#/c/8400/14/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/8400/14/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@304
PS14, Line 304:    * Creates an insert, an upsert, and a CTAS statement with 
the given hints
> Upsert isn't tested. Code or comment wrong?
Wrong comment, thanks for spotting it.
This remained from an attempt to merge UPSERT tests to this function, which 
turned out to be non practical, because UPSERT leads to parse failure for 
partition clauses.


http://gerrit.cloudera.org:8080/#/c/8400/14/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@336
PS14, Line 336:   static private List<String> 
ConvertHintsToStringList(List<PlanHint> hints) {
> shorter: hintsToStrings()
Done


http://gerrit.cloudera.org:8080/#/c/8400/14/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
File fe/src/test/java/org/apache/impala/common/FrontendTestBase.java:

http://gerrit.cloudera.org:8080/#/c/8400/14/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java@83
PS14, Line 83:          new String[] { "/* +", "*/" }, // traditional commented 
hint
> tabs
Done


http://gerrit.cloudera.org:8080/#/c/8400/14/testdata/workloads/functional-planner/queries/PlannerTest/ddl.test
File testdata/workloads/functional-planner/queries/PlannerTest/ddl.test:

http://gerrit.cloudera.org:8080/#/c/8400/14/testdata/workloads/functional-planner/queries/PlannerTest/ddl.test@207
PS14, Line 207: # IMPALA-4167: noclustered hint is ignored if the CTAS has 
partitions and sort columns.
> Let's follow a procedure to trim down these tests similar to what we are do
Done



--
To view, visit http://gerrit.cloudera.org:8080/8400
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d74bca999da8ae1bb89427c70841f33e3c56ab0
Gerrit-Change-Number: 8400
Gerrit-PatchSet: 14
Gerrit-Owner: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Gabor Kaszab <[email protected]>
Gerrit-Reviewer: Lars Volker <[email protected]>
Gerrit-Comment-Date: Tue, 20 Feb 2018 16:44:23 +0000
Gerrit-HasComments: Yes

Reply via email to