Skip to content

fix(literals): return long bounds for decimal conversion#3470

Merged
Fokko merged 1 commit into
apache:mainfrom
abnobdoss:fix/decimal-literal-long-bounds
Jun 9, 2026
Merged

fix(literals): return long bounds for decimal conversion#3470
Fokko merged 1 commit into
apache:mainfrom
abnobdoss:fix/decimal-literal-long-bounds

Conversation

@abnobdoss

@abnobdoss abnobdoss commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Closes #3469

Rationale for this change

Decimal literals converted to LongType should use long bound sentinels when the value is outside the long range. The existing conversion returned integer sentinels, mismatching the requested target type.

This returns LongAboveMax and LongBelowMin for decimal-to-long overflow.

Relationship to Java

Java's DecimalLiteral.to(...) only handles DECIMAL and returns null for other target types (Literals.java#L497-L505), so there is no direct decimal-to-long branch to mirror. This keeps PyIceberg's long conversion consistent with its typed integer overflow handling. Java's generic valueless overflow sentinels are a broader semantic difference and are out of scope here.

Are these changes tested?

Yes. New literal tests cover decimal values above and below the LongType range.

Are there any user-facing changes?

Yes. Decimal literal conversion to LongType now reports overflow with the correct long sentinel type.

@ebyhr ebyhr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The change makes sense to me. I've verified that the added tests are valid regression tests that fail without this PR's change.

@Fokko Fokko left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great catch @abnobdoss and thanks for the prompt review @ebyhr 🙌

@Fokko Fokko merged commit b231705 into apache:main Jun 9, 2026
16 checks passed
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.

DecimalLiteral.to(LongType) returns IntAboveMax instead of LongAboveMax

3 participants