impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lars Volker (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4167: Support insert plan hints for CREATE TABLE AS SELECT
Date Tue, 14 Nov 2017 21:50:05 GMT
Lars Volker has posted comments on this change. ( )

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

Patch Set 3:

Commit Message:
PS3, Line 9: Adding support for "clustered", "noclustered", "shuffle" and "noshuffle"
How about "This change adds support..."
PS3, Line 12: example:
PS3, Line 19: Sort by partition columns before insert to make the insert more efficient.
Mention where exactly the sort happens (locally vs distributed).
PS3, Line 26: exchenge
File fe/src/main/cup/sql-parser.cup:
PS3, Line 1054:     RESULT = new CreateTableAsSelectStmt(new CreateTableStmt(tbl_def),
nit: Can you wrap the lines at 90 chars?
PS3, Line 1167: tbl_def_without_col_defs ::=
Does that now also allow hints like "CREATE /*+ clustered */ TABLE FOO LIKE PARQUET...?
PS3, Line 1168:   KW_CREATE external_val:external opt_plan_hints:hints KW_TABLE if_not_exists_val:if_not_exists
long line
File fe/src/main/java/org/apache/impala/analysis/
PS3, Line 155:   TableDef(TableName tableName, boolean isExternal, boolean ifNotExists, List<PlanHint>
hints) {
long line
File fe/src/test/java/org/apache/impala/analysis/
PS3, Line 1633: Only
What does "only" mean here?
PS3, Line 1647:       // HBase tables cannot be tested, as currently Impala cannot create
HBase tables.
It's weird that this explanation occurs in the middle of the function. Can you move it to
the top?
PS3, Line 1653: 
Why is there a newline?
PS3, Line 1677:         "select * from functional.alltypes");
Please also add tests that have additional hints in the select clause.
File testdata/workloads/functional-planner/queries/PlannerTest/ddl.test:
PS3, Line 207: # IMPALA-4167: clustered CTAS into partitioned table adds sort node.
Can you add tests for noclustered and for sort by()?

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d74bca999da8ae1bb89427c70841f33e3c56ab0
Gerrit-Change-Number: 8400
Gerrit-PatchSet: 3
Gerrit-Owner: Csaba Ringhofer <>
Gerrit-Reviewer: Gabor Kaszab <>
Gerrit-Reviewer: Lars Volker <>
Gerrit-Comment-Date: Tue, 14 Nov 2017 21:50:05 +0000
Gerrit-HasComments: Yes

  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message