Alex Behm has posted comments on this change. Change subject: IMPALA-3687: Prefer Avro field name during schema reconciliation ......................................................................
Patch Set 9: Code-Review+2 (4 comments) http://gerrit.cloudera.org:8080/#/c/3331/6/fe/src/main/java/com/cloudera/impala/util/AvroSchemaUtils.java File fe/src/main/java/com/cloudera/impala/util/AvroSchemaUtils.java: Line 136: // are taken from the Avro schema. > I agree that we should not allow this things. Agreed that the current change is a bug fix. Based on my understanding and the comments in the code, it was never intended for the field name to be picked up from the column definitions. Only the type. Let's take your patch as is. http://gerrit.cloudera.org:8080/#/c/3331/9/fe/src/main/java/com/cloudera/impala/util/AvroSchemaUtils.java File fe/src/main/java/com/cloudera/impala/util/AvroSchemaUtils.java: Line 145: Preconditions.checkNotNull(null, "reconciledColDef.Analyze should never throw"); reconciledColDef.analyze() should never throw http://gerrit.cloudera.org:8080/#/c/3331/9/testdata/workloads/functional-query/queries/QueryTest/avro-schema-resolution.test File testdata/workloads/functional-query/queries/QueryTest/avro-schema-resolution.test: PS9, Line 78: IMPALA use colon, i.e. IMPALA-3687: Line 128: # IMPALA-3687. Create a table with column definition and later alter its Avro schema to use colon, i.e. IMPALA-3687: ... column definitions ... -- To view, visit http://gerrit.cloudera.org:8080/3331 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia3e43b2885853c2b4f207a45a873c9d7f31379cd Gerrit-PatchSet: 9 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Huaisi Xu <[email protected]> Gerrit-Reviewer: Juan Yu <[email protected]> Gerrit-HasComments: Yes
