Uh oh!
There was an error while loading. Please reload this page.
feat(expression): cast numeric literals to decimal type - #805
Conversation
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
e5e3610 to
09a45b6CompareThere was a problem hiding this comment.
Pull request overview
This PR extends Literal::CastTo to support casting numeric literals (int, long, float, double) into a decimal target type, aligning C++ literal-cast behavior with Java’s numeric-to-decimal default handling. It adds new decimal-cast logic for integer and real sources and introduces targeted unit tests to validate scaling, rounding, and rejection behavior.
Changes:
- Add integer→decimal and real→decimal cast paths in
LiteralCaster, including HALF_UP rounding behavior. - Add
RescaleHalfUphelper to support HALF_UP rounding (including negative target scales) while avoiding unsafe power-of-ten indexing in some cases. - Add new tests covering integer/real decimal casts, scale handling, rounding behavior, and error cases.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/iceberg/expression/literal.cc | Adds numeric→decimal casting logic and a HALF_UP rescaling helper used by Literal::CastTo. |
| src/iceberg/test/literal_test.cc | Adds unit tests validating integer/real casts to decimal, rounding rules, and error handling. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Validate target scale bounds, avoid int128 overflow in HALF_UP by comparing against divisor/2, and use std::to_chars for shortest round-trip conversion.
| int32_t parsed_scale = 0; | ||
| ICEBERG_ASSIGN_OR_RAISE( | ||
| auto parsed, | ||
| Decimal::FromString(std::string_view(buf.data(), ptr), nullptr, &parsed_scale)); |
There was a problem hiding this comment.
Decimal::FromString normalizes negative scales to 0 before this rescale, which can overflow its int128 intermediate. For example, Literal::Double(4e38).CastTo(decimal(38, 0)) may wrap 4 * 10^38 to a 38-digit value and accept the wrong literal instead of rejecting it. It also rejects representable negative-scale cases such as 1e39 -> decimal(2, -38). Could we retain the parsed coefficient and exponent scale, rescale directly to the target scale, and add regressions for both cases?
There was a problem hiding this comment.
Good catch. Now I parse the coefficient and its scale directly from the to_chars output (via a small ParseRealCoefficient helper) instead of going through Decimal::FromString, which was normalizing the negative scale by multiplying the coefficient by 10^-scale and overflowing int128. The coefficient itself always fits, and RescaleHalfUp combines the exponent with the target scale and rejects true overflow. Added regressions for both 4e38 -> decimal(38, 0) (rejected) and 1e39 -> decimal(2, -38) (accepted as 10).
…nitude decimal casts
What
Add casting of numeric literals (
int,long,float,double) to a decimal target type inLiteral::CastTo, so a numeric value can be used as a default for a decimal column.Previously
CastFromInt/CastFromLong/CastFromFloat/CastFromDoublehad nokDecimalcase and fell through toNotSupported, so a default likeLiteral::Int(12)orLiteral::Double(9.99)for adecimal(9, 2)column was rejected. Java allows these (IntegerLiteral.to,DoubleLiteral.to, etc. scale the value to the target scale), so this brings the C++ literal cast layer to parity for numeric sources.How
CastIntegerToDecimalscales the integer (scale 0) up to the target scale viaDecimal::Rescale(0, scale), then verifies the result fits the target precision (FitsInPrecision). Example:12→decimal(9,2)yields unscaled1200(12.00).CastRealToDecimalparses the value's shortest round-tripping decimal representation (matching Java'sBigDecimal.valueOf(double)viaDouble.toString), then rounds to the target scale with HALF_UP rounding (round half away from zero, as Java does —2.5→3,-2.5→-3), and checks precision. Non-finite values are rejected.DecimalTypedoes not bound its scale on construction, sodecimal(9, 40)would otherwise read past the table).Scope
Follow-up split out from the v3 default-value work (see the
CastDefaultToTypediscussion on #793). It only extends the sharedLiteral::CastTolayer for numeric sources.Testing
LiteralTest.IntegerCastToDecimal(int/long scaling, out-of-precision rejection, out-of-range scale rejection) andLiteralTest.RealCastToDecimal(float/double scaling, HALF_UP rounding incl. negative, round-down, out-of-precision and non-finite rejection), all verified fail-without / pass-with. Fullexpression_testpasses (495 tests).