ShivamSharma43 opened a new issue, #2457:
URL: https://github.com/apache/iceberg-go/issues/2457
### Apache Iceberg version
main (development)
### Please describe the bug 🐞
Converting a string literal to a decimal type rounds it to the type's scale
without any error. If a predicate's bound has more fractional digits than the
column's scale, the bound gets rounded, and the filter returns the wrong rows.
Reproduced on main (2e69b8a):
```go
dec92 := iceberg.MustDecimalTypeOf(9, 2)
lit, err := iceberg.NewLiteral("12.345").To(dec92)
fmt.Println(lit, err)
sc := iceberg.NewSchema(0,
iceberg.NestedField{ID: 1, Name: "price", Type: dec92, Required: true})
bound, err := iceberg.BindExpr(sc,
iceberg.GreaterThan(iceberg.Reference("price"), "12.345"), true)
fmt.Println(bound, err)
```
Output:
```
12.35 <nil>
BoundGreaterThan(term=BoundReference(field=1: price: required decimal(9, 2),
accessor=Accessor(position=0, inner=<nil>)), literal=12.35) <nil>
```
`price > 12.345` is bound as `price > 12.35`, so a row with `price = 12.35`
gets filtered out even though 12.35 > 12.345. In the same way, `price =
"12.345"` matches rows with 12.35 when it should match none. Decimal values in
expression JSON are always strings (spec Appendix D), so decimal filters parsed
with `ParseExpr` take this path too.
**Cause:** `StringLiteral.To` in `literals.go` calls arrow's
`decimal128.FromString(s, precision, scale)`. That function scales the value,
then adds or subtracts 0.5 and truncates, so any extra digits are rounded away.
**Other implementations:**
- Java's `StringLiteral.to(DECIMAL)` keeps the exact `BigDecimal` ("do not
change decimal scale"), so `12.35 > 12.345` evaluates correctly.
- PyIceberg's `StringLiteral.to(DecimalType)` raises `ValueError` when the
scales differ.
- This repo already avoids rounding in a similar spot:
`DecimalLiteral.integral` returns `ErrBadCast` rather than binding `qty <
12.34` as `qty < 12` (#2096).
**Proposed fix:** have `StringLiteral.To(DecimalType)` return `ErrBadCast`
when the string has more significant fractional digits than the scale. Values
that the scale represents exactly still convert as they do today (`"12.3"`
becomes `12.30`, `"12.340"` stays allowed), so callers that pass shorter
strings keep working.
I'd like to work on this. I have a fix and a unit test ready.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
