Tim Armstrong has posted comments on this change. Change subject: IMPALA-2057: Better error message for incorrect avro decimal column declaration ......................................................................
Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5255/1/fe/src/main/java/org/apache/impala/util/AvroSchemaParser.java File fe/src/main/java/org/apache/impala/util/AvroSchemaParser.java: Line 131: String logicalType = schema.getProp("logicalType"); The control-flow here is unnecessarily convoluted (it was before your patch with returning null and the fall-through of the case). I think we should clean it up instead of adding to the complexity. It would make more sense to: 1. check if a logical type is present, and throw an exception if it's non-NULL (since BYTES requires a logical type) 2. call getDecimalType(schema, logicalType) 3. getDecimalType() should now either return a valid type or throw an exception. -- To view, visit http://gerrit.cloudera.org:8080/5255 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iad23706128223b6537d565471ef5d8faa91b0b5a Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: anujphadke <apha...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes
