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-3809: Show Kudu-specific column metadata in DESCRIBE.
Date Fri, 18 Nov 2016 18:20:20 GMT
Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-3809: Show Kudu-specific column metadata in DESCRIBE.
......................................................................


Patch Set 1:

(7 comments)

Thanks for doing this. Nice! IMPALA-3726 is checked in, so you can augment the tests.

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

PS1, Line 44: (column name, data type, and
            :  * comment)
Update comment for Kudu tables?


PS1, Line 129: if (path_.destTable() != null) {
             :       // Describing a table.
             :       return;
             :     } else
if (path_.destTable() != null) return;


PS1, Line 142: Strings.join(rawPath_, ".") 
Sorry, I know it's not related to this change but if it's easy let's get rid of this parquet.Strings
class and use the Joiner instead.


http://gerrit.cloudera.org:8080/#/c/5125/1/fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java
File fe/src/main/java/org/apache/impala/service/DescribeResultFactory.java:

PS1, Line 269: kuduColumn.getComment() != null ? kuduColumn.getComment() : ""
I believe Guava has a Strings.nullToEmpty() that could be used here.


PS1, Line 273: kuduTable.isPrimaryKeyColumn(kuduColumn.getName()
You can use kuduColumn.isKey() instead


Line 280:       compressionCol.setString_val("N/A");
Default values and desired block size?


http://gerrit.cloudera.org:8080/#/c/5125/1/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

PS1, Line 321: columns.add(new TColumn("primary_key", Type.STRING.toThrift()));
             :         columns.add(new TColumn("nullable", Type.STRING.toThrift()));
             :         columns.add(new TColumn("encoding", Type.STRING.toThrift()));
             :         columns.add(new TColumn("compression", Type.STRING.toThrift()));
We also need default value and desired block size.


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

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

Mime
View raw message