Skip to content

Commit

Permalink
SQL null semantics for and, or, not (#5502)
Browse files Browse the repository at this point in the history
Closes #5465
  • Loading branch information
mattnibs authored Nov 26, 2024
1 parent f030936 commit f166ba9
Show file tree
Hide file tree
Showing 13 changed files with 393 additions and 211 deletions.
14 changes: 7 additions & 7 deletions compiler/kernel/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,8 @@ func (f *DeleteFilter) AsEvaluator() (expr.Evaluator, error) {
return nil, nil
}
// For a DeleteFilter Evaluator the pushdown gets wrapped in a unary !
// expression so we get all values that don't match. We also add a missing
// call so if the expression results in an error("missing") the value is
// kept.
// expression so we get all values that don't match. We also add an error
// and null check because we want to keep these values around.
return f.builder.compileExpr(&dag.BinaryExpr{
Kind: "BinaryExpr",
Op: "or",
Expand All @@ -47,10 +46,11 @@ func (f *DeleteFilter) AsEvaluator() (expr.Evaluator, error) {
Op: "!",
Operand: f.pushdown,
},
RHS: &dag.Call{
Kind: "Call",
Name: "missing",
Args: []dag.Expr{f.pushdown},
RHS: &dag.BinaryExpr{
Kind: "BinaryExpr",
Op: "or",
LHS: &dag.IsNullExpr{Kind: "IsNullExpr", Expr: f.pushdown},
RHS: &dag.Call{Kind: "Call", Name: "is_error", Args: []dag.Expr{f.pushdown}},
},
})
}
Expand Down
3 changes: 1 addition & 2 deletions lake/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,7 @@ func filter(zctx *super.Context, ectx expr.Context, this super.Value, e expr.Eva
if e == nil {
return true
}
val, ok := expr.EvalBool(zctx, ectx, this, e)
return ok && val.Bool()
return expr.EvalBool(zctx, ectx, this, e).Ptr().AsBool()
}

type BranchTip struct {
Expand Down
2 changes: 1 addition & 1 deletion runtime/sam/expr/agg.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func (a *Aggregator) NewFunction() agg.Function {

func (a *Aggregator) Apply(zctx *super.Context, ectx Context, f agg.Function, this super.Value) {
if a.where != nil {
if val, ok := EvalBool(zctx, ectx, this, a.where); !ok || !val.Bool() {
if val := EvalBool(zctx, ectx, this, a.where); !val.AsBool() {
// XXX Issue #3401: do something with "where" errors.
return
}
Expand Down
75 changes: 40 additions & 35 deletions runtime/sam/expr/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,11 @@ func NewLogicalNot(zctx *super.Context, e Evaluator) *Not {
}

func (n *Not) Eval(ectx Context, this super.Value) super.Value {
val, ok := EvalBool(n.zctx, ectx, this, n.expr)
if !ok {
val := EvalBool(n.zctx, ectx, this, n.expr)
if val.IsError() || val.IsNull() {
return val
}
if val.Bool() {
return super.False
}
return super.True
return super.NewBool(!val.Bool())
}

type And struct {
Expand All @@ -64,54 +61,62 @@ func NewLogicalOr(zctx *super.Context, lhs, rhs Evaluator) *Or {
return &Or{zctx, lhs, rhs}
}

// EvalBool evaluates e with this and if the result is a Zed bool, returns the
// result and true. Otherwise, a Zed error (inclusive of missing) and false
// are returned.
func EvalBool(zctx *super.Context, ectx Context, this super.Value, e Evaluator) (super.Value, bool) {
// EvalBool evaluates e with this and returns the result if it is a bool or error.
// Otherwise, EvalBool returns an error.
func EvalBool(zctx *super.Context, ectx Context, this super.Value, e Evaluator) super.Value {
val := e.Eval(ectx, this)
if super.TypeUnder(val.Type()) == super.TypeBool {
return val, true
}
if val.IsError() {
return val, false
if super.TypeUnder(val.Type()) == super.TypeBool || val.IsError() {
return val
}
return zctx.WrapError("not type bool", val), false
return zctx.WrapError("not type bool", val)
}

func (a *And) Eval(ectx Context, this super.Value) super.Value {
lhs, ok := EvalBool(a.zctx, ectx, this, a.lhs)
if !ok {
return lhs
}
if !lhs.Bool() {
lhs := EvalBool(a.zctx, ectx, this, a.lhs)
rhs := EvalBool(a.zctx, ectx, this, a.rhs)
if isfalse(lhs) || isfalse(rhs) {
// anything AND FALSE = FALSE
return super.False
}
rhs, ok := EvalBool(a.zctx, ectx, this, a.rhs)
if !ok {
// ERROR AND NULL = ERROR
// ERROR AND TRUE = ERROR
if lhs.IsError() {
return lhs
}
if rhs.IsError() {
return rhs
}
if !rhs.Bool() {
return super.False
if lhs.IsNull() || rhs.IsNull() {
// NULL AND TRUE = NULL
return super.NullBool
}
return super.True
}

func isfalse(val super.Value) bool {
return val.Type().ID() == super.IDBool && !val.IsNull() && !val.Bool()
}

func (o *Or) Eval(ectx Context, this super.Value) super.Value {
lhs, ok := EvalBool(o.zctx, ectx, this, o.lhs)
if ok && lhs.Bool() {
lhs := EvalBool(o.zctx, ectx, this, o.lhs)
rhs := EvalBool(o.zctx, ectx, this, o.rhs)
if lhs.AsBool() || rhs.AsBool() {
// anything OR TRUE = TRUE
return super.True
}
if lhs.IsError() && !lhs.IsMissing() {
if lhs.IsNull() || rhs.IsNull() {
// NULL OR FALSE = NULL
// NULL OR ERROR = NULL
return super.NullBool
}
// ERROR OR FALSE = ERROR
if lhs.IsError() {
return lhs
}
rhs, ok := EvalBool(o.zctx, ectx, this, o.rhs)
if ok {
if rhs.Bool() {
return super.True
}
return super.False
if rhs.IsError() {
return rhs
}
return rhs
return super.False
}

type In struct {
Expand Down
4 changes: 2 additions & 2 deletions runtime/sam/expr/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,8 @@ func NewFilterApplier(zctx *super.Context, e Evaluator) Evaluator {
}

func (f *filterApplier) Eval(ectx Context, this super.Value) super.Value {
val, ok := EvalBool(f.zctx, ectx, this, f.expr)
if ok {
val := EvalBool(f.zctx, ectx, this, f.expr)
if val.Type().ID() == super.IDBool {
if val.Bool() {
return this
}
Expand Down
102 changes: 0 additions & 102 deletions runtime/sam/expr/ztests/logical.yaml

This file was deleted.

Loading

0 comments on commit f166ba9

Please sign in to comment.