impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-3809: Show Kudu-specific column metadata in DESCRIBE.
Date Sat, 19 Nov 2016 01:18:48 GMT
Alex Behm has posted comments on this change.

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


Patch Set 1:

(7 comments)

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?
Done


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


PS1, Line 142: Strings.join(rawPath_, ".") 
> Sorry, I know it's not related to this change but if it's easy let's get ri
Done


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.
Done


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


Line 280:       compressionCol.setString_val("N/A");
> Default values and desired block size?
Totally missed those! Thanks. Done.


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.
Done


-- 
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: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message