impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dimitris Tsirogiannis (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4561: Replace DISTRIBUTE BY with PARTITION BY in CREATE TABLE
Date Fri, 02 Dec 2016 22:19:24 GMT
Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-4561: Replace DISTRIBUTE BY with PARTITION BY in CREATE TABLE
......................................................................


Patch Set 1:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/5317/1/common/thrift/CatalogObjects.thrift
File common/thrift/CatalogObjects.thrift:

Line 380: struct TPartitionParam {
> TKuduPartitionParam?
Done


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

Line 404: nonterminal TableDataLayout opt_tbl_data_layout, distributed_data_layout;
> also change distributed_data_layout
Done


Line 411: nonterminal PartitionParam partition_hash_param;
> KuduPartitionParam?
Done


Line 1084:     tbl_def.getPartitionColumnDefs().addAll(data_layout.getPartitionColumnDefs());
> regarding my rename suggestion: I'd imagine these two lines here are rather
Done


http://gerrit.cloudera.org:8080/#/c/5317/1/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:

Line 88:   public List<ColumnDef> getPartitionColumnDefs() {
> these two getters are also confusing
Done


http://gerrit.cloudera.org:8080/#/c/5317/1/fe/src/main/java/org/apache/impala/analysis/RangePartition.java
File fe/src/main/java/org/apache/impala/analysis/RangePartition.java:

Line 113:   public void analyze(Analyzer analyzer, List<ColumnDef> partitioningColDefs)
> partColDefs?
Done


http://gerrit.cloudera.org:8080/#/c/5317/1/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java
File fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java:

Line 25:  * Represents the PARTITION BY and PARTITIONED BY clauses of a DDL statement.
> Fingers crossed.
:)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0e07c41eabb4c8cb95754cf04293cbd9e03d6ab2
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message