Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AVRO] Support of Avro logical type Decimal. #542

Conversation

MichalFoksa
Copy link
Contributor

@MichalFoksa MichalFoksa commented Jan 2, 2025

Support of Avro logical type decimal. See Logical Types - Decimal specification.

@Decimal annotation instructs generator to create schema with logical type decimal in bytes Avro type. To create decimal in fixed type, the field has to be annotated also with @AvroFixedSize.
It only works when log

Limitations:

  • Exception is not thrown when other than BigDecimal filed is annotated with @Decimal. The @Decimal annotation is ignored.

BigDecimal serialization is delegated to Apache org.apache.avro.Conversions.DecimalConversion.
It validates whether value fits available schema. If not, then it throws org.apache.avro.AvroTypeException.

BigDecimal deserialization is taken from #133 . Deserialization is implemented in AvroParserImpl where decoding from bytes of fixed is delegated to child implementations (ApacheAvroParserImpl, resp JacksonAvroParserImpl).

Fixes #308 , #535.

@cowtowncoder
Copy link
Member

@MichalFoksa is this ready? If so, would need to convert from Draft to regular PR.

@MichalFoksa
Copy link
Contributor Author

Functionality is ready - it works.
I want to assert serialized value in BigDecimalTest.

Will let you know when done.

@cowtowncoder
Copy link
Member

Functionality is ready - it works. I want to assert serialized value in BigDecimalTest.

Will let you know when done.

Sounds good, thanks!

@MichalFoksa MichalFoksa marked this pull request as ready for review January 3, 2025 20:39
@MichalFoksa
Copy link
Contributor Author

@cowtowncoder PR is ready for review.

*/
@Target({ElementType.ANNOTATION_TYPE, ElementType.FIELD})
@Retention(RetentionPolicy.RUNTIME)
public @interface Decimal {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this should be "namespaced" as @AvroDecimal? Similar to existing @AvroNamespace.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Decimal renamed to @AvroDecimal.

Copy link
Member

@cowtowncoder cowtowncoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just one question/suggestion wrt naming of annotation.

@cowtowncoder
Copy link
Member

Fixes #132 , #308 , #535.

@MichalFoksa I can see #308 and #535 being fixed, but isn't #132 about Date/Time types?

@MichalFoksa
Copy link
Contributor Author

I think, issue #132 was originally about logical type support as whole. Now the issue is called Avro logical type support for Date/Time types. I do not know what is missing from Logical Types for Date/Time types.
It can be discussed in there.

I have removed #132 from "fixes" statement.

@cowtowncoder cowtowncoder merged commit 2ed9c3f into FasterXML:2.19 Jan 4, 2025
4 checks passed
@cowtowncoder
Copy link
Member

@MichalFoksa ah yes. I did close #132, since it does seem completed. Added two others in release notes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect serialization for LogicalType.Decimal (Java BigDecimal)
2 participants