AVRO-2918: Polymorphism #1776
AVRO-2918: Polymorphism #1776
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| final Schema realSchema; | ||
| if (expected.hasChild()) { | ||
| int index = in.readExtends(); | ||
| realSchema = expected.findInHierachy(index); | ||
| } else { | ||
| realSchema = expected; | ||
| } |
There was a problem hiding this comment.
This seems to be at odds with GenericDatumWriter, lines 239-241: there, the extends is optional, based on whether the parent schema or one of its children is written.
There was a problem hiding this comment.
indeed, i have to add unit test and fix that.
There was a problem hiding this comment.
You were right, so now writer will always write the "extends" symbol if schema has child.
| if (!Objects.equals(realSchema, schema)) { | ||
| out.writeExtends(realSchema.getIndex()); | ||
| } |
There was a problem hiding this comment.
This creates two possible binary representations of the same (parent) schema: one for writing a parent schema, and a different one for child schemata.
How can the reader distinguish between the two?
| @@ -178,6 +178,11 @@ public void writeIndex(int unionIndex) throws IOException { | |||
| encodeLong(unionIndex, out); | |||
| } | |||
|
|
|||
| @Override | |||
| public void writeExtends(int extendsIndex) throws IOException { | |||
| encodeLong(extendsIndex, out); | |||
There was a problem hiding this comment.
Should the extends clause be supported by the legacy encoder at all?
Or should it throw?
There was a problem hiding this comment.
I don't know ... i can throw an exception ....
| @@ -345,6 +379,9 @@ public String getFullName() { | |||
| return getName(); | |||
| } | |||
|
|
|||
| public void terminateInit() { | |||
There was a problem hiding this comment.
What does this method do?
I understand methods like hasChild, visitHierarchy and findInHierarchy above (though they also need javadoc), but I cannot understand the what&why of this one.
There was a problem hiding this comment.
It's a kind of PostConstruct, for SchemaRecord, once all schemas has been built and list of fields set.
But, indeed, i will see if we can do without it
There was a problem hiding this comment.
Ah, sort of like how the fields are marked immutable once set for the first time? (and any field use before they are set yields an exception)
How about this idea:
- in the constructor, mark if the record schema will have children (if the parameter is omitted, it will not)
- have a method where you can set all child schemata, once
This behaves like setFields: it allows us both to finalise initialisation and makes the schema unusable for (de)serialisation until the child schemata have been set (so it cannot be forgotten).
There was a problem hiding this comment.
So, i finally find an easy way to remove this weird method.
There was a problem hiding this comment.
I saw; lazy initialisation. Also a good idea.
clesaec
force-pushed
the
avro-2918_Polymorphism
branch
from
9afb5d3 to
c704d9a
Compare
clesaec
force-pushed
the
avro-2918_Polymorphism
branch
from
c704d9a to
0796783
Compare
clesaec
force-pushed
the
avro-2918_Polymorphism
branch
from
0796783 to
ed665ae
Compare
clesaec
force-pushed
the
avro-2918_Polymorphism
branch
from
ec3bfc1 to
bb84a43
Compare
clesaec
force-pushed
the
avro-2918_Polymorphism
branch
from
bb84a43 to
9d3b24c
Compare
clesaec
force-pushed
the
avro-2918_Polymorphism
branch
from
9d3b24c to
5d8ab89
Compare
clesaec
force-pushed
the
avro-2918_Polymorphism
branch
from
5d8ab89 to
050fb06
Compare
clesaec
force-pushed
the
avro-2918_Polymorphism
branch
from
050fb06 to
6c8ddae
Compare
AVRO-2918 : Proposition for polymorphism