impala-reviews mailing list archives

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

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

Patch Set 4:

File fe/src/main/cup/sql-parser.cup:
PS3, Line 1167: tbl_def_without_col_defs ::=
> It's important that it is not accepted: we should only accept supported opt
Done in cup. I did not find another way (that does not add a lot of duplication) than moving
KW_CREATE to high level rules. The problem was that the parser cannot differentiate between
the CTAS and CREATE until it reaches the AS SELECT part, but it needs this information to
decide whether to add an opt_plan_hints non-terminal or not if there is no hint after CREATE.
File fe/src/test/java/org/apache/impala/analysis/
PS3, Line 1633: Only
> Did you find a good way? If not, please clean up this code with its own int
No, I did not find a really good way. The duplication could be merged, but the resulting code
would be quite hard to understand, mainly because the format + partitions of a table are given
in inserts, but have to specified in CTAS. This could be done by mapping table names (used
in insert) to sql options (used in CTAS), and substituting these strings in the statements,
but this would ruin the readability of the statements.
PS3, Line 1647:       // HBase tables cannot be tested, as currently Impala cannot create
HBase tables.
> I see. I think it should go somewhere near the top. See my previous comment
PS3, Line 1653: 
> See above.
File testdata/workloads/functional-planner/queries/PlannerTest/ddl.test:
PS4, Line 207: noclustered
> nit: remove the "" or add them in the test below?
PS4, Line 233: +noclustered,shuffle
> nit: spaces
PS4, Line 251: # IMPALA-4167: noclustered hint in CTAS into partitioned HDFS table has no
effect as it
> I think it's better to not mention the default behavior here. The important

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: 4
Gerrit-Owner: Csaba Ringhofer <>
Gerrit-Reviewer: Csaba Ringhofer <>
Gerrit-Reviewer: Gabor Kaszab <>
Gerrit-Reviewer: Lars Volker <>
Gerrit-Comment-Date: Fri, 22 Dec 2017 15:14:05 +0000
Gerrit-HasComments: Yes

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