Skip to content

Fix ExprEval of BigDecimal data type#17889

Merged
gianm merged 1 commit intoapache:masterfrom
maytasm:fix_bigdecimal
Apr 8, 2025
Merged

Fix ExprEval of BigDecimal data type#17889
gianm merged 1 commit intoapache:masterfrom
maytasm:fix_bigdecimal

Conversation

@maytasm
Copy link
Contributor

@maytasm maytasm commented Apr 7, 2025

Fix ExprEval of BigDecimal data type

Description

This PR fixes a regression caused by #13947 (more specifically at https://github.com/apache/druid/pull/13947/files#diff-199493eb5b59a386d1fcade78da6578074cd94c80c114cfdd875a218106ca302L127 was changed to https://github.com/apache/druid/pull/13947/files#diff-5be9af304a8dda2062ca806bcc5320628757609c441fee5aec6e1ce51f71ded6R120)
Previously, we were just returning the raw but now we are using the method bestEffortOf which incorrectly handle BigDecimal data type. BigDecimal data type is incorrectly converted to Long. This issue was discovered with ingestion of Parquet files which use BigDecimal data type

Key changed/added classes in this PR

processing/src/main/java/org/apache/druid/math/expr/ExprEval.java

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

Copy link
Contributor

@gianm gianm left a comment

Choose a reason for hiding this comment

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

LGTM, seems like a reasonable thing to do for bestEffortOf, since there isn't a native big-decimal type.

@gianm gianm merged commit a02ebfb into apache:master Apr 8, 2025
76 checks passed
@capistrant capistrant added this to the 34.0.0 milestone Jul 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants