tanmayrauth commented on issue #1028:
URL: https://github.com/apache/iceberg-go/issues/1028#issuecomment-4502007526
Honestly my opinion will be Option B - fix the known broken consumer,
document the limitation.
The confirmed impact is narrow: only the Substrait conversion at
substrait.go:249 actually reads DecimalLiteral.Type().Precision(). A breaking
API change (Option A) to fix one caller is overkill. Fix toDecimalLiteral to
take precision from the bound field's DecimalType instead of v.Type(), add a
doc comment on Type() noting the precision caveat, and we're done.
Also worth fixing on that same line: v.Type().(*iceberg.DecimalType) is a
pointer type assertion on a value type - that'll panic at runtime. Should be
v.Type().(iceberg.DecimalType).
If more affected call sites turn up later, Option A is still on the table
- and easier to justify with multiple concrete motivations.
@zeroshade @laskoviymishka - thoughts?
--
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]
