You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Brock Noland <br...@cloudera.com> on 2014/08/02 02:09:30 UTC
Re: Review Request 24081: HIVE-7432: Remove deprecated Avro's Schema.parse
usages
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24081/#review49410
-----------------------------------------------------------
Excellent cleanup!! Thank you so much.
serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java
<https://reviews.apache.org/r/24081/#comment86477>
I don't think this line will log the stack trace. We should instead change the line above to LOG.info(msg + e.getMessage(), e);
However, I think there is a larger issue. In the case of an exception, this method will return null which I don't see the callers expecting. If the exception is a true error condition we should consider throwing the IOException. You might have to wrap it in a RuntimeException.
- Brock Noland
On July 31, 2014, 3:49 a.m., Ashish Singh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24081/
> -----------------------------------------------------------
>
> (Updated July 31, 2014, 3:49 a.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-7432
> https://issues.apache.org/jira/browse/HIVE-7432
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-7432: Remove deprecated Avro's Schema.parse usages
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/io/avro/AvroGenericRecordReader.java 60b43888b957fe315720c4ee5562b9b67a07d0e2
> serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroGenericRecordWritable.java b55474331736ecbdeb5958dad9342e132642d889
> serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java 8c5cf3e87078fd87d0dc9b41d9545486d76903f3
> serde/src/java/org/apache/hadoop/hive/serde2/avro/SchemaResolutionProblem.java 3dceb6384000e255e87df832f6189c80c636531b
> serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java 915f01679183904d0d93b9b8a88dc1a64ac2af78
> serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroDeserializer.java 198bd24dcb1c2552fd45b919ecb39ef7a29ed321
> serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroObjectInspectorGenerator.java 76c1940fb05a0c8c6b74d570d6d788829e17de01
> serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroSerde.java 072225dcc80bfdb84f0a31f67693616393c264df
> serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroSerdeUtils.java 67d557082eec88eefdde76cb1fead6d51f7784a4
> serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroSerializer.java f8161da44312c2ad9b4dd2bab2aa242692a42d5a
> serde/src/test/org/apache/hadoop/hive/serde2/avro/TestGenericAvroRecordWritable.java cf3b16ce65d07c0a714530ef4a26adef7188ea2e
> serde/src/test/org/apache/hadoop/hive/serde2/avro/TestSchemaReEncoder.java 8dd61097433ab0c2b1c3e326978bf06337f815e6
> serde/src/test/org/apache/hadoop/hive/serde2/avro/TestThatEvolvedSchemasActAsWeWant.java 4b8cc98bfc75ea01f25944d7833f00da1b6911f0
>
> Diff: https://reviews.apache.org/r/24081/diff/
>
>
> Testing
> -------
>
> qTests
>
>
> Thanks,
>
> Ashish Singh
>
>
Re: Review Request 24081: HIVE-7432: Remove deprecated Avro's Schema.parse
usages
Posted by Ashish Singh <as...@cloudera.com>.
> On Aug. 2, 2014, 12:09 a.m., Brock Noland wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java, line 212
> > <https://reviews.apache.org/r/24081/diff/2/?file=646682#file646682line212>
> >
> > I don't think this line will log the stack trace. We should instead change the line above to LOG.info(msg + e.getMessage(), e);
> >
> > However, I think there is a larger issue. In the case of an exception, this method will return null which I don't see the callers expecting. If the exception is a true error condition we should consider throwing the IOException. You might have to wrap it in a RuntimeException.
Brock, thanks for reviewing!
Updated the patch based on your comment.
- Ashish
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24081/#review49410
-----------------------------------------------------------
On Aug. 2, 2014, 1:45 a.m., Ashish Singh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24081/
> -----------------------------------------------------------
>
> (Updated Aug. 2, 2014, 1:45 a.m.)
>
>
> Review request for hive.
>
>
> Bugs: HIVE-7432
> https://issues.apache.org/jira/browse/HIVE-7432
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-7432: Remove deprecated Avro's Schema.parse usages
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/io/avro/AvroGenericRecordReader.java 60b43888b957fe315720c4ee5562b9b67a07d0e2
> serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroGenericRecordWritable.java b55474331736ecbdeb5958dad9342e132642d889
> serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java 8c5cf3e87078fd87d0dc9b41d9545486d76903f3
> serde/src/java/org/apache/hadoop/hive/serde2/avro/SchemaResolutionProblem.java 3dceb6384000e255e87df832f6189c80c636531b
> serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java 915f01679183904d0d93b9b8a88dc1a64ac2af78
> serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroDeserializer.java 198bd24dcb1c2552fd45b919ecb39ef7a29ed321
> serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroObjectInspectorGenerator.java 76c1940fb05a0c8c6b74d570d6d788829e17de01
> serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroSerde.java 072225dcc80bfdb84f0a31f67693616393c264df
> serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroSerdeUtils.java 67d557082eec88eefdde76cb1fead6d51f7784a4
> serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroSerializer.java f8161da44312c2ad9b4dd2bab2aa242692a42d5a
> serde/src/test/org/apache/hadoop/hive/serde2/avro/TestGenericAvroRecordWritable.java cf3b16ce65d07c0a714530ef4a26adef7188ea2e
> serde/src/test/org/apache/hadoop/hive/serde2/avro/TestSchemaReEncoder.java 8dd61097433ab0c2b1c3e326978bf06337f815e6
> serde/src/test/org/apache/hadoop/hive/serde2/avro/TestThatEvolvedSchemasActAsWeWant.java 4b8cc98bfc75ea01f25944d7833f00da1b6911f0
>
> Diff: https://reviews.apache.org/r/24081/diff/
>
>
> Testing
> -------
>
> qTests
>
>
> Thanks,
>
> Ashish Singh
>
>