r2il: refuse out-of-range lane-compare constants in FoldDialect

crates/lance-graph-r2il/src/binding.rs

@@ -63,8 +63,9 @@
 //! Arguments: `Arg::Field(p)` is the value an earlier effect wrote to `p`,
 //! else the resident lane the caller bound `p` to ([`FieldLanes`]), whose
 //! kind is read from the lane itself. `Arg::Const(c)` is pushed as is. A
-//! constant compared against a lane must fit that lane's type; the dialect
-//! would otherwise saturate it, so the seam refuses it first.
+//! constant compared against a lane must fit that lane's type. The seam
+//! refuses an out-of-range constant here, at bind time; the dialect also
+//! refuses it (`FoldError::ConstOutOfRange`) rather than narrowing it.
 //!
 //! The answer is the value each written field holds after the last effect.
 //! A field that still holds a mask slot or an address is an intermediate
@@ -654,8 +655,9 @@ pub(crate) fn lane(
     Ok(Val::Address(Addr::Lane { idx, kind }))
 }
 
-/// A lane compare's constant must fit the lane's type: the dialect narrows
-/// it with a saturating conversion, which would change the predicate.
+/// A lane compare's constant must fit the lane's type: refused here so the
+/// binding reports it as an `ExecRefusal` before the dialect runs (the dialect
+/// itself refuses it too, with `FoldError::ConstOutOfRange`).
 pub(crate) fn check_compare(stack: &[Val], effect: usize) -> Result<(), ExecRefusal> {
     let [.., a, b] = stack else {
         return Ok(());

crates/lance-graph-r2il/src/lib.rs

@@ -39,15 +39,20 @@
 //! Fold band: `VIA`, `SUM`, `MIN`, `MAX`, `GROUP_SUM`, `KEY_RUNS`, `RANGE`,
 //! `ANY`, `ALL` and `KEEP` are wired. `SCATTER_OR` and `BLEND` are refused as
 //! [`FoldError::Unimplemented`]. `ANY`/`ALL` push 1 or 0; `KEY_RUNS` pushes
-//! `COUNT(DISTINCT key)` over a key-ordered lane.
+//! `COUNT(DISTINCT key)` over a key-ordered lane, provided the mask already
+//! excludes NULL keys: the lane carries no validity, so a selected NULL row
+//! counts its payload ([`procs::count_distinct_sorted`] ANDs the presence
+//! plane in).
 //!
 //! `MIN`/`MAX` of an empty selection push the fold's identity, `i64::MAX` /
 //! `i64::MIN`: the same seed mask-risc's grouped `MIN`/`MAX` leave in an
 //! empty group. No `I32` value equals it, so a caller reads it as SQL NULL.
 //!
 //! R2IL band: `Load` (spaces 0 = U32, 1 = I32, 2 = U64, 3 = resident mask
 //! plane), `IntAdd`, `IntSub`, `IntMult`, `IntSDiv` (scalar/scalar, checked), `IntAnd` (with survivor gating),
-//! `IntOr`, `IntXor`, `IntNot`, `IntEqual`, `IntSLess` and `PopCount` are wired. `IntLess` and
+//! `IntOr`, `IntXor`, `IntNot`, `IntEqual`, `IntSLess` and `PopCount` are wired. A lane
+//! compare's constant that does not fit the lane's type is refused as
+//! [`FoldError::ConstOutOfRange`], never narrowed. `IntLess` and
 //! `IntLessEqual` are refused as [`FoldError::NoUnsignedLaneCompare`]
 //! (mask-risc has no unsigned lane compare); `IntSub` of two addresses as
 //! [`FoldError::LaneVersusLane`]; an unknown `Load` space as
@@ -252,6 +257,23 @@ pub enum FoldError {
     /// `numeric_value_out_of_range` for `bigint`; the dialect refuses rather
     /// than wrap.
     Overflow(FnIndex),
+    /// A lane compare's constant does not fit the lane's type: `IntEqual`
+    /// against a `U32` lane (or a `VIA` join's `U32` key) with a value outside
+    /// `0..=u32::MAX`, or `IntEqual`/`IntSLess` against an `I32` lane with a
+    /// value outside `i32::MIN..=i32::MAX`. Narrowing it would compare against
+    /// a different number (a negative constant would match `u32::MAX`), so
+    /// the call is refused. Carries the opcode and the constant.
+    ConstOutOfRange(FnIndex, i64),
+}
+
+/// The constant of a `U32` lane compare, or [`FoldError::ConstOutOfRange`].
+fn const_u32(f: FnIndex, x: i64) -> Result<u32, FoldError> {
+    u32::try_from(x).map_err(|_| FoldError::ConstOutOfRange(f, x))
+}
+
+/// The constant of an `I32` lane compare, or [`FoldError::ConstOutOfRange`].
+fn const_i32(f: FnIndex, x: i64) -> Result<i32, FoldError> {
+    i32::try_from(x).map_err(|_| FoldError::ConstOutOfRange(f, x))
 }
 
 /// How a body's Boolean calls (`IntAnd`, `IntOr`, `IntNot` on masks) are
@@ -902,7 +924,7 @@ impl FoldDialect<'_> {
                         self.emit_pred(
                             Pred::EqU32 {
                                 lane: idx,
-                                v: u32::try_from(x).unwrap_or(u32::MAX),
+                                v: const_u32(f, x)?,
                             },
                             stack,
                         );
@@ -925,7 +947,7 @@ impl FoldDialect<'_> {
                         self.emit_pred(
                             Pred::EqI32 {
                                 lane: idx,
-                                v: i32::try_from(x).unwrap_or(i32::MAX),
+                                v: const_i32(f, x)?,
                             },
                             stack,
                         );
@@ -937,7 +959,7 @@ impl FoldDialect<'_> {
                             Pred::EqU32Via {
                                 fk,
                                 key,
-                                v: u32::try_from(x).unwrap_or(u32::MAX),
+                                v: const_u32(f, x)?,
                             },
                             stack,
                         );
@@ -969,7 +991,7 @@ impl FoldDialect<'_> {
                         self.emit_pred(
                             Pred::LtI32 {
                                 lane: idx,
-                                t: i32::try_from(t).unwrap_or(i32::MAX),
+                                t: const_i32(f, t)?,
                             },
                             stack,
                         );
@@ -985,7 +1007,7 @@ impl FoldDialect<'_> {
                         self.emit_pred(
                             Pred::GtI32 {
                                 lane: idx,
-                                t: i32::try_from(t).unwrap_or(i32::MAX),
+                                t: const_i32(f, t)?,
                             },
                             stack,
                         );
@@ -1164,6 +1186,12 @@ impl FoldDialect<'_> {
             // (`Terminal::CountKeyRunsU32`, O(1) state, no seen-set). An
             // unordered lane is refused by mask-risc (`LaneNotOrdered`),
             // never answered. Operands: mask, then the key lane.
+            //
+            // Precondition: the mask excludes every row whose key is NULL.
+            // The lane has no validity of its own, so a NULL row's payload
+            // (a sentinel, or a value equal to a real key) counts as a key
+            // if its row is selected. AND the key's presence plane into the
+            // mask first, as `procs::count_distinct_sorted` does.
             KEY_RUNS => {
                 let key = stack.pop().ok_or(FoldError::Underflow(f))?;
                 let m = fold_mask(stack.pop().ok_or(FoldError::Underflow(f))?)

crates/lance-graph-r2il/src/polyfill.rs

@@ -17,6 +17,11 @@
 //! identity (`0`, `i64::MAX`, `i64::MIN`); a three-valued binding is refused
 //! by [`crate::binding::prepare`], so `NULL` never reaches it.
 //!
+//! `KEY_RUNS` is `COUNT DISTINCT` only over a key-ordered lane under a mask
+//! that already excludes NULL keys. The lane has no validity of its own, so a
+//! selected NULL row counts its payload as a key;
+//! [`crate::procs::count_distinct_sorted`] ANDs the key's presence plane in.
+//!
 //! The positions are the [`crate::binding::EXECUTABLE`] opcodes in that
 //! order. The opcode, arity and R2IL name of each entry are checked against
 //! OGAR's `R2ILVocabulary` by test, not at run time.

crates/lance-graph-r2il/tests/dialect.rs

@@ -482,3 +482,198 @@ fn call_owned_reuses_consumed_slots_and_call_does_not() {
         "plain call must keep one slot per result, got {r:?}"
     );
 }
+
+/// One lane compare followed by `PopCount`: the operands are pushed in the
+/// given order (`lhs` then `rhs`), so `(addr, c)` is `lane OP c` and
+/// `(c, addr)` is `c OP lane`.
+fn compare_count(
+    planes: &Planes<'_>,
+    foreign: &Foreign<'_>,
+    f: FnIndex,
+    lhs: Val,
+    rhs: Val,
+) -> Result<i64, FoldError> {
+    let mut d = FoldDialect::new(planes, foreign);
+    let mut stack = vec![lhs, rhs];
+    call(&mut d, f, [0; 3], &mut stack)?;
+    call(&mut d, POP_COUNT, [0; 3], &mut stack)?;
+    match stack.as_slice() {
+        [Val::Scalar(n)] => Ok(*n),
+        other => panic!("PopCount left {other:?}"),
+    }
+}
+
+fn u32_lane(idx: u16) -> Val {
+    Val::Address(Addr::Lane {
+        idx,
+        kind: lance_graph_mask_risc::LaneKind::U32,
+    })
+}
+
+/// FAILS IF: a lane compare's constant that fits the lane's type stops
+/// behaving as before, including at both ends of each type's range.
+#[test]
+fn in_range_compare_constants_behave_as_before() {
+    let codes: Vec<u32> = vec![u32::MAX, 5, 5, 0];
+    let ints: Vec<i32> = vec![i32::MIN, -5, 0, 7, i32::MAX];
+    let lanes = [LaneRef::U32(&codes), LaneRef::I32(&ints)];
+    let planes = Planes {
+        n_rows: 4,
+        masks: &[],
+        lanes: &lanes[..1],
+    };
+    let iplanes = Planes {
+        n_rows: 5,
+        masks: &[],
+        lanes: &lanes[1..],
+    };
+    let f = Foreign::NONE;
+    let s = |x: i64| Val::Scalar(x);
+    let eq_u32 = |x| compare_count(&planes, &f, INT_EQUAL, u32_lane(0), s(x));
+    assert_eq!(eq_u32(5), Ok(2));
+    assert_eq!(eq_u32(0), Ok(1));
+    assert_eq!(eq_u32(i64::from(u32::MAX)), Ok(1));
+    // Symmetric: the constant pushed first.
+    assert_eq!(
+        compare_count(&planes, &f, INT_EQUAL, s(5), u32_lane(0)),
+        Ok(2)
+    );
+    let eq_i32 = |x| compare_count(&iplanes, &f, INT_EQUAL, i32_lane(0), s(x));
+    assert_eq!(eq_i32(i64::from(i32::MIN)), Ok(1));
+    assert_eq!(eq_i32(i64::from(i32::MAX)), Ok(1));
+    assert_eq!(eq_i32(-5), Ok(1));
+    // `lane < t` and `t < lane`.
+    let lt = |t| compare_count(&iplanes, &f, INT_S_LESS, i32_lane(0), s(t));
+    let gt = |t| compare_count(&iplanes, &f, INT_S_LESS, s(t), i32_lane(0));
+    assert_eq!(lt(i64::from(i32::MIN)), Ok(0));
+    assert_eq!(lt(i64::from(i32::MAX)), Ok(4));
+    assert_eq!(lt(0), Ok(2));
+    assert_eq!(gt(i64::from(i32::MIN)), Ok(4));
+    assert_eq!(gt(i64::from(i32::MAX)), Ok(0));
+    assert_eq!(gt(0), Ok(2));
+}
+
+/// FAILS IF: a lane compare's out-of-range constant is narrowed instead of
+/// refused. Before the refusal the dialect saturated: `-1` against a `U32`
+/// lane matched the row holding `u32::MAX` (1 instead of 0), and
+/// `lane < i32::MIN - 1` selected every row (3 instead of 0). A large
+/// NEGATIVE constant against an `I32` lane saturated to `i32::MAX`, not
+/// `i32::MIN`. Disable: put the `try_from(..).unwrap_or(MAX)` narrowing back.
+#[test]
+fn out_of_range_compare_constants_are_refused() {
+    let codes: Vec<u32> = vec![u32::MAX, 5, 5];
+    let ints: Vec<i32> = vec![-5, 0, 7, i32::MAX];
+    let u = [LaneRef::U32(&codes)];
+    let i = [LaneRef::I32(&ints)];
+    let planes = Planes {
+        n_rows: 3,
+        masks: &[],
+        lanes: &u,
+    };
+    let iplanes = Planes {
+        n_rows: 4,
+        masks: &[],
+        lanes: &i,
+    };
+    let f = Foreign::NONE;
+    let s = |x: i64| Val::Scalar(x);
+    let refused = |op, x| Err(FoldError::ConstOutOfRange(op, x));
+
+    // U32 lane, IntEqual, both sides of the range and both operand orders.
+    let above_u32 = i64::from(u32::MAX) + 1;
+    for x in [-1, i64::MIN, above_u32, i64::MAX] {
+        assert_eq!(
+            compare_count(&planes, &f, INT_EQUAL, u32_lane(0), s(x)),
+            refused(INT_EQUAL, x),
+            "lane = {x}"
+        );
+        assert_eq!(
+            compare_count(&planes, &f, INT_EQUAL, s(x), u32_lane(0)),
+            refused(INT_EQUAL, x),
+            "{x} = lane"
+        );
+    }
+
+    // I32 lane, IntEqual and IntSLess. `above` would have saturated onto the
+    // row holding `i32::MAX`; `below` too, because the old narrowing mapped
+    // every failure to `i32::MAX`.
+    let above = i64::from(i32::MAX) + 1;
+    let below = i64::from(i32::MIN) - 1;
+    for x in [above, below, i64::MIN, i64::MAX] {
+        for (op, lhs, rhs) in [
+            (INT_EQUAL, i32_lane(0), s(x)),
+            (INT_EQUAL, s(x), i32_lane(0)),
+            (INT_S_LESS, i32_lane(0), s(x)),
+            (INT_S_LESS, s(x), i32_lane(0)),
+        ] {
+            assert_eq!(
+                compare_count(&iplanes, &f, op, lhs, rhs),
+                refused(op, x),
+                "{op:?} with {x}"
+            );
+        }
+    }
+}
+
+/// FAILS IF: an `IntEqual` against a `VIA` join's `U32` key narrows an
+/// out-of-range constant instead of refusing it, or stops matching an
+/// in-range one at the top of the range.
+#[test]
+fn via_compare_constants_are_range_checked() {
+    // This table's fk lane names a row of the foreign key lane.
+    let fk: Vec<u32> = vec![0, 1, 1, 2];
+    let keys: Vec<u32> = vec![u32::MAX, 5, 9];
+    let lanes = [LaneRef::U32(&fk)];
+    let flanes = [LaneRef::U32(&keys)];
+    let planes = Planes {
+        n_rows: 4,
+        masks: &[],
+        lanes: &lanes,
+    };
+    let foreign = Foreign {
+        planes: &[],
+        lanes: &flanes,
+    };
+    let via = Val::Address(Addr::Via { fk: 0, key: 0 });
+    let count = |x: i64| compare_count(&planes, &foreign, INT_EQUAL, via, Val::Scalar(x));
+    assert_eq!(count(5), Ok(2));
+    assert_eq!(count(i64::from(u32::MAX)), Ok(1));
+    for x in [-1, i64::from(u32::MAX) + 1] {
+        assert_eq!(count(x), Err(FoldError::ConstOutOfRange(INT_EQUAL, x)));
+    }
+}
+
+/// FAILS IF: `KEY_RUNS`'s documented precondition stops holding as stated.
+/// The key lane carries no validity: a selected row whose key is NULL counts
+/// its payload, so the sorted lane `(NULL as 0, 1, 1, 2)` under an
+/// all-rows mask gives 3, where SQL `count(DISTINCT key)` gives 2. With the
+/// key's presence plane ANDed into the mask it gives 2.
+#[test]
+fn key_runs_counts_a_selected_null_key_unless_the_mask_excludes_it() {
+    let key: Vec<u32> = vec![0, 1, 1, 2];
+    let lanes = [LaneRef::U32(&key)];
+    let all = [0b1111u64];
+    let valid = [0b1110u64]; // row 0 is NULL
+    let masks: [&[u64]; 2] = [&all, &valid];
+    let planes = Planes {
+        n_rows: 4,
+        masks: &masks,
+        lanes: &lanes,
+    };
+    let foreign = Foreign::NONE;
+    let runs = |gate: bool| {
+        let mut d = FoldDialect::new(&planes, &foreign);
+        let mut stack = vec![Val::Scalar(0)];
+        call(&mut d, LOAD, [3, 0, 0], &mut stack).expect("LOAD plane 0");
+        if gate {
+            stack.push(Val::Scalar(1));
+            call(&mut d, LOAD, [3, 0, 0], &mut stack).expect("LOAD plane 1");
+            call(&mut d, INT_AND, [0; 3], &mut stack).expect("IntAnd");
+        }
+        stack.push(u32_lane(0));
+        call(&mut d, KEY_RUNS, [0; 3], &mut stack).expect("KEY_RUNS");
+        stack
+    };
+    assert_eq!(runs(false), vec![Val::Scalar(3)], "NULL row counted");
+    assert_eq!(runs(true), vec![Val::Scalar(2)], "NULL row masked out");
+}