laskoviymishka commented on code in PR #1371:
URL: https://github.com/apache/iceberg-go/pull/1371#discussion_r3553605427
##########
literals.go:
##########
@@ -174,6 +174,24 @@ func LiteralFromBytes(typ Type, data []byte) (Literal,
error) {
var v BinaryLiteral
err := v.UnmarshalBinary(data)
+ return v, err
+ case GeometryType, GeographyType:
Review Comment:
The validation and byte layout here are right, but the returned
`BinaryLiteral` reports `.Type() == Binary`, not Geometry/Geography. We're safe
today only because `evaluators.go` dispatches on `field.Type` rather than
`lit.Type()` — the moment anything keys off the literal's own type (pruning,
`.To()`, logging), it sees binary and routes through `bytes.Compare`, which
isn't a valid ordering over LE coords and gives silently-wrong answers instead
of erroring.
I'd rather not leave that as a doc comment asking every caller to check the
source type. Could we add a thin `GeoLiteral` wrapper (or parametrize
`BinaryLiteral`'s `Type()`) so `.Type()` is honest, and gate the
ordering-comparison paths to reject geo terms explicitly? wdyt?
--
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]
