henryf1991 commented on PR #2887:
URL: https://github.com/apache/avro/pull/2887#issuecomment-2113007382
> Sorry for barging into the discussion uninvited, but I am following this
PR, since I like the idea of such an annotation. If I understood the comments
on the original issue correctly, then the annotation should not only allow to
specify the namespace, but also the name of a record/enum - hence the name
`@AvroTypeName` for the annotation instead of `@AvroNamespace`. The following
suggestions are from Oscar's comment on the issue, I just adjusted the name of
the annotation:
>
> 1. `@AvroTypeName("simpleOrFullName")` for a named type would set its
name, optionally with namespace
> 2. `@AvroTypeName(value="simpleName", namespace="my.space")` for a named
type would set its name and namespace
> 3. `@AvroTypeName(namespace="my.space")` for a named type would set only
the namespace and let the name be inferred
>
> I guess this is what @opwvhk is referring to.
>
> On a related note: I really like the idea of this kind of annotation,
because namespace & name of a record schema can be independent from package &
name of the corresponding class. However, it does not end there: `ReflectData`
and `ReflectDatumReader` are not able to deserialize data when the schema of a
record/enum has a changed name or namespace. Currently, it assumes that the
class can be inferred from namespace & name of a record, see
`SpecificData.getClass(Schema)`, which is used by
`SpecificData.newRecord(Object, Schema)` (and `ReflectData` derives from
`SpecificData`). I stumbled upon this issue this week when trying to implement
an `@AvroNamespace` annotation myself.
>
> What solved this issue for me was to add a property to the generated
schema (`SpecificData.CLASS_PROP`, which is already used to specify classes for
Avro arrays) and override `ReflectData.getClass(Schema)` to read this property
(and cache the loaded class, just like
`ReflectData.getClassProp(Schema,String)`).
>
> I would appreciate an official solution, so I can throw away my code which
may break with new versions of the Avro library.
@ex-ratt Thanks for your suggestion and your comment was really helpful for
me to understand.
Regarding you point where we can extend the `@AvroTypeName` annotation
functionality to also override the name of the record, in my opinion there is
already `@AvroName` annotation that specially does this. Letting the users
override the record name using both `@AvroTypeName` and `@AvroName` might be
confusing and may also cause conflicts. Take the example below
`
@AvroTypeName(value = "OverridenAddressValue1")
class Address {
string address;
}
`
`
class Person {
@AvroName(value = "OverridenAddressValue2")
string address;
}
`
When the Person schema is generated, what should the name of the `Address`
record be, OverridenAddressValue1 or OverridenAddressValue2?
Hence I would rather have @AvroTypeName only applied at class level to only
override the namespace. Having thought about its better to rename @AvroTypeName
to @AvroNamespace. @opwvhk what are your thoughts on this?
Regarding your second point about deserialization failing when using the
annotation, that a good point, I l write a testcase to confirm and fix if
necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
