Bharath Vissapragada has posted comments on this change.

Change subject: IMPALA-3776: fix 'describe formatted' for Avro tables
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3466/1/fe/src/main/java/com/cloudera/impala/catalog/Table.java
File fe/src/main/java/com/cloudera/impala/catalog/Table.java:

PS1, Line 392:  ArrayList<Column> columns = 
Lists.newArrayList(getNonClusteringColumns());
             : 
             :     for (Column column: colsByPos_.subList(0, 
numClusteringCols_)) {
             :       columns.add(column);
             :     }
             :     return columns;
nit: This can be refactored to use getClusteringColumns()


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

PS1, Line 185: // For some table formats (e.g. Avro) the column list in the 
table can differ from the
             :     // one returned by the Hive metastore. To handle this we use 
the column list from the
             :     // table.
I think we should confirm this behavior change with Marcel/Alex because this 
method now shows the "impala reconciled" schema (in case of Avro tables) in the 
describe formatted output. Earlier it just showed whatever is in the HMS 
coldefs. Reason I'm saying this is the method javadoc mentions "For the 
formatted describe output the goal is to be exactly the same as what Hive (via 
HiveServer2) outputs, for compatibility reasons."


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If7137b2ef163368b467898d40718f79911c3cf82
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-2.6.0_5.8.0
Gerrit-Owner: Lars Volker <[email protected]>
Gerrit-Reviewer: Bharath Vissapragada <[email protected]>
Gerrit-Reviewer: Lars Volker <[email protected]>
Gerrit-HasComments: Yes

Reply via email to