AVRO-4165: [java] ability to specify AvroEncode on a class #3425
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Jul 5, 2025
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
opwvhk
requested changes
Jul 18, 2025
lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectionUtil.java
Outdated
Show resolved
Hide resolved
Comment on lines
1054
to
1079
| private CustomEncodingWrapper populateEncoderCache(Schema schema) { | ||
| var enc = ReflectionUtil.getAvroEncode(getClass(schema)); | ||
| if (enc != null) { | ||
| try { | ||
| return new CustomEncodingWrapper(enc.using().getDeclaredConstructor().newInstance()); | ||
| } catch (Exception e) { | ||
| throw new AvroRuntimeException("Could not instantiate custom Encoding"); | ||
| } | ||
| } | ||
| return new CustomEncodingWrapper(null); | ||
| } | ||
|
|
||
| private class CustomEncodingWrapper { | ||
|
|
||
| private final CustomEncoding customEncoding; | ||
|
|
||
| private CustomEncodingWrapper(CustomEncoding customEncoding) { | ||
| this.customEncoding = customEncoding; | ||
| } | ||
|
|
||
| public CustomEncoding get() { | ||
| return customEncoding; | ||
| } | ||
|
|
||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
I like this implementation! As you already mentioned in the PR, it's also suited for one of the superclasses. However, as the current use case for CustomEncoding is only from @AvroEncode, I think it's good as it is now.
Contributor
Author
|
@opwvhk thanks for the review. Have addressed the feedback. Question on process: Do you like conversations to be left for you to resolve? |
opwvhk
approved these changes
Jul 19, 2025
ashley-taylor
deleted the
AVRO-4165-ability-to-specify-AvroEncode-on-a-class
branch
opwvhk
pushed a commit
to opwvhk/avro
that referenced
this pull request
Sep 5, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds the ability to use the
CustomEncodeannotation directly on a class.It required some changes to when this annotation would be scanned. Currently, it is only used on fields.
Getting this to work for the first Record written required additional changes to support the feature.
I added a new map on
ReflectDataas I feel this is cleaner from an encapsulation point of view. Alternatively, it could be added toSpecifiedDataby changing theclassCachemap to an object that stores the class and thisCustomEncoding. This may be slightly faster, as there would be one less map lookup. However, it would probably be negligible as this new lookup is only performed once per row, whereas other maps can be queried per field.This change added tests and can be verified as follows:
Documentation