eneskeles commented on code in PR #2111:
URL: https://github.com/apache/iceberg-go/pull/2111#discussion_r4199572135
##########
table/transaction.go:
##########
@@ -2861,6 +2864,84 @@ func (t *Transaction) rewriteFilesWithFilter(ctx
context.Context, fs io.IO, upda
return nil
}
+// isNotTrueExpr returns an expression that holds exactly for the rows where
+// filter is not true, i.e. where it is false or NULL. Plain NOT(filter) is not
+// enough: under three-valued logic `v = 5` is NULL when v is NULL, and so is
+// NOT(v = 5), so a copy-on-write rewrite would drop rows that never matched.
+//
+// The result contains no NOT. Row-group pruning pushes a NOT down with
+// RewriteNotExpr, which turns NOT(x < 5) into x >= 5, and that is false for
+// NaN. Negating the predicates here keeps NaN rows in both places.
+func isNotTrueExpr(filter iceberg.BooleanExpression)
(iceberg.BooleanExpression, error) {
+ res, err := iceberg.VisitExpr(filter, isNotTrueVisitor{})
+ if err != nil {
+ return nil, err
+ }
+
+ return res.notTrue, nil
+}
+
+// truthExprs holds two never-NULL expressions for a sub-expression e:
+// notTrue holds iff e is false or NULL, notFalse holds iff e is true or NULL.
+type truthExprs struct {
+ notTrue, notFalse iceberg.BooleanExpression
+}
+
+type isNotTrueVisitor struct{}
+
+func (isNotTrueVisitor) VisitTrue() truthExprs {
+ return truthExprs{notTrue: iceberg.AlwaysFalse{}, notFalse:
iceberg.AlwaysTrue{}}
+}
+
+func (isNotTrueVisitor) VisitFalse() truthExprs {
+ return truthExprs{notTrue: iceberg.AlwaysTrue{}, notFalse:
iceberg.AlwaysFalse{}}
+}
+
+func (isNotTrueVisitor) VisitNot(child truthExprs) truthExprs {
+ return truthExprs{notTrue: child.notFalse, notFalse: child.notTrue}
+}
+
+func (isNotTrueVisitor) VisitAnd(left, right truthExprs) truthExprs {
+ return truthExprs{
+ notTrue: iceberg.NewOr(left.notTrue, right.notTrue),
+ notFalse: iceberg.NewAnd(left.notFalse, right.notFalse),
+ }
+}
+
+func (isNotTrueVisitor) VisitOr(left, right truthExprs) truthExprs {
+ return truthExprs{
+ notTrue: iceberg.NewAnd(left.notTrue, right.notTrue),
+ notFalse: iceberg.NewOr(left.notFalse, right.notFalse),
+ }
+}
+
+func (isNotTrueVisitor) VisitUnbound(pred iceberg.UnboundPredicate) truthExprs
{
+ negated := pred.Negate()
+ switch pred.Op() {
+ case iceberg.OpIsNull, iceberg.OpNotNull, iceberg.OpIsNan,
iceberg.OpNotNan:
+ // never evaluates to NULL
+ return truthExprs{notTrue: negated, notFalse: pred}
Review Comment:
Wow, thanks for the catch. This is now applied at e4b0394 with the
schema-evolution test.
--
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]
