avro-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ryan Blue <b...@cloudera.com>
Subject Re: Need review/merges for couple of pull requests
Date Wed, 27 May 2015 22:28:03 GMT
SG,

Now that logical types are in, I had some time to look at this issue. 
Thanks for your patience on this.

When I started looking at the use case, this began to look very much 
like a logical type issue. (I know, I've been saying that a lot.) When 
you write, you replace any referenced object with its id. When you read, 
you replace those ids with the correct object. I went ahead and 
implemented this using 2 logical types: Referenceable for an object with 
an id, and Reference for a record with a field that references another 
object by id.

There were some trade-offs to this approach. For example, I had to use a 
logical type for the object containing the reference rather than for the 
reference itself because I used a callback to set the object once it is 
found. That happens because children with references to a parent are 
deserialized completely first. The parent is the last object to be 
assembled and passed to the logical type conversion.

A bigger issue was that logical types are currently conservative and 
don't overlap with reflect types or anything that sets java-class. That 
means that this currently only works with generic types. But, I'd rather 
make logical types work for reflect than add more custom code to support 
this. Does that sound reasonable?

I'm attaching a diff with the working test code so you can take a look 
at the approach. Let me know what you are thinking.

rb

On 05/20/2015 12:05 PM, S G wrote:
> I am requesting some help with AVRO-695.
> Here are some pieces from the last conversation.
>
>
> Doug Cutting
> <https://issues.apache.org/jira/secure/ViewProfile.jspa?name=cutting> added
> a comment - 02/Oct/14 21:19
>
> Here's a modified version of the patch. It moves all significant changes to
> reflect. Since reflect is the only implementation that can generate an
> appropriate schema, changes should be confined to there.
>
> The tests need to be updated, as they still assume generic.
> <https://issues.apache.org/jira/browse/AVRO-695#>
> <https://issues.apache.org/jira/browse/AVRO-695?focusedCommentId=14286370&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14286370>
> Sachin Goyal
> <https://issues.apache.org/jira/secure/ViewProfile.jspa?name=sachingoyal> added
> a comment - 21/Jan/15 21:56
>
> Here is a patch with the updated test-cases.
> I also confirm that all my changes are there in the patch.
>
>
> The patch was submitted in June 2014 and was very hot till October 2014.
> Since then, there has been no action on this even though I have sent many
> reminders in this group.
>
> I understand that everyone is very busy with their own stuff but I would
> really appreciate if someone could help a fellow engineer in getting his
> patch accepted.
> It would encourage more participation as well as help people wanting to use
> Avro for circular references.
>
> Regards
> SG


-- 
Ryan Blue
Software Engineer
Cloudera, Inc.

Mime
View raw message