You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by GitBox <gi...@apache.org> on 2021/08/13 11:05:58 UTC
[GitHub] [avro] Dieken opened a new pull request #1304: AVRO-3186: correctly parse union of logical type in specific record
Dieken opened a new pull request #1304:
URL: https://github.com/apache/avro/pull/1304
… logical type in specific record
SpecificDatumReader.SpecificDatumReader(Schema writer, Schema reader)
-> SpecificData.getForSchema(reader) // It breaks on JDK 9+, fails to load generated class,
// thus return an empty SpecificData.
SpecificDatumReader.readField()
-> datum = readWithoutConversion(oldDatum, field.schema(), in); // conversion is null for union field
-> GenericDatumReader.read(Object old, Schema expected, ResolvingDecoder in)
-> Conversion<?> conversion = getData().getConversionFor(logicalType); // return null wrongly
--
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: dev-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] Dieken commented on pull request #1304: AVRO-3186: correctly parse union of logical type in specific record
Posted by GitBox <gi...@apache.org>.
Dieken commented on pull request #1304:
URL: https://github.com/apache/avro/pull/1304#issuecomment-903888380
> Hello! Thanks for the contribution, but I couldn't reproduce the failure running on JDK 11 -- am I missing something?
I encounter this problem on Quarkus(https://quarkus.io), I guess `quarkus dev` loads libraries and application classes with different class loaders.
I guess this guy has same cause: https://stackoverflow.com/questions/63642870/avro-is-not-able-to-deserialize-union-with-logical-types-in-fields
The patch is actually pretty obvious, it doesn't introduce new code, the correct code is already in the same class:
https://github.com/apache/avro/blob/release-1.10.2/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificData.java#L236
SpecificData.getClass() uses `ClassUtils.forName`, this is correct for multi class loader scenario.
https://github.com/apache/avro/blob/release-1.10.2/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificData.java#L141
SpecificData.getForSchema() uses `Class.forName()`, this is well-known buggy logic, `ClassUtils` does the right thing: https://github.com/apache/avro/blob/release-1.10.2/lang/java/avro/src/main/java/org/apache/avro/util/ClassUtils.java#L26
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] Dieken commented on pull request #1304: AVRO-3186: correctly parse union of logical type in specific record
Posted by GitBox <gi...@apache.org>.
Dieken commented on pull request #1304:
URL: https://github.com/apache/avro/pull/1304#issuecomment-907583829
@RyanSkraba
I just added a unit test, the test is meant for org.apache.avro:avro but it depends on SpecificCompiler, so I have to put it into org.apache.avro:avro-compiler.
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] Dieken commented on pull request #1304: AVRO-3186: correctly parse union of logical type in specific record
Posted by GitBox <gi...@apache.org>.
Dieken commented on pull request #1304:
URL: https://github.com/apache/avro/pull/1304#issuecomment-903313240
Anybody there to review and merge this obvious patch?
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] RyanSkraba commented on pull request #1304: AVRO-3186: correctly parse union of logical type in specific record
Posted by GitBox <gi...@apache.org>.
RyanSkraba commented on pull request #1304:
URL: https://github.com/apache/avro/pull/1304#issuecomment-906483121
Thanks for the info -- having Avro running correctly in Quarkus is a compelling reason to get this fix in! I've updated the JIRA to be included in the next major release (which is hopefully soon). I've got some things on my plate at the moment, so if anyone wants to jump in with a review, feel free!
Is there a way to unit test for Quarkus compatibility, or at least demonstrate the symptom on master?
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] Dieken commented on pull request #1304: AVRO-3186: correctly parse union of logical type in specific record
Posted by GitBox <gi...@apache.org>.
Dieken commented on pull request #1304:
URL: https://github.com/apache/avro/pull/1304#issuecomment-919240836
@RyanSkraba any further comment? I'm waiting for a new 1.10.x or just 1.11 release :-)
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] RyanSkraba merged pull request #1304: AVRO-3186: correctly parse union of logical type in specific record
Posted by GitBox <gi...@apache.org>.
RyanSkraba merged pull request #1304:
URL: https://github.com/apache/avro/pull/1304
--
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: dev-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] RyanSkraba commented on pull request #1304: AVRO-3186: correctly parse union of logical type in specific record
Posted by GitBox <gi...@apache.org>.
RyanSkraba commented on pull request #1304:
URL: https://github.com/apache/avro/pull/1304#issuecomment-903830646
Hello! Thanks for the contribution, but I couldn't reproduce the failure running on JDK 11 -- am I missing something?
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] Dieken commented on pull request #1304: AVRO-3186: correctly parse union of logical type in specific record
Posted by GitBox <gi...@apache.org>.
Dieken commented on pull request #1304:
URL: https://github.com/apache/avro/pull/1304#issuecomment-905091596
@RyanSkraba could you help to get this merged? I hope this can be picked by confluent kafka-avro-serializer and Quarkus as soon as possible, it's very inconvenient to have my local patched avro jar.
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org