Re: Need review/merges for couple of pull requests

2016-08-28 Thread S G
Hey Ryan, Any chance you were able to look into this (My email on Jun 6)? Thanks SG On Thu, Jun 16, 2016 at 2:05 PM, S G wrote: > Doug Cutting, Ryan Blue, > > I have worked with both of you on this ticket and would appreciate your > thoughts regarding circular references. > > If you think the

Re: Need review/merges for couple of pull requests

2016-06-16 Thread S G
Doug Cutting, Ryan Blue, I have worked with both of you on this ticket and would appreciate your thoughts regarding circular references. If you think the current circular references' implementation in Avro is indeed easy for clients to use (i.e. they do not have to write all those extra classes),

Re: Need review/merges for couple of pull requests

2016-06-06 Thread S G
As far as I have understood the attached testcase, I think the solution in https://github.com/apache/avro/pull/23/files (JIRA: https://issues.apache.org/jira/browse/AVRO-695) is more seamless from a user's perspective because the user does not have to write any classes extending LogicalType or Conv

Re: Need review/merges for couple of pull requests

2016-06-01 Thread S G
Thanks RB, But I am not sure I follow that example (Probably because there is no actual datum class there?). Consider a complex code running in production with lots of circular references. Something like: class Home { String address; Integer zipCode; List residents; } class Person {

Re: Need review/merges for couple of pull requests

2016-06-01 Thread Ryan Blue
Those aren't the datum classes, those are logical types that are added to the schema for your datum classes. The "referenceable" logical type is applied to the class that gets replaced with an ID reference and points to the field to use for that ID. The "reference" logical type is applied to the cl

Re: Need review/merges for couple of pull requests

2016-06-01 Thread S G
RB, The test TestCircularReferences shows that the classes having circular reference need to extend LogicalType. If every circular reference class has to do this, then I think this would be a big limitation for actual classes used in production code because they would be already extending other cl

Re: Need review/merges for couple of pull requests

2016-06-01 Thread Ryan Blue
SG, The example/test I built uses logical types to remove the circular reference when writing and restore it when reading. It doesn't look like your test adds logical types, so that's probably the issue. rb On Wed, Jun 1, 2016 at 1:51 PM, S G wrote: > Hey RB, > > I was trying out the circular

Re: Need review/merges for couple of pull requests

2016-06-01 Thread S G
Hey RB, I was trying out the circular refs with latest 1.8.1 version of Avro and it doesn't seem to be working out of the box for me. Perhaps I am missing something and would appreciate your help. Thanks, SG Here is my test code: public class CircularRefsTest { @Test public void testSe

Re: Need review/merges for couple of pull requests

2015-05-28 Thread Ryan Blue
SG, The data ends up looking like this: {"id":1,"p":"parent data!","child":{"c":"child data!","parent":{"long":1}}} I produced that with avro-tools 1.7.6 and tojson. Here's the schema: { "type" : "record", "name" : "Parent", "fields" : [ { "name" : "id", "type" : "long" }, {

Re: Need review/merges for couple of pull requests

2015-05-28 Thread S G
RB, Could you please attach the schema and the JSON serialized output from your test-code as well? My build environment is currently broken as I am grappling with some Java 8 update issues. Thanks SG On Wed, May 27, 2015 at 3:28 PM, Ryan Blue wrote: > SG, > > Now that logical types are in, I

Re: Need review/merges for couple of pull requests

2015-05-27 Thread Ryan Blue
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

Re: Need review/merges for couple of pull requests

2015-05-20 Thread S G
I am requesting some help with AVRO-695. Here are some pieces from the last conversation. Doug Cutting added a comment - 02/Oct/14 21:19 Here's a modified version of the patch. It moves all significant changes to reflect. Sinc

Re: Need review/merges for couple of pull requests

2015-04-30 Thread S G
Hey Ryan, Any chances we could atleast get the circular references patch in? (https://issues.apache.org/jira/browse/AVRO-695) It's been reviewed extensively by Doug Cutting already and my patch for circular references in Hive Avro SerDe has also been accepted in Hive. It would be good to have the

Re: Need review/merges for couple of pull requests

2015-03-25 Thread Ryan Blue
Hi S.G., The UUID issue is next on my list to review and I'm also planning on looking at the schema resolution problem in detail this week. Thanks for being patient on these, rb On 03/23/2015 10:15 AM, S G wrote: Hi, I have submitted patches/pull-requests for a couple of issues. Can someon

Need review/merges for couple of pull requests

2015-03-23 Thread S G
Hi, I have submitted patches/pull-requests for a couple of issues. Can someone please review/commit these contributions? 1) https://issues.apache.org/jira/browse/AVRO-695 Cycle Reference Support 2) https://issues.apache.org/jira/browse/AVRO-1569 (Bug fix) ReflectData.AllowNull fails with poly

Need review/merges for couple of pull requests

2015-03-06 Thread S G
Hi, I have submitted patches/pull-requests for couple of issues. Most of the review comments for these have also been fixed. Can someone please do the final reviews and help me commit the same? 1) https://issues.apache.org/jira/browse/AVRO-1568 Allow Java polymorphism in Avro for third-party cod