Alex Behm has posted comments on this change.

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


Patch Set 2:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/3474/2//COMMIT_MSG
Commit Message:

Line 7: IMPALA-3776: fix 'describe formatted' for Avro tables
For both cases below, is our output consistent with the regular describe 
(without formatted)?


Line 11: 'avro.schema.url' file. HIVE-6308 aimed to improve upon this, but for
Instead of 'avro.schema'url' let's just say Avro Schema because there are other 
ways of specifying an Avro Schema (e.g. avro.schema.literal)


Line 21: 2) The avro schema contains a column, which is not present in the
Did this second case also have a bug that was fixed, or did it already work?


Line 25: I don't know how to automatically test this, but I verified this
I think you might be able to test case 1) as follows:
- create an Avro table only with column definitions (no Avro schema)
- alter the table to give it an Avro schema that has fewer columns (use 
avro.schema.litera; for convenience)
- invalidate metadata on the table
- run describe formatted


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

Line 187:     // table.
table which has already reconciled those differences.


Line 189:     
msTable.setPartitionKeys(Column.toFieldSchemas(table.getClusteringColumns()));
Nice solution!


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic640f18acf7a1731f34b22c50ebbb462dfee78bd
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Lars Volker <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Bharath Vissapragada <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Lars Volker <[email protected]>
Gerrit-Reviewer: Marcel Kornacker <[email protected]>
Gerrit-HasComments: Yes

Reply via email to