impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dimitris Tsirogiannis (Code Review)" <>
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:


Thanks for doing this. Nice! IMPALA-3726 is checked in, so you can augment the tests.
File fe/src/main/java/org/apache/impala/analysis/

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.
File fe/src/main/java/org/apache/impala/service/

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?
File fe/src/main/java/org/apache/impala/service/

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
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I83c91b540bc6d27cb4f21535fe12f3f8658c233e
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <>
Gerrit-Reviewer: Dimitris Tsirogiannis <>
Gerrit-HasComments: Yes

View raw message