Merge pull request #84 from ilyakooo0/push-quqskwyyyqnm

fix: where filter on source relations silently ignored or panics

Author: ilyakooo0

crates/knot-compiler/src/codegen.rs

@@ -8526,6 +8526,37 @@ impl Codegen {
             })
     }
 
+    /// Does a `where` in these statements filter on `name`'s FIELDS?
+    ///
+    /// Only a per-row reading gives such a `where` any meaning: as a guard over
+    /// the whole relation, `u.age` is the FIRST row's age (field access on a
+    /// relation delegates to its first element), so the guard either waves every
+    /// row through or drops the block entirely. Seeing one therefore settles
+    /// `x <- *rel` as a comprehension bind even when the name is also used as a
+    /// whole value — `yield u` yields the ROW.
+    ///
+    /// A `where` that uses `name` as a value (`where count people > 3`) is a
+    /// real guard on the relation and is deliberately not counted. The scan
+    /// stops at a statement that rebinds `name`, past which the uses belong to
+    /// a different binding.
+    fn where_filters_row_fields(stmts: &[ast::Stmt], name: &str) -> bool {
+        for stmt in stmts {
+            match &stmt.node {
+                ast::StmtKind::Where { cond } => {
+                    if expr_refs_var(cond, name) && !expr_uses_var_as_value(cond, name) {
+                        return true;
+                    }
+                }
+                ast::StmtKind::Bind { pat, .. } | ast::StmtKind::Let { pat, .. }
+                    if pat_bound_names(pat).iter().any(|n| n == name) => {
+                    return false;
+                }
+                _ => {}
+            }
+        }
+        false
+    }
+
     /// Like `do_block_is_comprehension`, but also admits `groupBy`
     /// statements: a do-block ending in `yield` whose other statements are
     /// all Bind/Where/Let/GroupBy compiles through the relational loop
@@ -8830,9 +8861,30 @@ impl Codegen {
                     //                  yield (filter (\p -> p.age > 65) people) }`
                     // passes the name on as the whole relation, while the
                     // comprehension above only ever reads fields off it. Iterate
-                    // exactly when every use is a field access on the name —
-                    // referenced, but never as a value — which is meaningless
-                    // under whole-relation semantics anyway ("first row's age").
+                    // when every use is a field access on the name — referenced,
+                    // but never as a value — which is meaningless under
+                    // whole-relation semantics anyway ("first row's age").
+                    //
+                    // That rule alone misses the most ordinary comprehension of
+                    // all, `yield x` (yield the ROW):
+                    //
+                    //   &adults = do { u <- *users; where u.age >= 25; yield u }
+                    //
+                    // `yield u` uses the name as a value, so the block fell back
+                    // to whole-relation semantics and the `where` became a guard
+                    // on the FIRST row: `u.age >= 25` was true for row 1, the
+                    // guard passed, and `yield u` handed back the WHOLE relation
+                    // — every user, filter silently ignored. Flip the predicate
+                    // to one row 1 fails and the guard drops the whole block to
+                    // `{}`, so the caller's `count` panicked on a non-relation.
+                    //
+                    // A `where` that reads FIELDS off the bound name is itself
+                    // the proof that the bind iterates: as a whole-relation guard
+                    // it can only mean "the first row's field", which is never
+                    // what a filter means. So it settles the reading regardless
+                    // of how `yield` uses the name. A `where` over the name as a
+                    // VALUE (`where count people > 3`) is a genuine guard on the
+                    // relation and keeps the whole-relation reading.
                     let comprehension_tail = rhs_is_io_relation_source
                         && Self::do_block_is_comprehension(&stmts[stmt_idx..])
                         && match &pat.node {
@@ -8841,8 +8893,12 @@ impl Codegen {
                                     ast::ExprKind::Do(stmts[stmt_idx + 1..].to_vec()),
                                     stmt.span,
                                 );
-                                expr_refs_var(&tail, name)
-                                    && !expr_uses_var_as_value(&tail, name)
+                                (expr_refs_var(&tail, name)
+                                    && !expr_uses_var_as_value(&tail, name))
+                                    || Self::where_filters_row_fields(
+                                        &stmts[stmt_idx + 1..],
+                                        name,
+                                    )
                             }
                             _ => false,
                         };

crates/knot-compiler/tests/regress_knot_issues.rs

@@ -682,3 +682,84 @@ main = do
         "both derived impls should work, got: {stdout}",
     );
 }
+
+// ── 13. `where` over a source relation, yielding the whole row ────
+
+/// `x <- *rel` reads either as a comprehension bind (iterate ROWS) or as a
+/// whole-relation bind, and codegen picks between them from how the block uses
+/// the name. `yield u` — the most ordinary comprehension there is — looks like
+/// a whole-relation use, so the block took the guard reading: `where u.age >=
+/// 25` asked the FIRST row's age, waved every row through, and `yield u` handed
+/// back the entire relation with the filter silently dropped.
+const ADULTS: &str = "type User = {name: Text, age: Int}\n\
+                      *users : [User]\n\
+                      &adults = do\n\
+                      \x20 u <- *users\n\
+                      \x20 where u.age >= 25\n\
+                      \x20 yield u\n";
+
+#[test]
+fn a_where_filters_the_rows_of_a_source_relation() {
+    let src = format!(
+        "{ADULTS}\
+         main = do\n\
+         \x20 replace *users = [{{name: \"Alice\", age: 30}}, {{name: \"Bob\", age: 20}}, {{name: \"Carol\", age: 40}}]\n\
+         \x20 a <- &adults\n\
+         \x20 forEach a (\\u -> println u.name)\n\
+         \x20 yield {{}}\n"
+    );
+    let (stdout, stderr, ok) = compile_and_run("where_filters_source", &src, &[]);
+    assert!(ok, "the program must run: {stderr}");
+    assert!(stdout.contains("Alice"), "a row passing the where must be yielded: {stdout:?}");
+    assert!(stdout.contains("Carol"), "a row passing the where must be yielded: {stdout:?}");
+    assert!(
+        !stdout.contains("Bob"),
+        "a row failing the where must be filtered out, got: {stdout:?}",
+    );
+}
+
+/// The same misreading, with a predicate the first row fails: the guard was
+/// false, so the whole block collapsed to `{}` and the caller's `count` met a
+/// Unit where it expected a relation ("expected Relation in len, got Unit").
+/// Filtering every row out must give an empty relation, not a non-relation.
+#[test]
+fn a_where_that_no_row_satisfies_yields_an_empty_relation() {
+    let src = "type User = {name: Text, age: Int}\n\
+               *users : [User]\n\
+               &adults = do\n\
+               \x20 u <- *users\n\
+               \x20 where u.age >= 99\n\
+               \x20 yield u\n\
+               main = do\n\
+               \x20 replace *users = [{name: \"Alice\", age: 30}]\n\
+               \x20 a <- &adults\n\
+               \x20 println (\"count: \" ++ show (count a))\n\
+               \x20 yield {}\n";
+    let (stdout, stderr, ok) = compile_and_run("where_matches_nothing", src, &[]);
+    assert!(ok, "filtering every row out must not panic: {stderr}");
+    assert!(stdout.contains("count: 0"), "the result must be empty, got: {stdout:?}");
+}
+
+/// The other reading of `x <- *rel` — bind the WHOLE relation and pass it on as
+/// a value (DESIGN.md's `&seniors`) — must survive the fix. Here a `where` over
+/// the name *as a value* is a genuine guard on the relation, not a row filter.
+#[test]
+fn a_where_over_the_relation_as_a_value_stays_a_guard() {
+    let src = "type Person = {name: Text, age: Int}\n\
+               *people : [Person]\n\
+               &seniorsIfCrowd = do\n\
+               \x20 people <- *people\n\
+               \x20 where count people > 1\n\
+               \x20 yield (filter (\\p -> p.age > 65) people)\n\
+               main = do\n\
+               \x20 replace *people = [{name: \"Alice\", age: 70}, {name: \"Bob\", age: 20}]\n\
+               \x20 s <- &seniorsIfCrowd\n\
+               \x20 forEach s (\\p -> println p.name)\n\
+               \x20 yield {}\n";
+    let (stdout, stderr, ok) = compile_and_run("where_as_relation_guard", src, &[]);
+    assert!(ok, "the program must run: {stderr}");
+    assert!(
+        stdout.contains("Alice") && !stdout.contains("Bob"),
+        "the whole relation must reach `filter`, which keeps only Alice, got: {stdout:?}",
+    );
+}