tanmayrauth commented on code in PR #1312:
URL: https://github.com/apache/iceberg-go/pull/1312#discussion_r3470843851
##########
literals.go:
##########
@@ -1246,12 +1246,16 @@ func (DecimalLiteral) Comparator() Comparator[Decimal] {
}
}
-// Type returns a DecimalType built from the literal's scale and a hardcoded
-// precision of 9. The precision is NOT the originating column's declared
-// precision; DecimalLiteral does not carry precision. Callers that need the
-// real column precision must consult the bound field's type rather than
-// lit.Type(). See https://github.com/apache/iceberg-go/issues/1028.
-func (d DecimalLiteral) Type() Type { return DecimalTypeOf(9, d.Scale) }
+// Type returns a DecimalType built from the literal's scale and a placeholder
+// precision that is at least 9. The precision is NOT the originating column's
+// declared precision; DecimalLiteral does not carry precision. Callers that
+// need the real column precision must consult the bound field's type rather
+// than lit.Type(). See https://github.com/apache/iceberg-go/issues/1028.
+func (d DecimalLiteral) Type() Type {
+ precision := max(9, d.Scale)
+
+ return DecimalTypeOf(precision, d.Scale)
Review Comment:
Good catch using `max(9, d.Scale)` here — without it the new precision/scale
validation would panic for literals with scale > 9.
The same reasoning seems to apply to `FixedLiteral.Type()` at line 1122,
which still does `FixedTypeOf(len(f))`. A zero-length fixed literal (e.g. the
zero value, or `UnmarshalBinary` with empty data) now panics there instead of
returning `FixedType{0}` as it used to. It's an unusual input so maybe not
worth much, but since you guarded the decimal side, was leaving the fixed side
intentional? A `len(f) == 0` guard (or just documenting it) would keep the two
paths consistent.
--
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]
