zeroshade commented on code in PR #1675:
URL: https://github.com/apache/iceberg-go/pull/1675#discussion_r3831819743
##########
exprs.go:
##########
@@ -790,7 +788,7 @@ type BoundLiteralPredicate interface {
func newBoundLiteralPredicate[T LiteralType](op Operation, term BoundTerm, lit
Literal) BoundPredicate {
return &boundLiteralPredicate[T]{
- op: op, term: term.(bound[T]),
+ op: op, term: term,
Review Comment:
**Blocking:** accepting a `BoundTransform` here exposes it to the metrics
and bloom evaluators, which call `term.Ref()` and compare the transformed
literal against raw source-column bounds/bloom values. For example,
`truncate[3](category) == "boo"` with a file containing `"books"` is
incorrectly pruned because the inclusive metrics evaluator compares raw
`"books"` with `"boo"`, even though the transformed predicate matches. This can
skip matching files during scans and dynamic overwrite. Please make all
source-column pruning evaluators conservatively handle transformed terms (or
correctly transform metrics where safe); strict metrics and bloom pruning need
the same audit.
##########
exprs.go:
##########
@@ -838,7 +836,7 @@ func createBoundLiteralPredicate(op Operation, term
BoundTerm, lit Literal) (Bou
type boundLiteralPredicate[T LiteralType] struct {
op Operation
- term bound[T]
+ term BoundTerm
Review Comment:
**Blocking:** the Substrait row-filter converter is not transform-aware. It
converts predicates through `Term().Ref()` and therefore emits
`equal(.field(0), "boo")` for `truncate[3](category) == "boo"`, silently
dropping the transform. Partial-file rewrites can consequently retain rows that
should be replaced, and bucket/time transforms may also produce type-invalid
filters. Please encode the transform in Substrait or reject transformed
predicates on this path rather than changing their semantics.
--
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]
