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.b...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-HasComments: Yes
