diff --git a/CHANGELOG.md b/CHANGELOG.md index 66f299cd..9a1c3a3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added ### Fixed +- Evaluation of logical ops (AND, OR, NOT) with `MISSING` argument ## [0.7.1] - 2024-03-15 ### Changed diff --git a/partiql-eval/src/eval/expr/operators.rs b/partiql-eval/src/eval/expr/operators.rs index 461e5762..7f31c4c2 100644 --- a/partiql-eval/src/eval/expr/operators.rs +++ b/partiql-eval/src/eval/expr/operators.rs @@ -80,15 +80,25 @@ impl BindEvalExpr for EvalOpUnary { args: Vec>, ) -> Result, BindError> { let any_num = PartiqlType::any_of(TYPE_NUMERIC_TYPES); - - let unop = |types, f: fn(&Value) -> Value| { - UnaryValueExpr::create_typed::<{ STRICT }, _>(types, args, f) - }; + type CheckNull = DefaultArgChecker>; + type CheckMissing = DefaultArgChecker>; match self { - EvalOpUnary::Pos => unop([any_num], std::clone::Clone::clone), - EvalOpUnary::Neg => unop([any_num], |operand| -operand), - EvalOpUnary::Not => unop([TYPE_BOOL], |operand| !operand), + EvalOpUnary::Pos => UnaryValueExpr::create_checked::< + { STRICT }, + CheckMissing, + fn(&Value) -> Value, + >([any_num], args, std::clone::Clone::clone), + EvalOpUnary::Neg => UnaryValueExpr::create_checked::< + { STRICT }, + CheckMissing, + fn(&Value) -> Value, + >([any_num], args, |operand| -operand), + EvalOpUnary::Not => UnaryValueExpr::create_checked::< + { STRICT }, + CheckNull, + fn(&Value) -> Value, + >([TYPE_BOOL], args, |operand| !operand), } } } @@ -138,7 +148,7 @@ impl ArgChecker ) -> ArgCheckControlFlow> { match arg.borrow() { Boolean(b) if b == &TARGET => ArgCheckControlFlow::ShortCircuit(Value::Boolean(*b)), - Missing => ArgCheckControlFlow::ShortCircuit(OnMissing::propagate()), + Missing => ArgCheckControlFlow::Propagate(OnMissing::propagate()), Null => ArgCheckControlFlow::Propagate(Null), _ => ArgCheckControlFlow::Continue(arg), } diff --git a/partiql-eval/src/lib.rs b/partiql-eval/src/lib.rs index a7f3a53c..fb81499c 100644 --- a/partiql-eval/src/lib.rs +++ b/partiql-eval/src/lib.rs @@ -23,7 +23,8 @@ mod tests { use crate::plan::EvaluationMode; use partiql_logical::{ BagExpr, BetweenExpr, BinaryOp, BindingsOp, CoalesceExpr, ExprQuery, IsTypeExpr, JoinKind, - ListExpr, LogicalPlan, NullIfExpr, PathComponent, TupleExpr, Type, ValueExpr, VarRefType, + ListExpr, LogicalPlan, NullIfExpr, PathComponent, TupleExpr, Type, UnaryOp, ValueExpr, + VarRefType, }; use partiql_value as value; use partiql_value::Value::{Missing, Null}; @@ -641,9 +642,23 @@ mod tests { } #[test] - fn and_or_null() { + fn logical_ops() { #[track_caller] - fn eval_to_null(op: BinaryOp, lhs: Value, rhs: Value) { + fn eval_unary(op: UnaryOp, expr: Value, expected: Value) { + let mut plan = LogicalPlan::new(); + let expq = plan.add_operator(BindingsOp::ExprQuery(ExprQuery { + expr: ValueExpr::UnExpr(op, Box::new(ValueExpr::Lit(Box::new(expr)))), + })); + + let sink = plan.add_operator(BindingsOp::Sink); + plan.add_flow(expq, sink); + + let actual = evaluate(plan, MapBindings::default()); + assert_eq!(expected, actual); + } + + #[track_caller] + fn eval_binary(op: BinaryOp, lhs: Value, rhs: Value, expected: Value) { let mut plan = LogicalPlan::new(); let expq = plan.add_operator(BindingsOp::ExprQuery(ExprQuery { expr: ValueExpr::BinaryExpr( @@ -656,18 +671,175 @@ mod tests { let sink = plan.add_operator(BindingsOp::Sink); plan.add_flow(expq, sink); - let result = evaluate(plan, MapBindings::default()); - assert_eq!(result, Value::Null); + let actual = evaluate(plan, MapBindings::default()); + assert_eq!(expected, actual); } - eval_to_null(BinaryOp::And, Value::Null, Value::Boolean(true)); - eval_to_null(BinaryOp::And, Value::Missing, Value::Boolean(true)); - eval_to_null(BinaryOp::And, Value::Boolean(true), Value::Null); - eval_to_null(BinaryOp::And, Value::Boolean(true), Value::Missing); - eval_to_null(BinaryOp::Or, Value::Null, Value::Boolean(false)); - eval_to_null(BinaryOp::Or, Value::Missing, Value::Boolean(false)); - eval_to_null(BinaryOp::Or, Value::Boolean(false), Value::Null); - eval_to_null(BinaryOp::Or, Value::Boolean(false), Value::Missing); + // NOT bools only + eval_unary(UnaryOp::Not, Value::Boolean(true), Value::Boolean(false)); + eval_unary(UnaryOp::Not, Value::Boolean(false), Value::Boolean(true)); + // NOT null propagation + eval_unary(UnaryOp::Not, Value::Null, Value::Null); + eval_unary(UnaryOp::Not, Value::Missing, Value::Null); + + // AND/OR bools only + eval_binary( + BinaryOp::And, + Value::Boolean(true), + Value::Boolean(true), + Value::Boolean(true), + ); + eval_binary( + BinaryOp::And, + Value::Boolean(false), + Value::Boolean(true), + Value::Boolean(false), + ); + eval_binary( + BinaryOp::And, + Value::Boolean(true), + Value::Boolean(false), + Value::Boolean(false), + ); + eval_binary( + BinaryOp::And, + Value::Boolean(false), + Value::Boolean(false), + Value::Boolean(false), + ); + eval_binary( + BinaryOp::Or, + Value::Boolean(true), + Value::Boolean(true), + Value::Boolean(true), + ); + eval_binary( + BinaryOp::Or, + Value::Boolean(true), + Value::Boolean(false), + Value::Boolean(true), + ); + eval_binary( + BinaryOp::Or, + Value::Boolean(false), + Value::Boolean(true), + Value::Boolean(true), + ); + eval_binary( + BinaryOp::Or, + Value::Boolean(false), + Value::Boolean(false), + Value::Boolean(false), + ); + + // AND/OR short circuit + eval_binary( + BinaryOp::And, + Value::Boolean(false), + Value::Null, + Value::Boolean(false), + ); + eval_binary( + BinaryOp::And, + Value::Boolean(false), + Value::Missing, + Value::Boolean(false), + ); + eval_binary( + BinaryOp::And, + Value::Null, + Value::Boolean(false), + Value::Boolean(false), + ); + eval_binary( + BinaryOp::And, + Value::Missing, + Value::Boolean(false), + Value::Boolean(false), + ); + eval_binary( + BinaryOp::Or, + Value::Boolean(true), + Value::Null, + Value::Boolean(true), + ); + eval_binary( + BinaryOp::Or, + Value::Boolean(true), + Value::Missing, + Value::Boolean(true), + ); + eval_binary( + BinaryOp::Or, + Value::Null, + Value::Boolean(true), + Value::Boolean(true), + ); + eval_binary( + BinaryOp::Or, + Value::Missing, + Value::Boolean(true), + Value::Boolean(true), + ); + + // AND/OR propagate null + eval_binary( + BinaryOp::And, + Value::Boolean(true), + Value::Null, + Value::Null, + ); + eval_binary( + BinaryOp::And, + Value::Boolean(true), + Value::Missing, + Value::Null, + ); + eval_binary( + BinaryOp::And, + Value::Null, + Value::Boolean(true), + Value::Null, + ); + eval_binary( + BinaryOp::And, + Value::Missing, + Value::Boolean(true), + Value::Null, + ); + eval_binary( + BinaryOp::Or, + Value::Boolean(false), + Value::Null, + Value::Null, + ); + eval_binary( + BinaryOp::Or, + Value::Boolean(false), + Value::Missing, + Value::Null, + ); + eval_binary( + BinaryOp::Or, + Value::Null, + Value::Boolean(false), + Value::Null, + ); + eval_binary( + BinaryOp::Or, + Value::Missing, + Value::Boolean(false), + Value::Null, + ); + + eval_binary(BinaryOp::And, Value::Null, Value::Null, Value::Null); + eval_binary(BinaryOp::And, Value::Null, Value::Missing, Value::Null); + eval_binary(BinaryOp::And, Value::Missing, Value::Null, Value::Null); + eval_binary(BinaryOp::And, Value::Missing, Value::Missing, Value::Null); + eval_binary(BinaryOp::Or, Value::Null, Value::Null, Value::Null); + eval_binary(BinaryOp::Or, Value::Null, Value::Missing, Value::Null); + eval_binary(BinaryOp::Or, Value::Missing, Value::Null, Value::Null); + eval_binary(BinaryOp::Or, Value::Missing, Value::Missing, Value::Null); } #[test]