Re: Review Request 24609: Hive-7653: AvroSerDe does not support circular references in Schema

[Sachin Goyal <sg...@gmail.com>]

> On Nov. 11, 2014, 5:34 a.m., Szehon Ho wrote:
> > serde/src/java/org/apache/hadoop/hive/serde2/avro/SchemaToTypeInfo.java, line 170
> > <https://reviews.apache.org/r/24609/diff/1/?file=658578#file658578line170>
> >
> >     This is probably moot if we can use a Set (see below), but why IdentityHashMap?  Schema has hashcode defined.

Avro hashCode() for records takes all its elements hash-code.
For a circular schema, the hash-code will be different when cycle has been populated vs when the cycle has not been populated.
(Even when its the same object but hashcode is calculated at different times.)

Identity hash-map would be a little more efficient as well.


- Sachin


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


On Aug. 12, 2014, 4:35 p.m., Sachin Goyal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24609/
> -----------------------------------------------------------
> 
> (Updated Aug. 12, 2014, 4:35 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/HIVE-7653
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/HIVE-7653
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Hi,
> 
> I have submitted a patch for the following issue:
> https://issues.apache.org/jira/browse/HIVE-7653
> 
> But the build is failing due to some other issue.
> Its been failing for the past 70 builds or so and I don't think its related to my change.
> Also, my local build for the same is passing.
> 
> Can someone please help me override/fix this test-failure?
> 
> Also, a code review of the above patch would be much appreciated.
> 
> Thanks
> Sachin
> 
> 
> Diffs
> -----
> 
>   serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroDeserializer.java 688b072 
>   serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroObjectInspectorGenerator.java 46cdb4f 
>   serde/src/java/org/apache/hadoop/hive/serde2/avro/AvroSerializer.java 2bd48ca 
>   serde/src/java/org/apache/hadoop/hive/serde2/avro/InstanceCache.java d848005 
>   serde/src/java/org/apache/hadoop/hive/serde2/avro/SchemaToTypeInfo.java 23e024f 
>   serde/src/test/org/apache/hadoop/hive/serde2/avro/TestAvroSerializer.java f8161da 
>   serde/src/test/org/apache/hadoop/hive/serde2/avro/TestInstanceCache.java 1df88ee 
> 
> Diff: https://reviews.apache.org/r/24609/diff/
> 
> 
> Testing
> -------
> 
> All tests pass.
> Also added a new unit-test for the patch.
> 
> 
> Thanks,
> 
> Sachin Goyal
> 
>