subject:"Re\: Review Request 23387\: HIVE\-6806\: Native avro support"

Re: Review Request 23387: HIVE-6806: Native avro support

2014-07-23 Thread David Chen


---
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

2014-07-23 Thread Lars Francke


---
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

2014-07-23 Thread Ashish Singh



> 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

2014-07-23 Thread Ashish Singh



> On July 23, 2014, 7:26 a.m., Lars Francke wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java, line 89
> > 
> >
> > 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

2014-07-23 Thread Ashish Singh


---
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

2014-07-23 Thread Ashish Singh


---
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

2014-07-23 Thread Tom White



> On July 18, 2014, 1:57 p.m., Tom White wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java, line 112
> > 
> >
> > 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

Re: Review Request 23387: HIVE-6806: Native avro support

2014-07-23 Thread Lars Francke


---
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


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


nit: these two variables can be List<> instead of ArrayList<>



serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java


This should be replaced with  org.junit.Assert. The junit one is obsolete.



serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java


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

2014-07-22 Thread Brock Noland


---
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

2014-07-22 Thread Brock Noland


---
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


These can be static?



serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java


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

2014-07-22 Thread Ashish Singh



> On July 19, 2014, 12:43 a.m., David Chen wrote:
> > serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java,
> >  line 294
> > 
> >
> > 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

2014-07-22 Thread Ashish Singh


---
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

2014-07-22 Thread Ashish Singh



> On July 18, 2014, 1:57 p.m., Tom White wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java, line 112
> > 
> >
> > 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

2014-07-22 Thread Tom White



> On July 18, 2014, 1:57 p.m., Tom White wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java, line 112
> > 
> >
> > 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

2014-07-21 Thread Ashish Singh



> On July 18, 2014, 1:57 p.m., Tom White wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java, line 112
> > 
> >
> > 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

2014-07-21 Thread Tom White



> On July 18, 2014, 1:57 p.m., Tom White wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java, line 112
> > 
> >
> > 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

2014-07-19 Thread Lars Francke



> On July 18, 2014, 11:42 p.m., Lars Francke wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java, 
> > line 269
> > 
> >
> > "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

2014-07-19 Thread David Chen



> On July 19, 2014, 12:43 a.m., David Chen wrote:
> > serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java,
> >  line 294
> > 
> >
> > 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

2014-07-18 Thread Ashish Singh


---
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

2014-07-18 Thread Ashish Singh



> On July 19, 2014, 12:43 a.m., David Chen wrote:
> > serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java,
> >  line 294
> > 
> >
> > 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

2014-07-18 Thread Ashish Singh



> On July 18, 2014, 11:42 p.m., Lars Francke wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java, 
> > line 269
> > 
> >
> > "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
> > 
> >
> > 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
> > 
> >
> > "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
> > 
> >
> > .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
> > 
> >
> > .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

2014-07-18 Thread Ashish Singh



> On July 18, 2014, 1:57 p.m., Tom White wrote:
> > ql/src/test/queries/clientpositive/avro_schema_evolution_native.q, line 25
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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

2014-07-18 Thread Ashish Singh



> On July 17, 2014, 1:49 p.m., Tom White wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java, 
> > line 229
> > 
> >
> > 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

2014-07-18 Thread Ashish Singh


---
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


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

2014-07-18 Thread David Chen


---
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


Having the ternary expression on a separate line would improve readability.



serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java


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

2014-07-18 Thread Lars Francke


---
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


"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


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


"final" not consistent with Hive codebase



serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java


.length() == 0

can be replaced with

.isEmpty()



serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java


.isEmpty()

and two more times



serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java


Maybe add some Javadoc to this class?

"final" used again for local variables.



serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java


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

2014-07-18 Thread Tom White



> On July 17, 2014, 1:49 p.m., Tom White wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java, 
> > line 229
> > 
> >
> > 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

2014-07-18 Thread Tom White


---
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


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


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


Remove this now.



serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java


Should be null not "" for no doc.



serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java


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


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

2014-07-17 Thread Ashish Singh



> On July 17, 2014, 5:33 a.m., David Chen wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java, line 37
> > 
> >
> > 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

2014-07-17 Thread David Chen



> On July 17, 2014, 5:33 a.m., David Chen wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java, line 37
> > 
> >
> > 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

2014-07-17 Thread Ashish Singh



> On July 17, 2014, 5:33 a.m., David Chen wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java, line 37
> > 
> >
> > 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

2014-07-17 Thread Ashish Singh



> 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
> > 
> >
> > 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

2014-07-17 Thread Ashish Singh


---
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

2014-07-17 Thread Tom White


---
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


Still need to pass the Hive column definition here as the field comment.



serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java


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


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


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

2014-07-16 Thread David Chen


---
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


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


Nitpick: Better to have this indented correctly.



ql/src/test/queries/clientpositive/avro_joins_native.q


Nitpick: better to have this indented correctly.



ql/src/test/queries/clientpositive/avro_joins_native.q


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

2014-07-16 Thread Ashish Singh


---
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


Done.



serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java


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

2014-07-16 Thread Ashish Singh


---
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

2014-07-16 Thread Ashish Singh



> On July 16, 2014, 4:45 p.m., Tom White wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java, 
> > line 97
> > 
> >
> > 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

2014-07-16 Thread Tom White


---
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


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


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


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


This should be BINARY. BYTE can be mapped to Avro Schema.Type.INT



serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java


Throw an exception if the key type isn't string.



serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java


COLUMN_NAMES



serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java


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


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

2014-07-15 Thread Brock Noland


---
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


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


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

2014-07-15 Thread Ashish Singh


---
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


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

2014-07-15 Thread Ashish Singh


---
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

2014-07-15 Thread Ashish Singh


---
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


Done.



serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java


Done.



serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java


Done.



serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java


Changed it to convert, as TypeInfoToSchema.convert says it all.



serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java


Done.



serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java


Done.



serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java


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

2014-07-15 Thread Ashish Singh


---
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

2014-07-15 Thread Ashish Singh


---
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

2014-07-14 Thread David Chen


---
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


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


Extra space after =



serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java


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


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


These two lines should be indented 4 spaces rather than 2



serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java


Nitpick: space after `for`



serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java


If these lines are more than 100 characters wide, please split them.



serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java


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

2014-07-14 Thread Ashish Singh


---
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

2014-07-14 Thread Ashish Singh


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23387/
---

(Updated July 14, 2014, 10:02 p.m.)


Review request for hive.


Changes
---

Reverting the description to original description. rbt post tool changes it to 
last commit message.


Bugs: HIVE-6806
https://issues.apache.org/jira/browse/HIVE-6806


Repository: hive-git


Description (updated)
---

HIVE-6806: Native Avro support in Hive


Diffs
-

  ql/src/java/org/apache/hadoop/hive/ql/io/AvroStorageFormatDescriptor.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/io/IOConstants.java 
1bae0a8fee04049f90b16d813ff4c96707b349c8 
  
ql/src/main/resources/META-INF/services/org.apache.hadoop.hive.ql.io.StorageFormatDescriptor
 a23ff115512da5fe3167835a88d582c427585b8e 
  ql/src/test/org/apache/hadoop/hive/ql/io/TestStorageFormatDescriptor.java 
d53ebc65174d66bfeee25fd2891c69c78f9137ee 
  ql/src/test/queries/clientpositive/avro_compression_enabled_native.q 
PRE-CREATION 
  ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION 
  ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION 
  ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION 
  ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION 
  ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION 
  serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 
1fe31e0034f8988d03a0c51a90904bb93e7cb157 
  serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java 
PRE-CREATION 
  serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java 
PRE-CREATION 

Diff: https://reviews.apache.org/r/23387/diff/


Testing
---

Added qTests and unit tests


Thanks,

Ashish Singh

Re: Review Request 23387: HIVE-6806: Native avro support

2014-07-14 Thread Ashish Singh


---
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 in Hive

2014-07-11 Thread Ashish Singh


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23387/
---

(Updated July 12, 2014, 2:18 a.m.)


Review request for hive.


Changes
---

Add missing .q.out file and have no schema provided be dealt as it used to be 
before this patch


Bugs: HIVE-6806
https://issues.apache.org/jira/browse/HIVE-6806


Repository: hive-git


Description
---

HIVE-6806: Native Avro support in Hive


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 
75394f3bc4f2285a7ced97ea90788d5bbff6b563 
  ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
6cd1f39df3d419651755c35aab7cfc06833b16a4 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 
412a046488eaea42a6416c7cbd514715d37e249f 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 
f934ac4e3b736eed1b3060fa516124c67f9a2f87 
  ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 
9c001c1495b423c19f3fa710c74f1bb1e24a08f4 
  ql/src/test/queries/clientpositive/avro_compression_enabled_native.q 
PRE-CREATION 
  ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION 
  ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION 
  ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION 
  ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION 
  ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/avro_native.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION 
  serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 
1fe31e0034f8988d03a0c51a90904bb93e7cb157 
  serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java 
PRE-CREATION 
  serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java 
PRE-CREATION 

Diff: https://reviews.apache.org/r/23387/diff/


Testing
---

Added qTests and unit tests


Thanks,

Ashish Singh

Re: Review Request 23387: HIVE-6806: Native Avro support in Hive

2014-07-11 Thread Ashish Singh


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23387/
---

(Updated July 11, 2014, 9:16 p.m.)


Review request for hive.


Changes
---

RBT tools seems to mess up the summary. Restoring it back to original summary.


Summary (updated)
-

HIVE-6806: Native Avro support in Hive


Bugs: HIVE-6806
https://issues.apache.org/jira/browse/HIVE-6806


Repository: hive-git


Description
---

HIVE-6806: Native Avro support in Hive


Diffs
-

  ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 
75394f3bc4f2285a7ced97ea90788d5bbff6b563 
  ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
6cd1f39df3d419651755c35aab7cfc06833b16a4 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 
412a046488eaea42a6416c7cbd514715d37e249f 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 
f934ac4e3b736eed1b3060fa516124c67f9a2f87 
  ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 
9c001c1495b423c19f3fa710c74f1bb1e24a08f4 
  ql/src/test/queries/clientpositive/avro_compression_enabled_native.q 
PRE-CREATION 
  ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION 
  ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION 
  ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION 
  ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION 
  ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION 
  serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 
1fe31e0034f8988d03a0c51a90904bb93e7cb157 
  serde/src/java/org/apache/hadoop/hive/serde2/avro/TypeInfoToSchema.java 
PRE-CREATION 
  serde/src/test/org/apache/hadoop/hive/serde2/avro/TestTypeInfoToSchema.java 
PRE-CREATION 

Diff: https://reviews.apache.org/r/23387/diff/


Testing
---

Added qTests and unit tests


Thanks,

Ashish Singh

Re: Review Request 23387: HIVE-6806: Native Avro support in Hive

2014-07-10 Thread Ashish Singh



> On July 10, 2014, 8:13 p.m., David Chen wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java, 
> > line 34
> > 
> >
> > It might be better to call this TypeInfoToSchema to make it consistent 
> > with SchemaToTypeInfo, which converts from Avro Schema to Hive TypeInfo.
> 
> Ashish Singh wrote:
> That sounds reasonable, but the functionality of this class is not just 
> to convert typeinfo to schema. It also uses column names. 
> 
> From functionality point of view, AvroSchemaGenerator still sounds better 
> than TypeInfoToSchema. TypeInfoToSchema would have been suitable had the 
> class been converting one typeInfo to avro schema. Let me know if you 
> disagree.

Just realized that SchemaToTypeInfo also generates all typeinfos from schema. 
That makes your comment even more reasonable. Will do.


- Ashish


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23387/#review47609
---


On July 10, 2014, 4:50 a.m., Ashish Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23387/
> ---
> 
> (Updated July 10, 2014, 4:50 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-6806
> https://issues.apache.org/jira/browse/HIVE-6806
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-6806: Native Avro support in Hive
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 
> 75394f3bc4f2285a7ced97ea90788d5bbff6b563 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
> 6cd1f39df3d419651755c35aab7cfc06833b16a4 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 
> 412a046488eaea42a6416c7cbd514715d37e249f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 
> f934ac4e3b736eed1b3060fa516124c67f9a2f87 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 
> 9c001c1495b423c19f3fa710c74f1bb1e24a08f4 
>   ql/src/test/queries/clientpositive/avro_compression_enabled_native.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_partitioned_native.q.out 
> PRE-CREATION 
>   serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java 
> PRE-CREATION 
>   serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 
> 1fe31e0034f8988d03a0c51a90904bb93e7cb157 
> 
> Diff: https://reviews.apache.org/r/23387/diff/
> 
> 
> Testing
> ---
> 
> Added qTests
> 
> 
> Thanks,
> 
> Ashish Singh
> 
>

Re: Review Request 23387: HIVE-6806: Native Avro support in Hive

2014-07-10 Thread Ashish Singh



> On July 10, 2014, 8:13 p.m., David Chen wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java, 
> > line 34
> > 
> >
> > It might be better to call this TypeInfoToSchema to make it consistent 
> > with SchemaToTypeInfo, which converts from Avro Schema to Hive TypeInfo.

That sounds reasonable, but the functionality of this class is not just to 
convert typeinfo to schema. It also uses column names. 

>From functionality point of view, AvroSchemaGenerator still sounds better than 
>TypeInfoToSchema. TypeInfoToSchema would have been suitable had the class been 
>converting one typeInfo to avro schema. Let me know if you disagree.


- Ashish


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23387/#review47609
---


On July 10, 2014, 4:50 a.m., Ashish Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23387/
> ---
> 
> (Updated July 10, 2014, 4:50 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-6806
> https://issues.apache.org/jira/browse/HIVE-6806
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-6806: Native Avro support in Hive
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 
> 75394f3bc4f2285a7ced97ea90788d5bbff6b563 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
> 6cd1f39df3d419651755c35aab7cfc06833b16a4 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 
> 412a046488eaea42a6416c7cbd514715d37e249f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 
> f934ac4e3b736eed1b3060fa516124c67f9a2f87 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 
> 9c001c1495b423c19f3fa710c74f1bb1e24a08f4 
>   ql/src/test/queries/clientpositive/avro_compression_enabled_native.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_partitioned_native.q.out 
> PRE-CREATION 
>   serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java 
> PRE-CREATION 
>   serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 
> 1fe31e0034f8988d03a0c51a90904bb93e7cb157 
> 
> Diff: https://reviews.apache.org/r/23387/diff/
> 
> 
> Testing
> ---
> 
> Added qTests
> 
> 
> Thanks,
> 
> Ashish Singh
> 
>

Re: Review Request 23387: HIVE-6806: Native Avro support in Hive

2014-07-10 Thread Ashish Singh



> On July 10, 2014, 8:28 p.m., Brock Noland wrote:
> > ql/src/test/queries/clientpositive/avro_decimal_native.q, line 13
> > 
> >
> > I think we can remove  COMMENT 'from deserializer' as well.
> > 
> > Thx!!

Yes, it is redundant and we will be better without it. Will do.


- Ashish


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23387/#review47612
---


On July 10, 2014, 4:50 a.m., Ashish Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23387/
> ---
> 
> (Updated July 10, 2014, 4:50 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-6806
> https://issues.apache.org/jira/browse/HIVE-6806
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-6806: Native Avro support in Hive
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 
> 75394f3bc4f2285a7ced97ea90788d5bbff6b563 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
> 6cd1f39df3d419651755c35aab7cfc06833b16a4 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 
> 412a046488eaea42a6416c7cbd514715d37e249f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 
> f934ac4e3b736eed1b3060fa516124c67f9a2f87 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 
> 9c001c1495b423c19f3fa710c74f1bb1e24a08f4 
>   ql/src/test/queries/clientpositive/avro_compression_enabled_native.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_partitioned_native.q.out 
> PRE-CREATION 
>   serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java 
> PRE-CREATION 
>   serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 
> 1fe31e0034f8988d03a0c51a90904bb93e7cb157 
> 
> Diff: https://reviews.apache.org/r/23387/diff/
> 
> 
> Testing
> ---
> 
> Added qTests
> 
> 
> Thanks,
> 
> Ashish Singh
> 
>

Re: Review Request 23387: HIVE-6806: Native Avro support in Hive

2014-07-10 Thread Ashish Singh



> On July 10, 2014, 3:01 p.m., Brock Noland wrote:
> > I *love* this patch! Thank you so much.

Thanks!


> On July 10, 2014, 3:01 p.m., Brock Noland wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java, 
> > line 34
> > 
> >
> > Can we add some unit tests for this class?

Will do.


> On July 10, 2014, 3:01 p.m., Brock Noland wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java, 
> > line 110
> > 
> >
> > Two thoughts:
> > 
> > 1) Char/varchar support?
> > 2) By defaulting to null won't any new types end up with null if this 
> > code is not updated? I think instead we should throw an exception for 
> > unknown types.

1. Char/varchar is not yet supported by avro. It is planned though.
2. Good point. Will do.


- Ashish


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23387/#review47568
---


On July 10, 2014, 4:50 a.m., Ashish Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23387/
> ---
> 
> (Updated July 10, 2014, 4:50 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-6806
> https://issues.apache.org/jira/browse/HIVE-6806
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-6806: Native Avro support in Hive
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 
> 75394f3bc4f2285a7ced97ea90788d5bbff6b563 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
> 6cd1f39df3d419651755c35aab7cfc06833b16a4 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 
> 412a046488eaea42a6416c7cbd514715d37e249f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 
> f934ac4e3b736eed1b3060fa516124c67f9a2f87 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 
> 9c001c1495b423c19f3fa710c74f1bb1e24a08f4 
>   ql/src/test/queries/clientpositive/avro_compression_enabled_native.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_partitioned_native.q.out 
> PRE-CREATION 
>   serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java 
> PRE-CREATION 
>   serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 
> 1fe31e0034f8988d03a0c51a90904bb93e7cb157 
> 
> Diff: https://reviews.apache.org/r/23387/diff/
> 
> 
> Testing
> ---
> 
> Added qTests
> 
> 
> Thanks,
> 
> Ashish Singh
> 
>

Re: Review Request 23387: HIVE-6806: Native Avro support in Hive

2014-07-10 Thread Brock Noland


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23387/#review47612
---



ql/src/test/queries/clientpositive/avro_decimal_native.q


I think we can remove  COMMENT 'from deserializer' as well.

Thx!!


- Brock Noland


On July 10, 2014, 4:50 a.m., Ashish Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23387/
> ---
> 
> (Updated July 10, 2014, 4:50 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-6806
> https://issues.apache.org/jira/browse/HIVE-6806
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-6806: Native Avro support in Hive
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 
> 75394f3bc4f2285a7ced97ea90788d5bbff6b563 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
> 6cd1f39df3d419651755c35aab7cfc06833b16a4 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 
> 412a046488eaea42a6416c7cbd514715d37e249f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 
> f934ac4e3b736eed1b3060fa516124c67f9a2f87 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 
> 9c001c1495b423c19f3fa710c74f1bb1e24a08f4 
>   ql/src/test/queries/clientpositive/avro_compression_enabled_native.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_partitioned_native.q.out 
> PRE-CREATION 
>   serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java 
> PRE-CREATION 
>   serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 
> 1fe31e0034f8988d03a0c51a90904bb93e7cb157 
> 
> Diff: https://reviews.apache.org/r/23387/diff/
> 
> 
> Testing
> ---
> 
> Added qTests
> 
> 
> Thanks,
> 
> Ashish Singh
> 
>

Re: Review Request 23387: HIVE-6806: Native Avro support in Hive

2014-07-10 Thread David Chen


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23387/#review47609
---



serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java


It might be better to call this TypeInfoToSchema to make it consistent with 
SchemaToTypeInfo, which converts from Avro Schema to Hive TypeInfo.


- David Chen


On July 10, 2014, 4:50 a.m., Ashish Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23387/
> ---
> 
> (Updated July 10, 2014, 4:50 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-6806
> https://issues.apache.org/jira/browse/HIVE-6806
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-6806: Native Avro support in Hive
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 
> 75394f3bc4f2285a7ced97ea90788d5bbff6b563 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
> 6cd1f39df3d419651755c35aab7cfc06833b16a4 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 
> 412a046488eaea42a6416c7cbd514715d37e249f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 
> f934ac4e3b736eed1b3060fa516124c67f9a2f87 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 
> 9c001c1495b423c19f3fa710c74f1bb1e24a08f4 
>   ql/src/test/queries/clientpositive/avro_compression_enabled_native.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_partitioned_native.q.out 
> PRE-CREATION 
>   serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java 
> PRE-CREATION 
>   serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 
> 1fe31e0034f8988d03a0c51a90904bb93e7cb157 
> 
> Diff: https://reviews.apache.org/r/23387/diff/
> 
> 
> Testing
> ---
> 
> Added qTests
> 
> 
> Thanks,
> 
> Ashish Singh
> 
>

Re: Review Request 23387: HIVE-6806: Native Avro support in Hive

2014-07-10 Thread Brock Noland


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23387/#review47568
---


I *love* this patch! Thank you so much.


serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java


Can we add some unit tests for this class?



serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java


Two thoughts:

1) Char/varchar support?
2) By defaulting to null won't any new types end up with null if this code 
is not updated? I think instead we should throw an exception for unknown types.


- Brock Noland


On July 10, 2014, 4:50 a.m., Ashish Singh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23387/
> ---
> 
> (Updated July 10, 2014, 4:50 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-6806
> https://issues.apache.org/jira/browse/HIVE-6806
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-6806: Native Avro support in Hive
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 
> 75394f3bc4f2285a7ced97ea90788d5bbff6b563 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
> 6cd1f39df3d419651755c35aab7cfc06833b16a4 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 
> 412a046488eaea42a6416c7cbd514715d37e249f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 
> f934ac4e3b736eed1b3060fa516124c67f9a2f87 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 
> 9c001c1495b423c19f3fa710c74f1bb1e24a08f4 
>   ql/src/test/queries/clientpositive/avro_compression_enabled_native.q 
> PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/avro_partitioned_native.q.out 
> PRE-CREATION 
>   serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java 
> PRE-CREATION 
>   serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 
> 1fe31e0034f8988d03a0c51a90904bb93e7cb157 
> 
> Diff: https://reviews.apache.org/r/23387/diff/
> 
> 
> Testing
> ---
> 
> Added qTests
> 
> 
> Thanks,
> 
> Ashish Singh
> 
>

Re: Review Request 23387: HIVE-6806: Native Avro support in Hive

2014-07-09 Thread Ashish Singh


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23387/
---

(Updated July 10, 2014, 4:50 a.m.)


Review request for hive.


Changes
---

Link hive jira.


Bugs: HIVE-6806
https://issues.apache.org/jira/browse/HIVE-6806


Repository: hive-git


Description
---

HIVE-6806: Native Avro support in Hive


Diffs
-

  ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 
75394f3bc4f2285a7ced97ea90788d5bbff6b563 
  ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
6cd1f39df3d419651755c35aab7cfc06833b16a4 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 
412a046488eaea42a6416c7cbd514715d37e249f 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 
f934ac4e3b736eed1b3060fa516124c67f9a2f87 
  ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 
9c001c1495b423c19f3fa710c74f1bb1e24a08f4 
  ql/src/test/queries/clientpositive/avro_compression_enabled_native.q 
PRE-CREATION 
  ql/src/test/queries/clientpositive/avro_decimal_native.q PRE-CREATION 
  ql/src/test/queries/clientpositive/avro_joins_native.q PRE-CREATION 
  ql/src/test/queries/clientpositive/avro_native.q PRE-CREATION 
  ql/src/test/queries/clientpositive/avro_partitioned_native.q PRE-CREATION 
  ql/src/test/results/clientpositive/avro_compression_enabled_native.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/avro_decimal_native.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/avro_joins_native.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/avro_partitioned_native.q.out PRE-CREATION 
  serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSchemaGenerator.java 
PRE-CREATION 
  serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerDe.java 
1fe31e0034f8988d03a0c51a90904bb93e7cb157 

Diff: https://reviews.apache.org/r/23387/diff/


Testing
---

Added qTests


Thanks,

Ashish Singh

59 matches

Mail list logo