You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Sean Busbey <se...@manvsbeard.com> on 2012/10/04 00:51:25 UTC
Review Request: Unit tests to show failure to handle nullable complex types
on serialization
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7431/
-----------------------------------------------------------
Review request for hive.
Description
-------
Tests that express AvroSerDe's erroneous handling of Nullable complex types on serialization
Diffs
-----
/trunk/serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroSerializer.java 1393805
Diff: https://reviews.apache.org/r/7431/diff/
Testing
-------
Adds 7 tests that check each of the Avro types that Serialization needs to use a user-provided schema to handle.
Thanks,
Sean Busbey
Re: Review Request: HIVE-3528
Posted by Sean Busbey <se...@manvsbeard.com>.
> On Oct. 9, 2012, 1:06 a.m., Jakob Homan wrote:
> > /trunk/serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerializer.java, line 97
> > <https://reviews.apache.org/r/7431/diff/3/?file=174393#file174393line97>
> >
> > Good optimization on the null check. Any reason not to put it as the first check in the method and potentially save the trip to the AvroSerde methods?
As an aside, it wasn't an optimization. Since we obscure nullable fields, the null case gets handled by the same path as actual values. Without the check, the tests that make sure nullable records work explode with NPEs.
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7431/#review12257
-----------------------------------------------------------
On Oct. 16, 2012, 1:08 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7431/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2012, 1:08 p.m.)
>
>
> Review request for hive.
>
>
> Description
> -------
>
> Changes AvroSerDe to properly give the non-null schema to serialization routines when using Nullable complex types
>
>
> Diffs
> -----
>
> /trunk/serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerializer.java 1398763
> /trunk/serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroSerializer.java 1398763
>
> Diff: https://reviews.apache.org/r/7431/diff/
>
>
> Testing
> -------
>
> Adds tests that check each of the Avro types that Serialization needs to use a user-provided schema, both as top level fields and as nested members of a complex type.
>
>
> Thanks,
>
> Sean Busbey
>
>
Re: Review Request: HIVE-3528
Posted by Jakob Homan <jg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7431/#review12257
-----------------------------------------------------------
/trunk/serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerializer.java
<https://reviews.apache.org/r/7431/#comment26006>
May as well make this static as well.
/trunk/serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerializer.java
<https://reviews.apache.org/r/7431/#comment26007>
Good optimization on the null check. Any reason not to put it as the first check in the method and potentially save the trip to the AvroSerde methods?
- Jakob Homan
On Oct. 5, 2012, 8:39 a.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7431/
> -----------------------------------------------------------
>
> (Updated Oct. 5, 2012, 8:39 a.m.)
>
>
> Review request for hive.
>
>
> Description
> -------
>
> Changes AvroSerDe to properly give the non-null schema to serialization routines when using Nullable complex types
>
>
> Diffs
> -----
>
> /trunk/serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerializer.java 1394121
> /trunk/serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroSerializer.java 1394121
>
> Diff: https://reviews.apache.org/r/7431/diff/
>
>
> Testing
> -------
>
> Adds tests that check each of the Avro types that Serialization needs to use a user-provided schema, both as top level fields and as nested members of a complex type.
>
>
> Thanks,
>
> Sean Busbey
>
>
Re: Review Request: HIVE-3528
Posted by Jakob Homan <jg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7431/#review15161
-----------------------------------------------------------
Ship it!
+1. Looks great.
- Jakob Homan
On Jan. 2, 2013, 11:11 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7431/
> -----------------------------------------------------------
>
> (Updated Jan. 2, 2013, 11:11 p.m.)
>
>
> Review request for hive and Jakob Homan.
>
>
> Description
> -------
>
> Changes AvroSerDe to properly give the non-null schema to serialization routines when using Nullable complex types. Properly restores the enum-ness of Avro Enums prior to serialization.
>
>
> Diffs
> -----
>
> /trunk/data/files/csv.txt PRE-CREATION
> /trunk/ql/src/test/queries/clientpositive/avro_nullable_fields.q PRE-CREATION
> /trunk/ql/src/test/results/clientpositive/avro_nullable_fields.q.out PRE-CREATION
> /trunk/serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerializer.java 1426606
> /trunk/serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroDeserializer.java 1426606
> /trunk/serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroObjectInspectorGenerator.java 1426606
> /trunk/serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroSerializer.java 1426606
>
> Diff: https://reviews.apache.org/r/7431/diff/
>
>
> Testing
> -------
>
> Adds tests that check each of the Avro types that Serialization needs to use a user-provided schema, both as top level fields and as nested members of a complex type.
>
> Adds a client positive test that reads in a CSV table with NULLs, copies that data into an Avro backed table, then reads the data out of the table.
>
>
> Thanks,
>
> Sean Busbey
>
>
Re: Review Request: HIVE-3528
Posted by Sean Busbey <se...@manvsbeard.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7431/
-----------------------------------------------------------
(Updated Jan. 2, 2013, 11:11 p.m.)
Review request for hive and Jakob Homan.
Changes
-------
Added a clientpositive test for all nullable types. Subsumed HIVE-3538, with changes in anticipation of AVRO-997's stricter handling of enums.
Description (updated)
-------
Changes AvroSerDe to properly give the non-null schema to serialization routines when using Nullable complex types. Properly restores the enum-ness of Avro Enums prior to serialization.
Diffs (updated)
-----
/trunk/data/files/csv.txt PRE-CREATION
/trunk/ql/src/test/queries/clientpositive/avro_nullable_fields.q PRE-CREATION
/trunk/ql/src/test/results/clientpositive/avro_nullable_fields.q.out PRE-CREATION
/trunk/serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerializer.java 1426606
/trunk/serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroDeserializer.java 1426606
/trunk/serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroObjectInspectorGenerator.java 1426606
/trunk/serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroSerializer.java 1426606
Diff: https://reviews.apache.org/r/7431/diff/
Testing (updated)
-------
Adds tests that check each of the Avro types that Serialization needs to use a user-provided schema, both as top level fields and as nested members of a complex type.
Adds a client positive test that reads in a CSV table with NULLs, copies that data into an Avro backed table, then reads the data out of the table.
Thanks,
Sean Busbey
Re: Review Request: HIVE-3528
Posted by Sean Busbey <se...@manvsbeard.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7431/
-----------------------------------------------------------
(Updated Oct. 16, 2012, 1:08 p.m.)
Review request for hive.
Changes
-------
Updated per notes on review. Also fixed the access on the shared String Schema and added some comments to alleviate my nervousness about making it static.
Description
-------
Changes AvroSerDe to properly give the non-null schema to serialization routines when using Nullable complex types
Diffs (updated)
-----
/trunk/serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerializer.java 1398763
/trunk/serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroSerializer.java 1398763
Diff: https://reviews.apache.org/r/7431/diff/
Testing
-------
Adds tests that check each of the Avro types that Serialization needs to use a user-provided schema, both as top level fields and as nested members of a complex type.
Thanks,
Sean Busbey
Re: Review Request: HIVE-3528
Posted by Sean Busbey <se...@manvsbeard.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7431/
-----------------------------------------------------------
(Updated Oct. 5, 2012, 8:39 a.m.)
Review request for hive.
Changes
-------
Spun off nullable enums as HIVE-3538, because the issue appears to be String vs Arvo EnumSymbol and the changes involved were way more involved then what's here.
Description
-------
Changes AvroSerDe to properly give the non-null schema to serialization routines when using Nullable complex types
Diffs (updated)
-----
/trunk/serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerializer.java 1394121
/trunk/serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroSerializer.java 1394121
Diff: https://reviews.apache.org/r/7431/diff/
Testing
-------
Adds tests that check each of the Avro types that Serialization needs to use a user-provided schema, both as top level fields and as nested members of a complex type.
Thanks,
Sean Busbey
Re: Review Request: HIVE-3528
Posted by Sean Busbey <se...@manvsbeard.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7431/
-----------------------------------------------------------
(Updated Oct. 5, 2012, 3:23 a.m.)
Review request for hive.
Changes
-------
WIP solution. 1 failing case remains, nullable enums.
Summary (updated)
-----------------
HIVE-3528
Description (updated)
-------
Changes AvroSerDe to properly give the non-null schema to serialization routines when using Nullable complex types
Diffs (updated)
-----
/trunk/serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerializer.java 1394121
/trunk/serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroObjectInspectorGenerator.java 1394121
/trunk/serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroSerializer.java 1394121
Diff: https://reviews.apache.org/r/7431/diff/
Testing (updated)
-------
Adds tests that check each of the Avro types that Serialization needs to use a user-provided schema, both as top level fields and as nested members of a complex type.
Thanks,
Sean Busbey