Alex Behm has posted comments on this change.

Change subject: Refactor CREATE TABLE grammar rules in prep for PRIMARY KEY
......................................................................


Patch Set 7:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/2865/7/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:

Line 400: nonterminal ArrayList<ColumnDef> column_def_list, 
partitioned_column_defs,
partition_col_defs


Line 937:     // "distributed_data_arrangement" cannot be optional because it 
would conflict with
update comment (production rule was renamed)


Line 962:     // If "data_arrangement_clause" were used instead so that this 
rule could be combined
update comment (production rule was renamed)


http://gerrit.cloudera.org:8080/#/c/2865/7/fe/src/main/java/com/cloudera/impala/analysis/TableDef.java
File fe/src/main/java/com/cloudera/impala/analysis/TableDef.java:

Line 48:   private final TableDataLayout dataArrangement_;
dataLayout_

rename arrangement to layout everywhere


Line 101:     for (ColumnDef colDef: getPartitionColumnDefs()) {
Not your change and not for this patch, but should we not check the fileformat 
here? Partitioning does not make sense for Kudu or HBase tables.

We should also consider moving the analysis of distributed by in here

Feel free to defer to a different patch


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9a2b9e380a0b90c0e2e6f10f6905cab5164cb3c4
Gerrit-PatchSet: 7
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Casey Ching <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Casey Ching <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-HasComments: Yes

Reply via email to