Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48481 --- Ship it! All of these are nitpicks. Feel free to ignore. serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java https://reviews.apache.org/r/23387/#comment85120 At this point columnNameProperty can't be empty because it'll have been checked by the previous if statement already. So this can be simplified/unwrapped to just the else branch serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment85121 nit: these two variables can be List instead of ArrayList serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment85122 This should be replaced with org.junit.Assert. The junit one is obsolete. serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment85123 LOGGER and all of these are non static and thus should be either lowercased/camelCase or static. typeInfoToSchema shouldn't be static I think. - Lars Francke On July 22, 2014, 5:13 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 22, 2014, 5:13 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java 4564e75d9bfc73f8e10f160e2535f1a08b90ff79 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION serde/src/test/resources/avro-nested-struct.avsc PRE-CREATION serde/src/test/resources/avro-struct.avsc PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
On July 18, 2014, 1:57 p.m., Tom White wrote: serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java, line 112 https://reviews.apache.org/r/23387/diff/9/?file=634614#file634614line112 Is it possible to default name to table name, namespace to database name, and doc to table comment? Ashish Singh wrote: I was planning to do this, but slipped off my mind. Thanks for pointing this out. I don't think it is possible to retrieve database name inside serde. Addressed name and doc. Tom White wrote: Thanks for fixing this. There's no test that name and comment are picked up from the table definition - perhaps you could add one, or at least confirm it manually. I couldn't see where in Hive they get set... Otherwise, +1 from me - thanks for addressing all my comments. This is a great feature to add. Ashish Singh wrote: Tom, actually its tested by all the unit tests now. Look at the diffs in https://reviews.apache.org/r/23387/diff/8-9/#index_header. Tom White wrote: Unless I am missing something, the unit tests in TestTypeInfoToSchema don't test this since they hardcode the table name to avrotest and the table comment to This is to test hive-avro. Perhaps this is tested indirectly through the ql tests since Avro schema resolution rules mean that a record schema's name must match for both the reader and writer. However, this isn't true for comments (Avro schema doc), and it would be good to confirm that inserting data into an Avro-backed Hive table creates Avro files with the expected top-level name and comment. Ashish Singh wrote: Tom, my bad. I thought we are talking about having hive typeinfo in doc for corresponding avro schema. I did verify that top-level name and comment are being created as expected before posting the patch here. I log created avro schema in AvroSerde.java and that came handy to verify this. OK, thanks for clarifying. +1 from me. - Tom --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48120 --- On July 22, 2014, 5:13 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 22, 2014, 5:13 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java 4564e75d9bfc73f8e10f160e2535f1a08b90ff79 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION serde/src/test/resources/avro-nested-struct.avsc PRE-CREATION serde/src/test/resources/avro-struct.avsc PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48541 --- - Ashish Singh On July 23, 2014, 6:38 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 23, 2014, 6:38 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java 4564e75d9bfc73f8e10f160e2535f1a08b90ff79 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION serde/src/test/resources/avro-nested-struct.avsc PRE-CREATION serde/src/test/resources/avro-struct.avsc PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 23, 2014, 6:38 p.m.) Review request for hive. Changes --- Addressed final review comments. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs (updated) - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java 4564e75d9bfc73f8e10f160e2535f1a08b90ff79 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION serde/src/test/resources/avro-nested-struct.avsc PRE-CREATION serde/src/test/resources/avro-struct.avsc PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
On July 23, 2014, 7:26 a.m., Lars Francke wrote: serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java, line 89 https://reviews.apache.org/r/23387/diff/9-11/?file=634614#file634614line89 At this point columnNameProperty can't be empty because it'll have been checked by the previous if statement already. So this can be simplified/unwrapped to just the else branch Great catch! - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48481 --- On July 23, 2014, 6:38 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 23, 2014, 6:38 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java 4564e75d9bfc73f8e10f160e2535f1a08b90ff79 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION serde/src/test/resources/avro-nested-struct.avsc PRE-CREATION serde/src/test/resources/avro-struct.avsc PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
On July 23, 2014, 4:29 a.m., Brock Noland wrote: Looks great! Can you fix my nits and attach the patch to jira? Done. - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48474 --- On July 23, 2014, 6:38 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 23, 2014, 6:38 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java 4564e75d9bfc73f8e10f160e2535f1a08b90ff79 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION serde/src/test/resources/avro-nested-struct.avsc PRE-CREATION serde/src/test/resources/avro-struct.avsc PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48546 --- Ship it! Ship It! - Lars Francke On July 23, 2014, 6:38 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 23, 2014, 6:38 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java 4564e75d9bfc73f8e10f160e2535f1a08b90ff79 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION serde/src/test/resources/avro-nested-struct.avsc PRE-CREATION serde/src/test/resources/avro-struct.avsc PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48563 --- Ship it! Ship It! - David Chen On July 23, 2014, 6:38 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 23, 2014, 6:38 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java 4564e75d9bfc73f8e10f160e2535f1a08b90ff79 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION serde/src/test/resources/avro-nested-struct.avsc PRE-CREATION serde/src/test/resources/avro-struct.avsc PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
On July 18, 2014, 1:57 p.m., Tom White wrote: serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java, line 112 https://reviews.apache.org/r/23387/diff/9/?file=634614#file634614line112 Is it possible to default name to table name, namespace to database name, and doc to table comment? Ashish Singh wrote: I was planning to do this, but slipped off my mind. Thanks for pointing this out. I don't think it is possible to retrieve database name inside serde. Addressed name and doc. Tom White wrote: Thanks for fixing this. There's no test that name and comment are picked up from the table definition - perhaps you could add one, or at least confirm it manually. I couldn't see where in Hive they get set... Otherwise, +1 from me - thanks for addressing all my comments. This is a great feature to add. Ashish Singh wrote: Tom, actually its tested by all the unit tests now. Look at the diffs in https://reviews.apache.org/r/23387/diff/8-9/#index_header. Unless I am missing something, the unit tests in TestTypeInfoToSchema don't test this since they hardcode the table name to avrotest and the table comment to This is to test hive-avro. Perhaps this is tested indirectly through the ql tests since Avro schema resolution rules mean that a record schema's name must match for both the reader and writer. However, this isn't true for comments (Avro schema doc), and it would be good to confirm that inserting data into an Avro-backed Hive table creates Avro files with the expected top-level name and comment. - Tom --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48120 --- On July 19, 2014, 5:11 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 19, 2014, 5:11 a.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java 4564e75d9bfc73f8e10f160e2535f1a08b90ff79 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
On July 18, 2014, 1:57 p.m., Tom White wrote: serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java, line 112 https://reviews.apache.org/r/23387/diff/9/?file=634614#file634614line112 Is it possible to default name to table name, namespace to database name, and doc to table comment? Ashish Singh wrote: I was planning to do this, but slipped off my mind. Thanks for pointing this out. I don't think it is possible to retrieve database name inside serde. Addressed name and doc. Tom White wrote: Thanks for fixing this. There's no test that name and comment are picked up from the table definition - perhaps you could add one, or at least confirm it manually. I couldn't see where in Hive they get set... Otherwise, +1 from me - thanks for addressing all my comments. This is a great feature to add. Ashish Singh wrote: Tom, actually its tested by all the unit tests now. Look at the diffs in https://reviews.apache.org/r/23387/diff/8-9/#index_header. Tom White wrote: Unless I am missing something, the unit tests in TestTypeInfoToSchema don't test this since they hardcode the table name to avrotest and the table comment to This is to test hive-avro. Perhaps this is tested indirectly through the ql tests since Avro schema resolution rules mean that a record schema's name must match for both the reader and writer. However, this isn't true for comments (Avro schema doc), and it would be good to confirm that inserting data into an Avro-backed Hive table creates Avro files with the expected top-level name and comment. Tom, my bad. I thought we are talking about having hive typeinfo in doc for corresponding avro schema. I did verify that top-level name and comment are being created as expected before posting the patch here. I log created avro schema in AvroSerde.java and that came handy to verify this. - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48120 --- On July 19, 2014, 5:11 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 19, 2014, 5:11 a.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java 4564e75d9bfc73f8e10f160e2535f1a08b90ff79 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
On July 19, 2014, 12:43 a.m., David Chen wrote: serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java, line 294 https://reviews.apache.org/r/23387/diff/9/?file=634616#file634616line294 It would improve maintainability to keep the test schemas in separate .avsc files under serde/src/test/resources rather than inline in the file. You can use Guava's Resources class to get the file and construct the schema. For example: Schema expectedSchema = new Schema.Parser().parse( Resources.getResource(record1.avsc).openStream()); Ashish Singh wrote: David, as the tests have a lot common in their schema I am using a method to generate the common schema part and each test only provides a part of schema that is specific to the test. This made my tests have much less LOC. If I create a .avsc file for each test it will much more cumbersome for both maintaining and adding new tests. David Chen wrote: Hi Ashish, sorry I was a bit unclear. The tests for the individual data types are fine. I thought that moving just the two large schemas into their own files may make them easier to maintain since keeping them inline requires a large number of escape characters. Davis, its done now. - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48168 --- On July 22, 2014, 5:13 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 22, 2014, 5:13 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java 4564e75d9bfc73f8e10f160e2535f1a08b90ff79 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION serde/src/test/resources/avro-nested-struct.avsc PRE-CREATION serde/src/test/resources/avro-struct.avsc PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48474 --- Looks great! Can you fix my nits and attach the patch to jira? serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java https://reviews.apache.org/r/23387/#comment85101 These can be static? serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment85102 This is not a constant and thus should be camel caps - Brock Noland On July 22, 2014, 5:13 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 22, 2014, 5:13 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java 4564e75d9bfc73f8e10f160e2535f1a08b90ff79 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION serde/src/test/resources/avro-nested-struct.avsc PRE-CREATION serde/src/test/resources/avro-struct.avsc PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48475 --- Ship it! Ship It! - Brock Noland On July 22, 2014, 5:13 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 22, 2014, 5:13 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java 4564e75d9bfc73f8e10f160e2535f1a08b90ff79 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION serde/src/test/resources/avro-nested-struct.avsc PRE-CREATION serde/src/test/resources/avro-struct.avsc PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
On July 18, 2014, 1:57 p.m., Tom White wrote: serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java, line 112 https://reviews.apache.org/r/23387/diff/9/?file=634614#file634614line112 Is it possible to default name to table name, namespace to database name, and doc to table comment? Ashish Singh wrote: I was planning to do this, but slipped off my mind. Thanks for pointing this out. I don't think it is possible to retrieve database name inside serde. Addressed name and doc. Thanks for fixing this. There's no test that name and comment are picked up from the table definition - perhaps you could add one, or at least confirm it manually. I couldn't see where in Hive they get set... Otherwise, +1 from me - thanks for addressing all my comments. This is a great feature to add. - Tom --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48120 --- On July 19, 2014, 5:11 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 19, 2014, 5:11 a.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java 4564e75d9bfc73f8e10f160e2535f1a08b90ff79 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
On July 18, 2014, 1:57 p.m., Tom White wrote: serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java, line 112 https://reviews.apache.org/r/23387/diff/9/?file=634614#file634614line112 Is it possible to default name to table name, namespace to database name, and doc to table comment? Ashish Singh wrote: I was planning to do this, but slipped off my mind. Thanks for pointing this out. I don't think it is possible to retrieve database name inside serde. Addressed name and doc. Tom White wrote: Thanks for fixing this. There's no test that name and comment are picked up from the table definition - perhaps you could add one, or at least confirm it manually. I couldn't see where in Hive they get set... Otherwise, +1 from me - thanks for addressing all my comments. This is a great feature to add. Tom, actually its tested by all the unit tests now. Look at the diffs in https://reviews.apache.org/r/23387/diff/8-9/#index_header. - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48120 --- On July 19, 2014, 5:11 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 19, 2014, 5:11 a.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java 4564e75d9bfc73f8e10f160e2535f1a08b90ff79 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
On July 19, 2014, 12:43 a.m., David Chen wrote: serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java, line 294 https://reviews.apache.org/r/23387/diff/9/?file=634616#file634616line294 It would improve maintainability to keep the test schemas in separate .avsc files under serde/src/test/resources rather than inline in the file. You can use Guava's Resources class to get the file and construct the schema. For example: Schema expectedSchema = new Schema.Parser().parse( Resources.getResource(record1.avsc).openStream()); Ashish Singh wrote: David, as the tests have a lot common in their schema I am using a method to generate the common schema part and each test only provides a part of schema that is specific to the test. This made my tests have much less LOC. If I create a .avsc file for each test it will much more cumbersome for both maintaining and adding new tests. Hi Ashish, sorry I was a bit unclear. The tests for the individual data types are fine. I thought that moving just the two large schemas into their own files may make them easier to maintain since keeping them inline requires a large number of escape characters. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48168 --- On July 19, 2014, 5:11 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 19, 2014, 5:11 a.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java 4564e75d9bfc73f8e10f160e2535f1a08b90ff79 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
On July 18, 2014, 11:42 p.m., Lars Francke wrote: serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java, line 269 https://reviews.apache.org/r/23387/diff/9/?file=634613#file634613line269 final is not used elsewhere in this file for local variables and I think it's true for most of the rest of Hive's code. Ashish Singh wrote: That does not mean using final for local variables is wrong. Convention over correctness? Unless there is a strong reason for not declaring variables, not supposed to be modified, as final, I would argue using final is correct. I'll obviously leave it up to you and it's just my personal opinion but Hive's codebase is hard enough to understand as it is for newcomers. Adding confusion by introducing multiple coding styles doesn't help even if your way may be strictly better it may still hurt in the long run. - Lars --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48167 --- On July 19, 2014, 5:11 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 19, 2014, 5:11 a.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java 4564e75d9bfc73f8e10f160e2535f1a08b90ff79 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48120 --- Getting closer - a few more comments. ql/src/test/queries/clientpositive/avro_schema_evolution_native.q https://reviews.apache.org/r/23387/#comment84420 It would be nice to support ALTER TABLE t ADD COLUMN ..., but that is an enhancement - file another JIRA for that? serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java https://reviews.apache.org/r/23387/#comment84417 Is it possible to default name to table name, namespace to database name, and doc to table comment? serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment84416 Remove this now. serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment84418 Should be null not for no doc. serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment84419 Null rather than to indicate the lack of a namespace. (This is fixed in https://issues.apache.org/jira/browse/AVRO-1535, but that hasn't been released yet.) serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment84413 This should add to the start of the list. Also, needs a test for this case. - Tom White On July 17, 2014, 9:10 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 17, 2014, 9:10 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
On July 17, 2014, 1:49 p.m., Tom White wrote: serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java, line 229 https://reviews.apache.org/r/23387/diff/8/?file=634160#file634160line229 It would be simpler to make sure that NULL is included (and is the first branch in the union) in the createAvroUnion() method, and just fall through here. Ashish Singh wrote: I do not think this will be something good or feasible without redesigning many parts without any obvious gain. createAvroUnion() only creates a schema for union, based on union typeinfo passed to it. If I hack it to add null to all unions, I will still need to handle union here differently as union of unions is not possible. That's OK. What you have now is much better, although I found a small bug where the null isn't being added to the start of the list (see other comment). - Tom --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48004 --- On July 17, 2014, 9:10 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 17, 2014, 9:10 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48167 --- serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java https://reviews.apache.org/r/23387/#comment84477 final is not used elsewhere in this file for local variables and I think it's true for most of the rest of Hive's code. serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java https://reviews.apache.org/r/23387/#comment84478 You're accessing fileSchema in the previous line so it's guaranteed to not be null here. Not sure if that's a bug and the check needs to happen a line earlier? If not this ternary can be removed. serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java https://reviews.apache.org/r/23387/#comment84480 final not consistent with Hive codebase serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java https://reviews.apache.org/r/23387/#comment84481 .length() == 0 can be replaced with .isEmpty() serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java https://reviews.apache.org/r/23387/#comment84482 .isEmpty() and two more times serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment84485 Maybe add some Javadoc to this class? final used again for local variables. serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment84486 String.valueOf not needed. pre-increment often confusing, I suggest doing this on a separate line Only minor comments. Patch looks good. Thanks for this and I'm looking forward to have it included. - Lars Francke On July 17, 2014, 9:10 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 17, 2014, 9:10 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48168 --- serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java https://reviews.apache.org/r/23387/#comment84487 Having the ternary expression on a separate line would improve readability. serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment84488 It would improve maintainability to keep the test schemas in separate .avsc files under serde/src/test/resources rather than inline in the file. You can use Guava's Resources class to get the file and construct the schema. For example: Schema expectedSchema = new Schema.Parser().parse( Resources.getResource(record1.avsc).openStream()); - David Chen On July 17, 2014, 9:10 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 17, 2014, 9:10 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48137 --- serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment84434 Good catch. - Ashish Singh On July 17, 2014, 9:10 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 17, 2014, 9:10 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
On July 17, 2014, 1:49 p.m., Tom White wrote: serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java, line 229 https://reviews.apache.org/r/23387/diff/8/?file=634160#file634160line229 It would be simpler to make sure that NULL is included (and is the first branch in the union) in the createAvroUnion() method, and just fall through here. Ashish Singh wrote: I do not think this will be something good or feasible without redesigning many parts without any obvious gain. createAvroUnion() only creates a schema for union, based on union typeinfo passed to it. If I hack it to add null to all unions, I will still need to handle union here differently as union of unions is not possible. Tom White wrote: That's OK. What you have now is much better, although I found a small bug where the null isn't being added to the start of the list (see other comment). OK. Dropping this issue. - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48004 --- On July 17, 2014, 9:10 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 17, 2014, 9:10 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
On July 18, 2014, 1:57 p.m., Tom White wrote: ql/src/test/queries/clientpositive/avro_schema_evolution_native.q, line 25 https://reviews.apache.org/r/23387/diff/9/?file=634606#file634606line25 It would be nice to support ALTER TABLE t ADD COLUMN ..., but that is an enhancement - file another JIRA for that? Created HIVE-7446. On July 18, 2014, 1:57 p.m., Tom White wrote: serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java, line 238 https://reviews.apache.org/r/23387/diff/9/?file=634615#file634615line238 This should add to the start of the list. Also, needs a test for this case. This is actually unreachable code. removeDuplicateNullSchemas makes sure that null is at the beginning of the list. And there will never be a case that a union will be reach here without having a null. Added a couple of test cases to be sure. On July 18, 2014, 1:57 p.m., Tom White wrote: serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java, line 112 https://reviews.apache.org/r/23387/diff/9/?file=634614#file634614line112 Is it possible to default name to table name, namespace to database name, and doc to table comment? I was planning to do this, but slipped off my mind. Thanks for pointing this out. I don't think it is possible to retrieve database name inside serde. Addressed name and doc. - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48120 --- On July 17, 2014, 9:10 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 17, 2014, 9:10 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
On July 18, 2014, 11:42 p.m., Lars Francke wrote: serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java, line 269 https://reviews.apache.org/r/23387/diff/9/?file=634613#file634613line269 final is not used elsewhere in this file for local variables and I think it's true for most of the rest of Hive's code. That does not mean using final for local variables is wrong. Convention over correctness? Unless there is a strong reason for not declaring variables, not supposed to be modified, as final, I would argue using final is correct. On July 18, 2014, 11:42 p.m., Lars Francke wrote: serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java, line 272 https://reviews.apache.org/r/23387/diff/9/?file=634613#file634613line272 You're accessing fileSchema in the previous line so it's guaranteed to not be null here. Not sure if that's a bug and the check needs to happen a line earlier? If not this ternary can be removed. Good catch, this got introduced while refactoring a dual ternary statement. On July 18, 2014, 11:42 p.m., Lars Francke wrote: serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java, line 76 https://reviews.apache.org/r/23387/diff/9/?file=634614#file634614line76 final not consistent with Hive codebase Same as above. On July 18, 2014, 11:42 p.m., Lars Francke wrote: serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java, line 82 https://reviews.apache.org/r/23387/diff/9/?file=634614#file634614line82 .length() == 0 can be replaced with .isEmpty() Done. On July 18, 2014, 11:42 p.m., Lars Francke wrote: serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java, line 87 https://reviews.apache.org/r/23387/diff/9/?file=634614#file634614line87 .isEmpty() and two more times Done. On July 18, 2014, 11:42 p.m., Ashish Singh wrote: Only minor comments. Patch looks good. Thanks for this and I'm looking forward to have it included. Thanks for the review. - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48167 --- On July 17, 2014, 9:10 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 17, 2014, 9:10 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
On July 19, 2014, 12:43 a.m., David Chen wrote: serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java, line 294 https://reviews.apache.org/r/23387/diff/9/?file=634616#file634616line294 It would improve maintainability to keep the test schemas in separate .avsc files under serde/src/test/resources rather than inline in the file. You can use Guava's Resources class to get the file and construct the schema. For example: Schema expectedSchema = new Schema.Parser().parse( Resources.getResource(record1.avsc).openStream()); David, as the tests have a lot common in their schema I am using a method to generate the common schema part and each test only provides a part of schema that is specific to the test. This made my tests have much less LOC. If I create a .avsc file for each test it will much more cumbersome for both maintaining and adding new tests. - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48168 --- On July 17, 2014, 9:10 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 17, 2014, 9:10 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 19, 2014, 5:11 a.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs (updated) - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerdeUtils.java 4564e75d9bfc73f8e10f160e2535f1a08b90ff79 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48004 --- Ashish, thanks for addressing my feedback. Here's a bit more. serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment84252 Still need to pass the Hive column definition here as the field comment. serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment84253 It would be simpler to make sure that NULL is included (and is the first branch in the union) in the createAvroUnion() method, and just fall through here. serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment84256 If you made all records have names then this case statement wouldn't be needed as the default case would be used. Also, having non-deterministic schemas is something we should avoid, since otherwise files in different partitions or written at different times would have schemas that differed only in the record names. Instead, use a counter for gensym - this will work since one instance of TypeInfoToSchema is only used to convert one schema (although it might be a good idea to enforce that). serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment84254 Also need to test the case when the union includes NULL, to check it's not included twice. Also, when it's included but not in the first branch of the union. - Tom White On July 17, 2014, 2:50 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 17, 2014, 2:50 a.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 17, 2014, 9:10 p.m.) Review request for hive. Changes --- Addressed more review comments. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs (updated) - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
On July 17, 2014, 1:49 p.m., Tom White wrote: Ashish, thanks for addressing my feedback. Here's a bit more. Thanks again for the review. On July 17, 2014, 1:49 p.m., Tom White wrote: serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java, line 229 https://reviews.apache.org/r/23387/diff/8/?file=634160#file634160line229 It would be simpler to make sure that NULL is included (and is the first branch in the union) in the createAvroUnion() method, and just fall through here. I do not think this will be something good or feasible without redesigning many parts without any obvious gain. createAvroUnion() only creates a schema for union, based on union typeinfo passed to it. If I hack it to add null to all unions, I will still need to handle union here differently as union of unions is not possible. - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review48004 --- On July 17, 2014, 2:50 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 17, 2014, 2:50 a.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
On July 17, 2014, 5:33 a.m., David Chen wrote: ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java, line 37 https://reviews.apache.org/r/23387/diff/8/?file=634143#file634143line37 Is using AVROFILE rather than AVRO a common use case? If not, should we be allowing both? David, I initially did not add AVROFILE. Brock suggested to be consistent with other storage formats, its good to have it. I do not see any harm in having it. - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review47984 --- On July 17, 2014, 9:10 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 17, 2014, 9:10 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
On July 17, 2014, 5:33 a.m., David Chen wrote: ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java, line 37 https://reviews.apache.org/r/23387/diff/8/?file=634143#file634143line37 Is using AVROFILE rather than AVRO a common use case? If not, should we be allowing both? Ashish Singh wrote: David, I initially did not add AVROFILE. Brock suggested to be consistent with other storage formats, its good to have it. I do not see any harm in having it. I see. That's fine with me. I was just wondering. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review47984 --- On July 17, 2014, 9:10 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 17, 2014, 9:10 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
On July 17, 2014, 5:33 a.m., David Chen wrote: ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java, line 37 https://reviews.apache.org/r/23387/diff/8/?file=634143#file634143line37 Is using AVROFILE rather than AVRO a common use case? If not, should we be allowing both? Ashish Singh wrote: David, I initially did not add AVROFILE. Brock suggested to be consistent with other storage formats, its good to have it. I do not see any harm in having it. David Chen wrote: I see. That's fine with me. I was just wondering. Ok, then I will leave it there. Thanks for the review. - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review47984 --- On July 17, 2014, 9:10 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 17, 2014, 9:10 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23387/#review47894
---
Overall looks good. I added a few comments regarding the Avro schema mapping.
It would also be useful to update the documentation
(https://cwiki.apache.org/confluence/display/Hive/AvroSerDe) with to cover this
feature once it is released, especially the type mappings table.
serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java
https://reviews.apache.org/r/23387/#comment84124
The doc string here could be the Hive column definition, which would be
helpful for debugging.
serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java
https://reviews.apache.org/r/23387/#comment84121
VOID is missing - it maps to Schema.Type.NULL in Avro.
Also,
SHORT and BYTE could map to INT. And CHAR, VARCHAR could map to STRING.
serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java
https://reviews.apache.org/r/23387/#comment84123
Are optional fields supported properly? All schemas should probably be
wrapped in an Avro null union to allow values to be null:
private static Schema optional(Schema schema) {
return Schema.createUnion(Arrays.asList(Schema.create(Schema.Type.NULL),
schema));
}
serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java
https://reviews.apache.org/r/23387/#comment84119
This should be BINARY. BYTE can be mapped to Avro Schema.Type.INT
serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java
https://reviews.apache.org/r/23387/#comment84122
Throw an exception if the key type isn't string.
serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java
https://reviews.apache.org/r/23387/#comment84126
COLUMN_NAMES
serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java
https://reviews.apache.org/r/23387/#comment84125
Pass this as a parameter to getAvroSchemaString() - it's a bit odd to set
it in tests then read it in getAvroSchemaString().
serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java
https://reviews.apache.org/r/23387/#comment84127
Add a field for each Hive type to test they can all work in the context of
a record. Also add a nested record.
- Tom White
On July 16, 2014, 3:35 a.m., Ashish Singh wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23387/
---
(Updated July 16, 2014, 3:35 a.m.)
Review request for hive.
Bugs: HIVE-6806
https://issues.apache.org/jira/browse/HIVE-6806
Repository: hive-git
Description
---
HIVE-6806: Native avro support
Diffs
-
ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java
PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java
1bae0a8fee04049f90b16d813ff4c96707b349c8
ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor
a23ff115512da5fe3167835a88d582c427585b8e
ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java
d53ebc65174d66bfeee25fd2891c69c78f9137ee
ql/src/test/queries/clientpositive/avro_compression_enabled_native.q
PRE-CREATION
ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION
ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION
ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION
ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION
ql/src/test/queries/clientpositive/avro_schema_evolution_native.q
PRE-CREATION
ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out
PRE-CREATION
ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION
ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION
ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION
ql/src/test/results/clientpositive/avro_partitioned_native.q.out
PRE-CREATION
ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out
PRE-CREATION
serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java
1fe31e0034f8988d03a0c51a90904bb93e7cb157
serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java
PRE-CREATION
serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java
PRE-CREATION
Diff: https://reviews.apache.org/r/23387/diff/
Testing
---
Added qTests and unit tests
Thanks,
Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
On July 16, 2014, 4:45 p.m., Tom White wrote:
serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java,
line 97
https://reviews.apache.org/r/23387/diff/7/?file=633382#file633382line97
Are optional fields supported properly? All schemas should probably be
wrapped in an Avro null union to allow values to be null:
private static Schema optional(Schema schema) {
return
Schema.createUnion(Arrays.asList(Schema.create(Schema.Type.NULL), schema));
}
Tom, could you elaborate on Are optional fields supported properly?. I am in
process of addressing your review comments, which are good. Thanks for the
review.
- Ashish
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23387/#review47894
---
On July 16, 2014, 3:35 a.m., Ashish Singh wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23387/
---
(Updated July 16, 2014, 3:35 a.m.)
Review request for hive.
Bugs: HIVE-6806
https://issues.apache.org/jira/browse/HIVE-6806
Repository: hive-git
Description
---
HIVE-6806: Native avro support
Diffs
-
ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java
PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java
1bae0a8fee04049f90b16d813ff4c96707b349c8
ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor
a23ff115512da5fe3167835a88d582c427585b8e
ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java
d53ebc65174d66bfeee25fd2891c69c78f9137ee
ql/src/test/queries/clientpositive/avro_compression_enabled_native.q
PRE-CREATION
ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION
ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION
ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION
ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION
ql/src/test/queries/clientpositive/avro_schema_evolution_native.q
PRE-CREATION
ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out
PRE-CREATION
ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION
ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION
ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION
ql/src/test/results/clientpositive/avro_partitioned_native.q.out
PRE-CREATION
ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out
PRE-CREATION
serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java
1fe31e0034f8988d03a0c51a90904bb93e7cb157
serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java
PRE-CREATION
serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java
PRE-CREATION
Diff: https://reviews.apache.org/r/23387/diff/
Testing
---
Added qTests and unit tests
Thanks,
Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 17, 2014, 2:50 a.m.) Review request for hive. Changes --- Addressed review comments. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs (updated) - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review47979 --- ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java https://reviews.apache.org/r/23387/#comment84224 Done. serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment84225 Done. - Ashish Singh On July 17, 2014, 2:50 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 17, 2014, 2:50 a.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review47984 --- ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java https://reviews.apache.org/r/23387/#comment84238 Is using AVROFILE rather than AVRO a common use case? If not, should we be allowing both? ql/src/test/queries/clientpositive/avro_compression_enabled_native.q https://reviews.apache.org/r/23387/#comment84239 Nitpick: Better to have this indented correctly. ql/src/test/queries/clientpositive/avro_joins_native.q https://reviews.apache.org/r/23387/#comment84240 Nitpick: better to have this indented correctly. ql/src/test/queries/clientpositive/avro_joins_native.q https://reviews.apache.org/r/23387/#comment84241 Here too and similarly in other qfiles added as part of this patch. - David Chen On July 17, 2014, 2:50 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 17, 2014, 2:50 a.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 0db12437406170686a21b6055d83156fe5d6a55f serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 16, 2014, 3:06 a.m.) Review request for hive. Changes --- Add qtest for avro schema evolution. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description (updated) --- HIVE-6806: Native avro support Diffs (updated) - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 16, 2014, 3:28 a.m.) Review request for hive. Changes --- Addressed review comments. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs (updated) - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review47852 --- serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java https://reviews.apache.org/r/23387/#comment84079 Done. serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java https://reviews.apache.org/r/23387/#comment84080 Done. serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java https://reviews.apache.org/r/23387/#comment84081 Done. serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment84082 Changed it to convert, as TypeInfoToSchema.convert says it all. serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment84083 Done. serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment84084 Done. serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment84085 Done. - Ashish Singh On July 16, 2014, 3:28 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 16, 2014, 3:28 a.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 16, 2014, 3:35 a.m.) Review request for hive. Changes --- Fixed some more spacing issue. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs (updated) - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review47854 --- serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment84087 Done. - Ashish Singh On July 16, 2014, 3:35 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 16, 2014, 3:35 a.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review47856 --- This looks really good and I think it's quite close to being committed!! I think we should handle CHAR and VARCHAR schema generation in a follow-on jira. Can you file a jira for this and link it to HIVE-6806? ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java https://reviews.apache.org/r/23387/#comment84089 Should we support AVROFILE as well? I don't like the FILE extension but the rest of the formats use this.. serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment84088 Let's throw UnsupportedOperationException here? I think it gives the user a better idea by the name alone. That is that it's not supported - Brock Noland On July 16, 2014, 3:35 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 16, 2014, 3:35 a.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native avro support Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_schema_evolution_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_schema_evolution_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 14, 2014, 9:57 p.m.) Review request for hive. Changes --- Rebased Summary (updated) - HIVE-6806: Native avro support Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description (updated) --- HIVE-6806: Native avro support Diffs (updated) - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 14, 2014, 10:02 p.m.) Review request for hive. Changes --- Reverting the description to original description. rbt post tool changes it to last commit message. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description (updated) --- HIVE-6806: Native Avro support in Hive Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 14, 2014, 10:05 p.m.) Review request for hive. Changes --- Reverting to original summary. rbt post tool changes it to last commit message. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native Avro support in Hive Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native avro support
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review47747 --- serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java https://reviews.apache.org/r/23387/#comment83943 Please do not use Yoda expressions (i.e. `value operator variable`). Also, I believe the coding conventions say to put the operator at the beginning of the line when the expression spans multiple lines and that the additional lines must be indented 4 spaces. serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java https://reviews.apache.org/r/23387/#comment83945 Extra space after = serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java https://reviews.apache.org/r/23387/#comment83944 Please indent these lines with 4 spaces since these are continuations of the previous line. serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment83949 I know this is a bit nitpicky but I think it is better to use convert rather than create. serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment83947 These two lines should be indented 4 spaces rather than 2 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment83948 Nitpick: space after `for` serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment83946 If these lines are more than 100 characters wide, please split them. serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java https://reviews.apache.org/r/23387/#comment83950 These lines should be indented 4 spaces rather than 2. Same with other places in this file where lines are split. - David Chen On July 14, 2014, 10:05 p.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 14, 2014, 10:05 p.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native Avro support in Hive Diffs - ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 1bae0a8fee04049f90b16d813ff4c96707b349c8 ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor a23ff115512da5fe3167835a88d582c427585b8e ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java d53ebc65174d66bfeee25fd2891c69c78f9137ee ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native Avro support in Hive
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 11, 2014, 9:16 p.m.) Review request for hive. Changes --- RBT tools seems to mess up the summary. Restoring it back to original summary. Summary (updated) - HIVE-6806: Native Avro support in Hive Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native Avro support in Hive Diffs - ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 75394f3bc4f2285a7ced97ea90788d5bbff6b563 ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 6cd1f39df3d419651755c35aab7cfc06833b16a4 ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 412a046488eaea42a6416c7cbd514715d37e249f ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g f934ac4e3b736eed1b3060fa516124c67f9a2f87 ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 9c001c1495b423c19f3fa710c74f1bb1e24a08f4 ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native Avro support in Hive
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 12, 2014, 2:18 a.m.) Review request for hive. Changes --- Add missing .q.out file and have no schema provided be dealt as it used to be before this patch Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native Avro support in Hive Diffs (updated) - ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 75394f3bc4f2285a7ced97ea90788d5bbff6b563 ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 6cd1f39df3d419651755c35aab7cfc06833b16a4 ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 412a046488eaea42a6416c7cbd514715d37e249f ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g f934ac4e3b736eed1b3060fa516124c67f9a2f87 ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 9c001c1495b423c19f3fa710c74f1bb1e24a08f4 ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java PRE-CREATION serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java PRE-CREATION Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests and unit tests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native Avro support in Hive
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review47568 --- I *love* this patch! Thank you so much. serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java https://reviews.apache.org/r/23387/#comment83585 Can we add some unit tests for this class? serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java https://reviews.apache.org/r/23387/#comment83588 Two thoughts: 1) Char/varchar support? 2) By defaulting to null won't any new types end up with null if this code is not updated? I think instead we should throw an exception for unknown types. - Brock Noland On July 10, 2014, 4:50 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 10, 2014, 4:50 a.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native Avro support in Hive Diffs - ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 75394f3bc4f2285a7ced97ea90788d5bbff6b563 ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 6cd1f39df3d419651755c35aab7cfc06833b16a4 ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 412a046488eaea42a6416c7cbd514715d37e249f ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g f934ac4e3b736eed1b3060fa516124c67f9a2f87 ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 9c001c1495b423c19f3fa710c74f1bb1e24a08f4 ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native Avro support in Hive
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review47609 --- serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java https://reviews.apache.org/r/23387/#comment83713 It might be better to call this TypeInfoToSchema to make it consistent with SchemaToTypeInfo, which converts from Avro Schema to Hive TypeInfo. - David Chen On July 10, 2014, 4:50 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 10, 2014, 4:50 a.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native Avro support in Hive Diffs - ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 75394f3bc4f2285a7ced97ea90788d5bbff6b563 ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 6cd1f39df3d419651755c35aab7cfc06833b16a4 ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 412a046488eaea42a6416c7cbd514715d37e249f ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g f934ac4e3b736eed1b3060fa516124c67f9a2f87 ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 9c001c1495b423c19f3fa710c74f1bb1e24a08f4 ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native Avro support in Hive
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review47612 --- ql/src/test/queries/clientpositive/avro_decimal_native.q https://reviews.apache.org/r/23387/#comment83721 I think we can remove COMMENT 'from deserializer' as well. Thx!! - Brock Noland On July 10, 2014, 4:50 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 10, 2014, 4:50 a.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native Avro support in Hive Diffs - ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 75394f3bc4f2285a7ced97ea90788d5bbff6b563 ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 6cd1f39df3d419651755c35aab7cfc06833b16a4 ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 412a046488eaea42a6416c7cbd514715d37e249f ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g f934ac4e3b736eed1b3060fa516124c67f9a2f87 ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 9c001c1495b423c19f3fa710c74f1bb1e24a08f4 ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native Avro support in Hive
On July 10, 2014, 3:01 p.m., Brock Noland wrote: I *love* this patch! Thank you so much. Thanks! On July 10, 2014, 3:01 p.m., Brock Noland wrote: serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java, line 34 https://reviews.apache.org/r/23387/diff/1/?file=627564#file627564line34 Can we add some unit tests for this class? Will do. On July 10, 2014, 3:01 p.m., Brock Noland wrote: serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java, line 110 https://reviews.apache.org/r/23387/diff/1/?file=627564#file627564line110 Two thoughts: 1) Char/varchar support? 2) By defaulting to null won't any new types end up with null if this code is not updated? I think instead we should throw an exception for unknown types. 1. Char/varchar is not yet supported by avro. It is planned though. 2. Good point. Will do. - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review47568 --- On July 10, 2014, 4:50 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 10, 2014, 4:50 a.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native Avro support in Hive Diffs - ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 75394f3bc4f2285a7ced97ea90788d5bbff6b563 ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 6cd1f39df3d419651755c35aab7cfc06833b16a4 ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 412a046488eaea42a6416c7cbd514715d37e249f ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g f934ac4e3b736eed1b3060fa516124c67f9a2f87 ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 9c001c1495b423c19f3fa710c74f1bb1e24a08f4 ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native Avro support in Hive
On July 10, 2014, 8:13 p.m., David Chen wrote: serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java, line 34 https://reviews.apache.org/r/23387/diff/1/?file=627564#file627564line34 It might be better to call this TypeInfoToSchema to make it consistent with SchemaToTypeInfo, which converts from Avro Schema to Hive TypeInfo. That sounds reasonable, but the functionality of this class is not just to convert typeinfo to schema. It also uses column names. From functionality point of view, AvroSchemaGenerator still sounds better than TypeInfoToSchema. TypeInfoToSchema would have been suitable had the class been converting one typeInfo to avro schema. Let me know if you disagree. - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review47609 --- On July 10, 2014, 4:50 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 10, 2014, 4:50 a.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native Avro support in Hive Diffs - ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 75394f3bc4f2285a7ced97ea90788d5bbff6b563 ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 6cd1f39df3d419651755c35aab7cfc06833b16a4 ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 412a046488eaea42a6416c7cbd514715d37e249f ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g f934ac4e3b736eed1b3060fa516124c67f9a2f87 ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 9c001c1495b423c19f3fa710c74f1bb1e24a08f4 ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native Avro support in Hive
On July 10, 2014, 8:28 p.m., Brock Noland wrote: ql/src/test/queries/clientpositive/avro_decimal_native.q, line 13 https://reviews.apache.org/r/23387/diff/1/?file=627556#file627556line13 I think we can remove COMMENT 'from deserializer' as well. Thx!! Yes, it is redundant and we will be better without it. Will do. - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review47612 --- On July 10, 2014, 4:50 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 10, 2014, 4:50 a.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native Avro support in Hive Diffs - ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 75394f3bc4f2285a7ced97ea90788d5bbff6b563 ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 6cd1f39df3d419651755c35aab7cfc06833b16a4 ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 412a046488eaea42a6416c7cbd514715d37e249f ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g f934ac4e3b736eed1b3060fa516124c67f9a2f87 ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 9c001c1495b423c19f3fa710c74f1bb1e24a08f4 ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native Avro support in Hive
On July 10, 2014, 8:13 p.m., David Chen wrote: serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java, line 34 https://reviews.apache.org/r/23387/diff/1/?file=627564#file627564line34 It might be better to call this TypeInfoToSchema to make it consistent with SchemaToTypeInfo, which converts from Avro Schema to Hive TypeInfo. Ashish Singh wrote: That sounds reasonable, but the functionality of this class is not just to convert typeinfo to schema. It also uses column names. From functionality point of view, AvroSchemaGenerator still sounds better than TypeInfoToSchema. TypeInfoToSchema would have been suitable had the class been converting one typeInfo to avro schema. Let me know if you disagree. Just realized that SchemaToTypeInfo also generates all typeinfos from schema. That makes your comment even more reasonable. Will do. - Ashish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/#review47609 --- On July 10, 2014, 4:50 a.m., Ashish Singh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 10, 2014, 4:50 a.m.) Review request for hive. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native Avro support in Hive Diffs - ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 75394f3bc4f2285a7ced97ea90788d5bbff6b563 ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 6cd1f39df3d419651755c35aab7cfc06833b16a4 ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 412a046488eaea42a6416c7cbd514715d37e249f ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g f934ac4e3b736eed1b3060fa516124c67f9a2f87 ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 9c001c1495b423c19f3fa710c74f1bb1e24a08f4 ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests Thanks, Ashish Singh
Re: Review Request 23387: HIVE-6806: Native Avro support in Hive
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23387/ --- (Updated July 10, 2014, 4:50 a.m.) Review request for hive. Changes --- Link hive jira. Bugs: HIVE-6806 https://issues.apache.org/jira/browse/HIVE-6806 Repository: hive-git Description --- HIVE-6806: Native Avro support in Hive Diffs - ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 75394f3bc4f2285a7ced97ea90788d5bbff6b563 ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 6cd1f39df3d419651755c35aab7cfc06833b16a4 ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 412a046488eaea42a6416c7cbd514715d37e249f ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g f934ac4e3b736eed1b3060fa516124c67f9a2f87 ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 9c001c1495b423c19f3fa710c74f1bb1e24a08f4 ql/src/test/queries/clientpositive/avro_compression_enabled_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java PRE-CREATION serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 1fe31e0034f8988d03a0c51a90904bb93e7cb157 Diff: https://reviews.apache.org/r/23387/diff/ Testing --- Added qTests Thanks, Ashish Singh
