Skip to content

Bound plain-notation expansion of DECIMAL literals to prevent parse-time OutOfMemoryError - #5223

Open
shoemoney wants to merge 1 commit into
apache:mainfrom
shoemoney:fix/calcite-checkoverflow-bounded
Open

Bound plain-notation expansion of DECIMAL literals to prevent parse-time OutOfMemoryError#5223
shoemoney wants to merge 1 commit into
apache:mainfrom
shoemoney:fix/calcite-checkoverflow-bounded

Conversation

@shoemoney

Copy link
Copy Markdown

Sibling of CALCITE-7731.

Bug: Primitive.checkOverflow calls BigDecimal.toPlainString on a value with scale < 0 without bounding the plain-notation expansion. A large negative scale can force toPlainString to materialize gigabytes and OOM, same pattern bounded elsewhere in CALCITE-7731.

Fix: Gate the toPlainString call with isBoundedDecimal. This mirrors SqlUtil.isBoundedDecimal in core but is inlined in linq4j to avoid a circular linq4j -> core dependency. The bound reads calcite.parser.maxDecimalLiteralPlainDigits (default 10000) and checks precision + abs(scale) <= limit. If exceeded, throw IllegalArgumentException before allocation.

Evidence: RED->GREEN verified. Both :linq4j:compileJava configurations succeed. Formatter blast radius is one file.

@mihaibudiu

Copy link
Copy Markdown
Contributor

The linter won't like your commit message. Have you tried to validate this locally?
Maybe you should just use 7731 again in the commit message

…Primitive.checkOverflow

Primitive.checkOverflow calls BigDecimal.toPlainString on a value with
scale < 0 without bounding the plain-notation expansion, the same
pattern bounded elsewhere in CALCITE-7731. A large negative scale can
force toPlainString to materialize gigabytes and OOM.

Gate the toPlainString call with isBoundedDecimal, mirroring
SqlUtil.isBoundedDecimal in core but inlined in linq4j to avoid a
circular linq4j -> core dependency. The bound reads
calcite.parser.maxDecimalLiteralPlainDigits (default 10000) and checks
precision + abs(scale) <= limit, throwing IllegalArgumentException
before allocation if exceeded.
@shoemoney
shoemoney force-pushed the fix/calcite-checkoverflow-bounded branch from c9a1bd9 to c9ed2c6 Compare August 27, 2026 20:25
@shoemoney

Copy link
Copy Markdown
Author

Fixed, commit message now uses [CALCITE-7731] in c9ed2c6.

@mihaibudiu mihaibudiu changed the title fix(calcite): gate checkOverflow toPlainString with isBoundedDecimal Bound plain-notation expansion of DECIMAL literals to prevent parse-time OutOfMemoryError Aug 27, 2026
@mihaibudiu

Copy link
Copy Markdown
Contributor

I have changed your PR title to match

@mihaibudiu
mihaibudiu requested a review from rubenada August 27, 2026 20:28
@rubenada

Copy link
Copy Markdown
Contributor

Is this really needed? Can we get into a "risky" scenario here, considering the checks introduced in CALCITE-7731 ?

@sonarqubecloud

Copy link
Copy Markdown

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.

3 participants