Code written for generic records should be directly applicable on equivalent specific records.
This patch updates SpecificRecordBase to implement GenericRecord.
Patch looks good but also includes changes for AVRO-1295.
Sorry for that, here is an updated diff against the latest trunk.
+1 This looks good to me. I'll commit it soon unless there are objections.
I'm uncomfortable with the mutator here. (put) This may make immutable records impossible in the future. Currently the only mutators are generated from the template, can these be optional from the compile template as well? Or can we break up the generic record interface into read/write bits and only expose the accessor by default?
We can disable the put() here (ie. let it throw a runtime exception).
Should the same treatment happen with IndexedRecord.put() implemented by SpecificRecordBase then?
My previous question is actually irrelevant at this point. Is there documentation or a plan sketched as to how to bring immutable records?
What's the timeline to remove setters?
Here is a patch in which put() is not supported (throws an AvroRuntimeException).
We can also explore splitting the GenericRecord interface into getter and setter interfaces.
I suppose IndexedRecord exists, and this is no different.
The work required for immutable records is some cleanup and performance improvement to the builders (e.g. remove unnecessary boxing), then those need to be used by the DatumReader classes.
There is no timeline, but I want to make sure we avoid making things harder in the future if we can and consider our options.
I don't yet see how implementing set() in SpecificRecordBase now makes implementing immutable records impossible in the future. If this is called on an immutable record then it should throw an exception, but not on a mutable record. What am I missing?
I guess only my sense of purity – an Immutable object that throws an exception on set feels very dirty and a violation of the contract. At this point, we already have a put() via IndexedRecord interface so I don't see any harm in this.
I think we agree. If we want to improve things for immutability then we might refactor the interfaces, but throwing an exception for mutable records seems like it accomplishes little. So my vote is for the 101039 version of the patch.
I agree, 101039 seems best. If we didn't already have IndexedRecord.put(), I might want to split the interfaces up now, but there is no reason to roll that in in this JIRA as is.
Note – it will probably be slower to serialize a SpecificRecord via its GenericRecord interface, but probably only a little since there is quite a bit of overhead in both at the moment anyway.
I committed this. Thanks, Christophe!
Integrated in AvroJava #364 (See https://builds.apache.org/job/AvroJava/364/)
AVRO-1299. Java: SpecificRecordBase implements GenericRecord. Contributed by Christophe Taton. (Revision 1471047)
Result = SUCCESS