From 1628b4729894cd1806e088f45a8b501e936e19ab Mon Sep 17 00:00:00 2001 From: Felipe705x Date: Mon, 29 Jun 2026 11:30:41 -0400 Subject: [PATCH 1/4] feat(3vl): three-valued logic for AND/OR/NOT at runtime Evaluate the boolean connectives under SQL 3VL with short-circuit. `AND`/`OR` get dedicated arms in `run_expr`, outside the implicit argument-cast wrapper, so a `Null` or `Failure` operand (missing attribute, unbound var, type error) is read as the 3VL unknown rather than collapsing the whole expression: true OR null -> true false AND null -> false false OR null -> null true AND null -> null null OR null -> null null AND null -> null NOT null -> null `Failure` is treated as the unknown input, matching the existing IsNull/Coalesce/aggregate convention, so e.g. `WHERE x.status OR true` returns all rows when `status` is absent (previously: every row dropped). The unreachable `eval_binop` And/Or arms are updated to the same table for any direct caller. Co-Authored-By: Claude Opus 4.8 --- src/runtime/engine.rs | 85 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 83 insertions(+), 2 deletions(-) diff --git a/src/runtime/engine.rs b/src/runtime/engine.rs index b5e05c4..0fe1c01 100644 --- a/src/runtime/engine.rs +++ b/src/runtime/engine.rs @@ -3283,6 +3283,69 @@ impl<'g, G: GraphAccess + 'g> Runtime<'g, G> { _ => ExprResult::Failure("invalid 'as' operands".into()), } } + // Logical connectives follow SQL three-valued logic (3VL) and + // short-circuit. They are handled here, *outside* the generic + // operator wrapper below, so a `Null`/`Failure` operand reaches + // the 3VL table instead of being rejected by the implicit + // argument cast. `Failure` (a missing attribute, an unbound + // variable, a type error) is treated as the unknown/null input, + // matching how `IsNull`/`Coalesce`/aggregates already read it. + BinOp::Or => { + let l = self.run_expr(mu, left); + // true OR _ = true (short-circuit; rhs not evaluated). + if matches!(l, ExprResult::Success(Value::Bool(true))) { + return ExprResult::Success(Value::Bool(true)); + } + let r = self.run_expr(mu, right); + match (&l, &r) { + // _ OR true = true + (_, ExprResult::Success(Value::Bool(true))) => { + ExprResult::Success(Value::Bool(true)) + } + // false OR false = false + ( + ExprResult::Success(Value::Bool(false)), + ExprResult::Success(Value::Bool(false)), + ) => ExprResult::Success(Value::Bool(false)), + // false OR null, null OR false, null OR null = null. + ( + ExprResult::Success(Value::Bool(false) | Value::Null) + | ExprResult::Failure(_), + ExprResult::Success(Value::Bool(false) | Value::Null) + | ExprResult::Failure(_), + ) => ExprResult::Success(Value::Null), + // A concrete non-bool operand (e.g. Int) is a real error. + _ => ExprResult::Failure("or requires bools".into()), + } + } + BinOp::And => { + let l = self.run_expr(mu, left); + // false AND _ = false (short-circuit; rhs not evaluated). + if matches!(l, ExprResult::Success(Value::Bool(false))) { + return ExprResult::Success(Value::Bool(false)); + } + let r = self.run_expr(mu, right); + match (&l, &r) { + // _ AND false = false + (_, ExprResult::Success(Value::Bool(false))) => { + ExprResult::Success(Value::Bool(false)) + } + // true AND true = true + ( + ExprResult::Success(Value::Bool(true)), + ExprResult::Success(Value::Bool(true)), + ) => ExprResult::Success(Value::Bool(true)), + // true AND null, null AND true, null AND null = null. + ( + ExprResult::Success(Value::Bool(true) | Value::Null) + | ExprResult::Failure(_), + ExprResult::Success(Value::Bool(true) | Value::Null) + | ExprResult::Failure(_), + ) => ExprResult::Success(Value::Null), + // A concrete non-bool operand (e.g. Int) is a real error. + _ => ExprResult::Failure("and requires bools".into()), + } + } _ => { let (ty_l, _, _) = op.delta(&SimpleType::Star, &SimpleType::Star); let cast_left = Expr::Binop { @@ -3324,6 +3387,9 @@ impl<'g, G: GraphAccess + 'g> Runtime<'g, G> { }, UnOp::Not => match val { Value::Bool(b) => ExprResult::Success(Value::Bool(!b)), + // 3VL: NOT null = null (a Failure operand already + // propagates to null below). + Value::Null => ExprResult::Success(Value::Null), _ => ExprResult::Failure("not requires bool".into()), }, }, @@ -4076,12 +4142,27 @@ impl<'g, G: GraphAccess + 'g> Runtime<'g, G> { } _ => ExprResult::Failure("'in' requires a list on the right".into()), }, + // NOTE: `And`/`Or` are normally intercepted in `run_expr` (3VL + + // short-circuit) and do not reach here. These arms are kept + // consistent with that 3VL table for any direct caller. BinOp::And => match (lv, rv) { - (Value::Bool(a), Value::Bool(b)) => ExprResult::Success(Value::Bool(*a && *b)), + (Value::Bool(false), _) | (_, Value::Bool(false)) => { + ExprResult::Success(Value::Bool(false)) + } + (Value::Bool(true), Value::Bool(true)) => ExprResult::Success(Value::Bool(true)), + (Value::Bool(true) | Value::Null, Value::Bool(true) | Value::Null) => { + ExprResult::Success(Value::Null) + } _ => ExprResult::Failure("and requires bools".into()), }, BinOp::Or => match (lv, rv) { - (Value::Bool(a), Value::Bool(b)) => ExprResult::Success(Value::Bool(*a || *b)), + (Value::Bool(true), _) | (_, Value::Bool(true)) => { + ExprResult::Success(Value::Bool(true)) + } + (Value::Bool(false), Value::Bool(false)) => ExprResult::Success(Value::Bool(false)), + (Value::Bool(false) | Value::Null, Value::Bool(false) | Value::Null) => { + ExprResult::Success(Value::Null) + } _ => ExprResult::Failure("or requires bools".into()), }, _ => ExprResult::Failure(format!("unexpected op {op} in eval_binop")), From ade7352c2f520cc73744ca4795a649e712735b8f Mon Sep 17 00:00:00 2001 From: Felipe705x Date: Mon, 29 Jun 2026 11:31:06 -0400 Subject: [PATCH 2/4] feat(3vl): pass null through the `as` cast (null AS T -> null) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `null AS T` returns null for any target type instead of failing the membership check. This satisfies `null AS T -> null` and lets a null operand slip through the implicit argument cast used by the generic operator wrapper, so it reaches the 3VL logic (the runtime relaxation of the SimpleType-only `△(bop)` check). The `IS`/`TYPED` predicate is untouched — it calls `value_is_type` directly, not this arm. Co-Authored-By: Claude Opus 4.8 --- src/runtime/engine.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/runtime/engine.rs b/src/runtime/engine.rs index 0fe1c01..192ab96 100644 --- a/src/runtime/engine.rs +++ b/src/runtime/engine.rs @@ -3272,6 +3272,15 @@ impl<'g, G: GraphAccess + 'g> Runtime<'g, G> { BinOp::As => { let l = self.run_expr(mu, left); match (&l, right.as_ref()) { + // 3VL: null inhabits every type — `null AS T` is null, + // regardless of target. This also lets a null operand + // pass the implicit argument cast in the wrapper below + // and reach the 3VL logic (the runtime `△(bop)` + // relaxation), without changing the `IS`/`TYPED` + // predicate (which uses `value_is_type` directly). + (ExprResult::Success(Value::Null), Expr::Type(_)) => { + ExprResult::Success(Value::Null) + } (ExprResult::Success(val), Expr::Type(ty)) => { if Expr::value_is_type(val, ty) { ExprResult::Success(val.clone()) From 8e36fe627ef1d82b542e2df5c127c58b3d9408cb Mon Sep 17 00:00:00 2001 From: Felipe705x Date: Mon, 29 Jun 2026 11:31:21 -0400 Subject: [PATCH 3/4] feat(3vl): typechecker short-circuit override for AND/OR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Override the codomain (c̃od) check for the short-circuit connectives: `AND`/`OR` type as `B` unless BOTH operands meet bottom (⊥) against `B`. A ⊥/Null operand beside a valid Bool still yields Bool (e.g. `Integer OR true : B`); only when both sides are incompatible with Bool does the expression degrade to ⊥ with a warning. All other operators keep the default delta-based codomain check. Co-Authored-By: Claude Opus 4.8 --- src/typing/checker.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/typing/checker.rs b/src/typing/checker.rs index 22ff420..6af68fe 100644 --- a/src/typing/checker.rs +++ b/src/typing/checker.rs @@ -738,6 +738,27 @@ impl Typechecker { } let t2 = self.check_expr(right, env); + + // Logical connectives short-circuit under 3VL, so we override + // the default `c̃od` propagation for them: the result is `B` + // unless BOTH operands are incompatible with `B` (a ⊥/Null + // operand beside a valid Bool still yields a Bool, e.g. + // `true OR ⊥`). Only when both sides meet ⊥ against `B` do we + // degrade to ⊥. + if matches!(op, BinOp::And | BinOp::Or) { + let both_bottom = SimpleType::meet(&t1, &SimpleType::B) == SimpleType::Zero + && SimpleType::meet(&t2, &SimpleType::B) == SimpleType::Zero; + return if both_bottom { + self.warnings.push(format!( + "Binop {:?} between types {} and {} is not defined", + op, t1, t2 + )); + SimpleType::Zero + } else { + SimpleType::B + }; + } + let (expected_t1, expected_t2, result_t) = op.delta(&t1, &t2); if SimpleType::meet(&t1, &expected_t1) != SimpleType::Zero From f508fabe3010534546735cc902477da82b9beda9 Mon Sep 17 00:00:00 2001 From: Felipe705x Date: Mon, 29 Jun 2026 11:31:21 -0400 Subject: [PATCH 4/4] test(3vl): three-valued-logic suite for the boolean connectives Covers the OR/AND/NOT truth tables, missing-property-as-unknown, the `null AS T` pass-through, invalid-cast handling (empty in WHERE, null cell in RETURN), and the typechecker short-circuit. Includes WHERE-clause differentiators that fail under the pre-fix behavior (e.g. `WHERE x.status OR true` returned no rows), and no-regression guards pinning the pre-existing comparison-operator behavior (handled in a follow-up). Co-Authored-By: Claude Opus 4.8 --- tests/three_valued_logic_test.rs | 274 +++++++++++++++++++++++++++++++ 1 file changed, 274 insertions(+) create mode 100644 tests/three_valued_logic_test.rs diff --git a/tests/three_valued_logic_test.rs b/tests/three_valued_logic_test.rs new file mode 100644 index 0000000..05735c3 --- /dev/null +++ b/tests/three_valued_logic_test.rs @@ -0,0 +1,274 @@ +//! SQL three-valued logic (3VL) for the boolean connectives `AND` / `OR` / +//! `NOT`, the `null AS T` cast pass-through, and the cast-error empties-the-row +//! behavior. Boolean-connective scope only — comparison/equality operators +//! (`=`, `<>`, `<`, ...) keep their pre-existing behavior here and are handled +//! in a follow-up PR; the guards at the bottom pin that pre-existing behavior. + +use frogql::elaborate; +use frogql::model::graph::MemoryGraphStore; +use frogql::model::value::Value; +use frogql::parser; +use frogql::runtime::engine::Runtime; +use frogql::runtime::result::QueryResult; +use frogql::typing::checker::{TypecheckResult, Typechecker}; + +// --------------------------------------------------------------------------- +// Harnesses +// --------------------------------------------------------------------------- + +fn run(g: &MemoryGraphStore, q: &str) -> Vec> { + let rt = Runtime::new(g); + let query = frogql::compile_query(q).expect("compile failed"); + match rt.run_query(&query, 0) { + QueryResult::Projected(rs) => rs, + other => panic!("expected projected rows for {q:?}, got {other:?}"), + } +} + +/// One bare node so a scalar `RETURN ` produces exactly one row. +fn one_node() -> MemoryGraphStore { + let json = r#"{"nodes":[{"id":"n1","labels":["N"],"props":{"s":"a"}}],"edges":[]}"#; + MemoryGraphStore::from_json_str(json).unwrap() +} + +/// Single-cell scalar result of `RETURN () AS b` over one node. +fn scalar(expr: &str) -> Value { + let g = one_node(); + let rows = run(&g, &format!("MATCH (x:N) RETURN ({expr}) AS b")); + assert_eq!(rows.len(), 1, "expected one row for {expr:?}"); + rows.into_iter().next().unwrap().into_iter().next().unwrap() +} + +/// Parse, elaborate, and typecheck a full query under the permissive star +/// schema. Returns the typecheck result plus errors/warnings. +fn check_full_query(query: &str) -> (TypecheckResult, Vec, Vec) { + let q = parser::parse_query(query).expect("parse failed"); + let q = elaborate::elaborate_query(q); + let mut tc = Typechecker::untyped(); + let r = tc.check_query(&q); + (r, tc.errors.clone(), tc.warnings.clone()) +} + +// --------------------------------------------------------------------------- +// Fix 2 — 3VL truth tables for OR / AND / NOT (literal null operand) +// --------------------------------------------------------------------------- + +#[test] +fn test_or_truth_table() { + assert_eq!(scalar("true OR null"), Value::Bool(true)); // short-circuit + assert_eq!(scalar("null OR true"), Value::Bool(true)); + assert_eq!(scalar("false OR null"), Value::Null); + assert_eq!(scalar("null OR false"), Value::Null); + assert_eq!(scalar("null OR null"), Value::Null); + assert_eq!(scalar("true OR false"), Value::Bool(true)); + assert_eq!(scalar("false OR false"), Value::Bool(false)); +} + +#[test] +fn test_and_truth_table() { + assert_eq!(scalar("false AND null"), Value::Bool(false)); // short-circuit + assert_eq!(scalar("null AND false"), Value::Bool(false)); + assert_eq!(scalar("true AND null"), Value::Null); + assert_eq!(scalar("null AND true"), Value::Null); + assert_eq!(scalar("null AND null"), Value::Null); + assert_eq!(scalar("true AND true"), Value::Bool(true)); + assert_eq!(scalar("true AND false"), Value::Bool(false)); +} + +#[test] +fn test_not_null() { + assert_eq!(scalar("NOT null"), Value::Null); + assert_eq!(scalar("NOT true"), Value::Bool(false)); + assert_eq!(scalar("NOT false"), Value::Bool(true)); +} + +// --------------------------------------------------------------------------- +// Fix 2 — a missing/null property surfaces as the 3VL unknown (the engine +// reads a missing attribute as a Failure, treated as null by the connectives). +// --------------------------------------------------------------------------- + +/// Three nodes: active=true, active=false, and one with `active` absent. +fn active_graph() -> MemoryGraphStore { + let json = r#"{ + "nodes": [ + {"id":"a","labels":["N"],"props":{"name":"A","active":true}}, + {"id":"b","labels":["N"],"props":{"name":"B","active":false}}, + {"id":"c","labels":["N"],"props":{"name":"C"}} + ], + "edges": [] + }"#; + MemoryGraphStore::from_json_str(json).unwrap() +} + +#[test] +fn test_missing_property_is_unknown_in_or() { + // false OR x.active: true -> true, false -> false, missing -> null. + let g = active_graph(); + let mut rows = run(&g, "MATCH (x:N) RETURN x.name, (false OR x.active) AS b"); + rows.sort_by(|l, r| format!("{:?}", l[0]).cmp(&format!("{:?}", r[0]))); + assert_eq!( + rows, + vec![ + vec![Value::Str("A".into()), Value::Bool(true)], + vec![Value::Str("B".into()), Value::Bool(false)], + vec![Value::Str("C".into()), Value::Null], + ] + ); +} + +#[test] +fn test_missing_property_short_circuits_in_and() { + // false AND x.active = false for every row, even the missing one. + let g = active_graph(); + let rows = run(&g, "MATCH (x:N) RETURN (false AND x.active) AS b"); + assert_eq!(rows.len(), 3); + for row in &rows { + assert_eq!(row, &vec![Value::Bool(false)]); + } +} + +/// Names returned by a query, sorted. +fn names(g: &MemoryGraphStore, q: &str) -> Vec { + let mut ns: Vec = run(g, q) + .into_iter() + .map(|r| match &r[0] { + Value::Str(s) => s.clone(), + other => panic!("expected Str name, got {other:?}"), + }) + .collect(); + ns.sort(); + ns +} + +// These WHERE-clause cases FAIL under the pre-fix behavior: `x.status` is a +// missing attribute, which the engine reads as a Failure. The old code routed +// the connective through the implicit argument-cast wrapper, which propagated +// that Failure (`get_bool` -> false) and dropped EVERY row. Correct 3VL keeps +// them all: `unknown OR true = true`. + +#[test] +fn test_missing_or_true_returns_all_rows() { + let g = active_graph(); // no node has a `status` property + assert_eq!( + names(&g, "MATCH (x:N) WHERE x.status OR true RETURN x.name"), + vec!["A", "B", "C"], + ); +} + +#[test] +fn test_true_or_missing_returns_all_rows() { + let g = active_graph(); + assert_eq!( + names(&g, "MATCH (x:N) WHERE true OR x.status RETURN x.name"), + vec!["A", "B", "C"], + ); +} + +#[test] +fn test_missing_or_true_in_projection_is_true() { + // Same divergence in a projection: pre-fix yielded a Null cell, correct + // 3VL yields true. + let g = active_graph(); + let rows = run(&g, "MATCH (x:N) RETURN (x.status OR true) AS b"); + assert_eq!(rows.len(), 3); + for row in &rows { + assert_eq!(row, &vec![Value::Bool(true)]); + } +} + +// --------------------------------------------------------------------------- +// Fix 3 / Fix 4 — cast (`as`) null pass-through and invalid-cast handling +// --------------------------------------------------------------------------- + +#[test] +fn test_null_cast_passes_through() { + // `null AS T` (bare AS = assert) is null regardless of target. + assert_eq!(scalar("null AS INTEGER"), Value::Null); + assert_eq!(scalar("null AS FLOAT"), Value::Null); + assert_eq!(scalar("null AS STRING"), Value::Null); +} + +#[test] +fn test_invalid_cast_in_where_empties_result() { + // Case A: an impossible cast in a WHERE filter drops the row (no crash). + let g = one_node(); // x.s = "a" + let rows = run(&g, "MATCH (x:N) WHERE CAST(x.s AS INTEGER) > 0 RETURN x.s"); + assert!(rows.is_empty(), "expected no rows, got {rows:?}"); +} + +#[test] +fn test_invalid_cast_in_return_is_null_cell() { + // Case B (Option R): an impossible cast in a projection yields a null + // cell; the row is kept. (No FPPCTypeError channel in this PR.) + let g = one_node(); // x.s = "a" + let rows = run(&g, "MATCH (x:N) RETURN CAST(x.s AS INTEGER) AS i"); + assert_eq!(rows, vec![vec![Value::Null]]); +} + +// --------------------------------------------------------------------------- +// Fix 1 — typechecker short-circuit override for AND / OR +// --------------------------------------------------------------------------- + +fn or_warns(query: &str) -> bool { + let (_, _, warnings) = check_full_query(query); + warnings + .iter() + .any(|w| w.contains("Or") && w.contains("is not defined")) +} + +#[test] +fn test_typecheck_or_with_one_bottom_operand_is_bool() { + // `1 OR true`: Integer meets ⊥ against Bool, but the other side is a + // valid Bool, so under the short-circuit override the result is Bool and + // the OR itself produces no "is not defined" warning (A1 leniency). + let (r, errs, _) = check_full_query("MATCH (x) WHERE 1 OR true RETURN x"); + assert!(r.ok, "expected ok, errors={errs:?}"); + assert!(!or_warns("MATCH (x) WHERE 1 OR true RETURN x")); +} + +#[test] +fn test_typecheck_or_with_both_bottom_operands_degrades() { + // `1 OR 'a'`: both operands meet ⊥ against Bool -> degrade to ⊥ + warn. + assert!( + or_warns("MATCH (x) WHERE 1 OR 'a' RETURN x"), + "expected an 'Or ... is not defined' warning" + ); +} + +#[test] +fn test_typecheck_arithmetic_mismatch_still_degrades() { + // Regression: non-short-circuit ops keep the default cod degradation. + let (_, _, warnings) = check_full_query("MATCH (x) RETURN 1 + true"); + assert!( + warnings.iter().any(|w| w.contains("is not defined")), + "expected a degradation warning for 1 + true, got: {warnings:?}" + ); +} + +// --------------------------------------------------------------------------- +// No-regression guards — comparison/equality stay at pre-existing behavior. +// These pin the current (non-3VL) results; the comparison-3VL follow-up PR +// is expected to flip the null-comparison ones. +// --------------------------------------------------------------------------- + +#[test] +fn test_guard_eq_null_literal_drops_rows() { + // Pre-existing (and ISO-aligned): `= null` never matches -> empty. + let g = active_graph(); + let rows = run(&g, "MATCH (x:N) WHERE x.active = null RETURN x.name"); + assert!(rows.is_empty(), "expected no rows, got {rows:?}"); +} + +#[test] +fn test_guard_null_eq_null_is_true_pre_existing() { + // PRE-EXISTING WART (to be fixed in the comparison-3VL follow-up PR): + // structural equality makes `null = null` true rather than null. + assert_eq!(scalar("null = null"), Value::Bool(true)); +} + +#[test] +fn test_guard_basic_comparison_unaffected() { + assert_eq!(scalar("1 = 1"), Value::Bool(true)); + assert_eq!(scalar("1 = 2"), Value::Bool(false)); + assert_eq!(scalar("2 > 1"), Value::Bool(true)); +}