You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Ashish Singh <as...@cloudera.com> on 2014/07/14 23:57:03 UTC
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
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
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
Posted by Ashish Singh <as...@cloudera.com>.
> 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
Posted by Tom White <to...@apache.org>.
-----------------------------------------------------------
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
Posted by Ashish Singh <as...@cloudera.com>.
> 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
Posted by Tom White <to...@apache.org>.
> 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
Posted by Ashish Singh <as...@cloudera.com>.
> 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
Posted by Tom White <to...@apache.org>.
-----------------------------------------------------------
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
Posted by Ashish Singh <as...@cloudera.com>.
> 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
Posted by David Chen <dc...@linkedin.com>.
> 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
Posted by Ashish Singh <as...@cloudera.com>.
> 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
Posted by David Chen <dc...@linkedin.com>.
-----------------------------------------------------------
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
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
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
Posted by Ashish Singh <as...@cloudera.com>.
> 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
Posted by Lars Francke <as...@lars-francke.de>.
-----------------------------------------------------------
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
Posted by Lars Francke <as...@lars-francke.de>.
-----------------------------------------------------------
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
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
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
Posted by David Chen <dc...@linkedin.com>.
-----------------------------------------------------------
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
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
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
Posted by Ashish Singh <as...@cloudera.com>.
> 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
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
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
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
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.
Changes
-------
Move avro schema in unit tests to separate .avsc files.
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
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
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
Posted by Tom White <to...@apache.org>.
> 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
Posted by Ashish Singh <as...@cloudera.com>.
> 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
Posted by Tom White <to...@apache.org>.
> 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
Posted by Ashish Singh <as...@cloudera.com>.
> 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
Posted by Tom White <to...@apache.org>.
> 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
Posted by Ashish Singh <as...@cloudera.com>.
> 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
Posted by Tom White <to...@apache.org>.
-----------------------------------------------------------
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
Posted by Lars Francke <as...@lars-francke.de>.
> 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
Posted by Ashish Singh <as...@cloudera.com>.
> 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
Posted by Lars Francke <as...@lars-francke.de>.
-----------------------------------------------------------
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
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
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
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
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
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
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
Posted by David Chen <dc...@linkedin.com>.
> 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
Posted by Ashish Singh <as...@cloudera.com>.
> 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
Posted by Ashish Singh <as...@cloudera.com>.
> 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
Posted by David Chen <dc...@linkedin.com>.
-----------------------------------------------------------
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
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
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
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
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
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
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
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
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
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
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
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
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
Posted by David Chen <dc...@linkedin.com>.
-----------------------------------------------------------
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
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
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
Posted by Ashish Singh <as...@cloudera.com>.
-----------------------------------------------------------
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