[CALCITE-7731] Bound plain-notation expansion of DECIMAL literals to prevent parse-time OutOfMemoryError

[rubenada]

Jira Link

CALCITE-7731

Changes Proposed

BigDecimal accepts any int exponent, so a DECIMAL literal such as DECIMAL '1E2147483647' (~12 characters) parses to a BigDecimal whose plain-notation form would be one character per digit: a multi-gigabyte allocation (potentially an OOM error).

Three places call BigDecimal.toPlainString() on a value derived from user-supplied input and would attempt that allocation:

SqlParserUtil.parseDecimalLiteral
SqlNumericLiteral.toValue
RexBuilder.makeLiteral

It is required to add a check in there to prevent an OOM error.

[mihaibudiu]

Not easy to tell by eye whether all the places where the conversion happens have been instrumented.

I wonder whether the exception message size is a concern. Is the problem only when the original literal is small but the internal representation is large?

[mihaibudiu]

could this error message be very large too?

[rubenada]

Yes, well spotted. I have changed it into bd.toString() , which should be the less OOM-risky alternative.

[rubenada]

Not easy to tell by eye whether all the places where the conversion happens have been instrumented.

I wonder whether the exception message size is a concern. Is the problem only when the original literal is small but the internal representation is large?

The problem is mostly when toPlainString method is used, whose OOM risk can be quite high.
I have searched manually the callers of this method, and I think the PR covers all places susceptible to have a potential OOM risky input value from the user. There was another occurrence in SqlImplementor, I've just included the check in there just in case.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
81.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[skrcode]

@rubenada — I ran JAIPilot Cloud against an earlier exact head of this now-merged PR. It produced an AI-generated test-only draft covering the three DECIMAL plain-notation bound guards that lacked direct tests: skrcode#4

The cloud run passed the focused tests and 16,649-test core build. Since the original PR is merged, this would need to be rebased as a follow-up if the coverage is still useful. Nothing is auto-merged.

[rubenada]

@skrcode you're right, initially I planned to apply this patch at SqlParser level only (which seems the more likely scenario for a risky user-defined input leading to this potential OOM situation), but finally I decided to include the check also in other (potentially risky) places calling BigDecimal#toPlainString (but did not add tests for them).
You can open a separate, follow-up PR to add the extra tests to improve our test coverage. We'd appreciate it.

[skrcode]

Thanks @rubenada — I opened #5212 with the three follow-up regression tests: #5212

It is rebased on current main and contains test changes only. The focused tests, full core build (16,658 tests with zero failures), and repository-wide build all pass locally; Apache Jenkins is also running its independent check.