fix(sql): surface division-by-zero and undefined-function errors (#216)

Two related classes of expression error silently evaluated to NULL
instead of failing the statement, letting bad queries return
plausible-looking wrong results:

- A `/` or `%` with a zero divisor folded to NULL in both the f64 and
  Decimal arithmetic paths (`SqlExpr::eval`'s row-scope evaluator) and
  in the `mod` scalar function. It now raises SQLSTATE 22012
  (division_by_zero) at the point the divisor value is evaluated.
  `SqlExpr::eval`/`eval_with_old`/`eval_with_excluded` and
  `eval_function` now return `Result<Value, EvalError>` instead of
  `Value`, threaded through every caller: the query evaluator's own
  scan/filter/window/aggregate paths, CRDT CHECK-constraint and
  transition-check validation, and every executor handler, columnar
  scan, and cold-storage predicate pushdown in `nodedb` that evaluates
  a row expression. CASE and COALESCE stay short-circuiting, so an
  untaken branch or an already-satisfied argument never raises.

- A call to a scalar/aggregate/window function with no registered
  definition fell through `eval_function`'s final
  `unwrap_or(Value::Null)` and never surfaced as an error. The SQL
  resolver now rejects any function name absent from
  `FunctionRegistry` at plan time (SQLSTATE 42883, undefined_function)
  instead of letting the call flow through to evaluate as NULL on
  every row. This gate exposed collection registry gaps for function
  families the runtime evaluator already supported (array-element,
  ID-generation, and legacy JSON functions, plus several math,
  datetime, misc, spatial, and string builtins) that would otherwise
  have been wrongly rejected; those registrations are added alongside
  the gate.

Adds pgwire integration coverage for both SQLSTATEs across the
document and columnar engines.

Author: emanzx

nodedb-crdt/src/constraint_checks.rs

@@ -85,16 +85,37 @@ impl Validator {
             }
         };
 
+        // A division/modulo-by-zero inside the predicate (nodedb issue #216)
+        // is neither a PASS nor an ordinary FAIL — it's an evaluation
+        // error. `Validator::validate`/`ValidationOutcome` (this crate's
+        // public API, consumed cross-crate by `nodedb`) has no error
+        // channel distinct from `Rejected(Vec<Violation>)`, so — exactly
+        // like the unparseable-expression case above — it fails closed as
+        // a `Violation` with a reason that names the real cause, rather
+        // than silently treating it as a pass. Giving eval errors a first-
+        // class outcome distinct from an ordinary CHECK failure would mean
+        // widening `ValidationOutcome` and its ~5 cross-crate call sites; a
+        // deliberate follow-up, not a mechanical part of this fix.
         match parsed.eval(&row_value) {
-            nodedb_types::Value::Bool(true) => None,
-            nodedb_types::Value::Null => None,
-            _ => Some(Violation {
+            Ok(nodedb_types::Value::Bool(true)) => None,
+            Ok(nodedb_types::Value::Null) => None,
+            Ok(_) => Some(Violation {
                 constraint_name: constraint.name.clone(),
                 reason: format!("CHECK `{}` failed: {expr}", constraint.name),
                 hint: CompensationHint::ManualIntervention {
                     reason: format!("row violates CHECK predicate `{expr}`"),
                 },
             }),
+            Err(e) => Some(Violation {
+                constraint_name: constraint.name.clone(),
+                reason: format!(
+                    "CHECK `{}` failed to evaluate: {expr}: {e}",
+                    constraint.name
+                ),
+                hint: CompensationHint::ManualIntervention {
+                    reason: format!("CHECK predicate `{expr}` raised an evaluation error: {e}"),
+                },
+            }),
         }
     }
 

nodedb-query/src/expr/binary.rs

@@ -10,6 +10,7 @@ use crate::value_ops::{
     coerced_eq, compare_values, is_truthy, to_value_number, value_to_display_string, value_to_f64,
 };
 
+use super::eval::EvalError;
 use super::types::BinaryOp;
 
 /// Coerce a `Value` to `Decimal` for precise arithmetic.
@@ -28,86 +29,119 @@ fn value_to_decimal(v: &Value) -> Option<Decimal> {
     }
 }
 
-/// Apply a Decimal arithmetic operation, returning `Value::Null` on overflow/div-zero.
-fn decimal_arith(a: Decimal, op: BinaryOp, b: Decimal) -> Value {
+/// Apply a Decimal arithmetic operation, returning `Value::Null` on overflow
+/// and `Err(EvalError::DivisionByZero)` when `/` or `%` see a zero divisor
+/// (nodedb issue #216). The zero check happens before `checked_div`/
+/// `checked_rem` are called because those methods return `None` for both
+/// div-by-zero *and* overflow, and only the former is a `22012` error —
+/// overflow keeps folding to `Value::Null` exactly as before.
+fn decimal_arith(a: Decimal, op: BinaryOp, b: Decimal) -> Result<Value, EvalError> {
     let result = match op {
         BinaryOp::Add => a.checked_add(b),
         BinaryOp::Sub => a.checked_sub(b),
         BinaryOp::Mul => a.checked_mul(b),
-        BinaryOp::Div => a.checked_div(b),
-        BinaryOp::Mod => a.checked_rem(b),
-        _ => return Value::Null,
+        BinaryOp::Div => {
+            if b.is_zero() {
+                return Err(EvalError::DivisionByZero);
+            }
+            a.checked_div(b)
+        }
+        BinaryOp::Mod => {
+            if b.is_zero() {
+                return Err(EvalError::DivisionByZero);
+            }
+            a.checked_rem(b)
+        }
+        _ => return Ok(Value::Null),
     };
-    result.map(Value::Decimal).unwrap_or(Value::Null)
+    Ok(result.map(Value::Decimal).unwrap_or(Value::Null))
 }
 
-pub(super) fn eval_binary_op(left: &Value, op: BinaryOp, right: &Value) -> Value {
+pub(super) fn eval_binary_op(
+    left: &Value,
+    op: BinaryOp,
+    right: &Value,
+) -> Result<Value, EvalError> {
     match op {
         // Arithmetic: prefer Decimal when either operand is Decimal to avoid f64 drift.
         BinaryOp::Add | BinaryOp::Sub | BinaryOp::Mul | BinaryOp::Div | BinaryOp::Mod => {
             let left_is_decimal = matches!(left, Value::Decimal(_));
             let right_is_decimal = matches!(right, Value::Decimal(_));
             if left_is_decimal || right_is_decimal {
-                match (value_to_decimal(left), value_to_decimal(right)) {
-                    (Some(a), Some(b)) => return decimal_arith(a, op, b),
-                    _ => return Value::Null,
-                }
+                return match (value_to_decimal(left), value_to_decimal(right)) {
+                    (Some(a), Some(b)) => decimal_arith(a, op, b),
+                    _ => Ok(Value::Null),
+                };
             }
             // Both operands are non-Decimal: use f64 path.
             match op {
-                BinaryOp::Add => match (value_to_f64(left, true), value_to_f64(right, true)) {
-                    (Some(a), Some(b)) => to_value_number(a + b),
-                    _ => Value::Null,
-                },
-                BinaryOp::Sub => match (value_to_f64(left, true), value_to_f64(right, true)) {
-                    (Some(a), Some(b)) => to_value_number(a - b),
-                    _ => Value::Null,
-                },
-                BinaryOp::Mul => match (value_to_f64(left, true), value_to_f64(right, true)) {
-                    (Some(a), Some(b)) => to_value_number(a * b),
-                    _ => Value::Null,
-                },
+                BinaryOp::Add => Ok(
+                    match (value_to_f64(left, true), value_to_f64(right, true)) {
+                        (Some(a), Some(b)) => to_value_number(a + b),
+                        _ => Value::Null,
+                    },
+                ),
+                BinaryOp::Sub => Ok(
+                    match (value_to_f64(left, true), value_to_f64(right, true)) {
+                        (Some(a), Some(b)) => to_value_number(a - b),
+                        _ => Value::Null,
+                    },
+                ),
+                BinaryOp::Mul => Ok(
+                    match (value_to_f64(left, true), value_to_f64(right, true)) {
+                        (Some(a), Some(b)) => to_value_number(a * b),
+                        _ => Value::Null,
+                    },
+                ),
                 BinaryOp::Div => match (value_to_f64(left, true), value_to_f64(right, true)) {
                     (Some(a), Some(b)) => {
                         if b == 0.0 {
-                            Value::Null
+                            Err(EvalError::DivisionByZero)
                         } else {
-                            to_value_number(a / b)
+                            Ok(to_value_number(a / b))
                         }
                     }
-                    _ => Value::Null,
+                    _ => Ok(Value::Null),
                 },
                 BinaryOp::Mod => match (value_to_f64(left, true), value_to_f64(right, true)) {
                     (Some(a), Some(b)) => {
                         if b == 0.0 {
-                            Value::Null
+                            Err(EvalError::DivisionByZero)
                         } else {
-                            to_value_number(a % b)
+                            Ok(to_value_number(a % b))
                         }
                     }
-                    _ => Value::Null,
+                    _ => Ok(Value::Null),
                 },
-                _ => Value::Null,
+                _ => Ok(Value::Null),
             }
         }
         BinaryOp::Concat => {
             let ls = value_to_display_string(left);
             let rs = value_to_display_string(right);
-            Value::String(format!("{ls}{rs}"))
+            Ok(Value::String(format!("{ls}{rs}")))
         }
-        BinaryOp::Eq => Value::Bool(coerced_eq(left, right)),
-        BinaryOp::NotEq => Value::Bool(!coerced_eq(left, right)),
-        BinaryOp::Gt => Value::Bool(compare_values(left, right) == std::cmp::Ordering::Greater),
+        BinaryOp::Eq => Ok(Value::Bool(coerced_eq(left, right))),
+        BinaryOp::NotEq => Ok(Value::Bool(!coerced_eq(left, right))),
+        BinaryOp::Gt => Ok(Value::Bool(
+            compare_values(left, right) == std::cmp::Ordering::Greater,
+        )),
         BinaryOp::GtEq => {
             let c = compare_values(left, right);
-            Value::Bool(c == std::cmp::Ordering::Greater || c == std::cmp::Ordering::Equal)
+            Ok(Value::Bool(
+                c == std::cmp::Ordering::Greater || c == std::cmp::Ordering::Equal,
+            ))
         }
-        BinaryOp::Lt => Value::Bool(compare_values(left, right) == std::cmp::Ordering::Less),
+        BinaryOp::Lt => Ok(Value::Bool(
+            compare_values(left, right) == std::cmp::Ordering::Less,
+        )),
         BinaryOp::LtEq => {
             let c = compare_values(left, right);
-            Value::Bool(c == std::cmp::Ordering::Less || c == std::cmp::Ordering::Equal)
+            Ok(Value::Bool(
+                c == std::cmp::Ordering::Less || c == std::cmp::Ordering::Equal,
+            ))
         }
-        BinaryOp::And => Value::Bool(is_truthy(left) && is_truthy(right)),
-        BinaryOp::Or => Value::Bool(is_truthy(left) || is_truthy(right)),
+        BinaryOp::And => Ok(Value::Bool(is_truthy(left) && is_truthy(right))),
+        BinaryOp::Or => Ok(Value::Bool(is_truthy(left) || is_truthy(right))),
     }
 }