Skip to content

Commit 5ef9bb4

Browse files
MagicalTuxclaude
andcommitted
feat(vdbe): seek an INNER/LEFT rowid-join with a compound ON clause (B5b-2c)
The live inner-cursor seek (B5b-2a/2b) no longer requires the ipk equality to be the entire `ON`. The `ON` is split into its top-level `AND` conjuncts and the first conjunct that binds the inner table's INTEGER PRIMARY KEY to a plain outer column drives the rowid seek (in either operand order); the full `ON` is still re-evaluated against the assembled row after the seek, so any additional conjunct (`… AND t.name = o.y`) simply filters the seeked row. This rides the same superset invariant as the single-equality case and introduces no new comparison semantics — the seek key still coerces through `to_i64` exactly as the tree-walker's rowid seek does. Deliberately does not extend to a secondary-index seek: `index_seek_rowids` compares raw keys with the index collation and skips the comparison-affinity that `o.x = t.k` applies, so routing it could silently drop a matching row (a false negative that would not fall back) — that needs the tree-walker's affinity machinery first and is left for a later step. Verified VDBE-vs-tree-walker and vs sqlite3 3.50.4 (`tests/vdbe_live_cursor.rs`). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 3be9101 commit 5ef9bb4

3 files changed

Lines changed: 72 additions & 25 deletions

File tree

ROADMAP.md

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1195,10 +1195,19 @@ gated on VDBE-vs-tree-walker parity, so it can't regress correctness):
11951195
order/content to the existing null-padding nested loop, but without scanning or
11961196
materializing the inner table. Same narrow routing as 2a plus `JoinKind::Left`.
11971197
Verified vs the tree-walker and sqlite3 3.50.4.
1198+
- **B5b-2c — compound `ON` seek. DONE.** The ipk equality no longer has to be the
1199+
*whole* `ON` — it may be one top-level `AND` conjunct (`… ON o.x = t.id AND
1200+
t.name = o.y`, in either order). That conjunct drives the rowid seek; the full
1201+
`ON` is re-evaluated after the seek (superset invariant), so the extra conjunct
1202+
just filters the seeked row. Affinity-neutral (same `to_i64` rowid coercion).
1203+
Verified vs the tree-walker and sqlite3 3.50.4.
11981204
- **Remaining:** in-*interpreter* `OpenRead`/`SeekRowid` opcodes over B5b-1's
1199-
multi-cursor foundation (so the seek lives in bytecode, not row-assembly);
1200-
seek by a **secondary index** / `WITHOUT ROWID` PK; the bare-`rowid`-alias `ON`
1201-
spelling (blocked on a pre-existing divergence — `t.rowid` in a join projection
1205+
multi-cursor foundation (so the seek lives in bytecode, not row-assembly); seek
1206+
by a **secondary index** / `WITHOUT ROWID` PK (*affinity-blocked*: `index_seek_rowids`
1207+
compares raw keys with the index collation and skips the comparison-affinity that
1208+
`o.x = t.k` applies, so routing it risks a silent false-negative that would not
1209+
fall back — needs the tree-walker's affinity machinery first); the bare-`rowid`-alias
1210+
`ON` spelling (blocked on a pre-existing divergence — `t.rowid` in a join projection
12021211
already errors on the tree-walker); N-table chains.
12031212
- **B1c — RIGHT/FULL join inner seeks.** INNER/LEFT already seek; RIGHT/FULL still
12041213
materialize the inner table.

src/exec/mod.rs

Lines changed: 50 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2070,21 +2070,15 @@ impl Connection {
20702070
.map(|_| inner_qual.clone())
20712071
.collect();
20722072
let ipk_name = inner_meta.columns[ipk].name.clone();
2073-
// The `ON` must be a single `Eq` (parens stripped) binding the
2074-
// inner ipk column on one side and a plain outer column on the
2075-
// other.
2073+
// The `ON` must contain (as one top-level `AND` conjunct, parens
2074+
// stripped) an `Eq` binding the inner ipk column on one side and a
2075+
// plain outer column on the other. Any additional conjuncts ride
2076+
// the superset invariant: the whole `ON` is re-evaluated after the
2077+
// seek, so an extra `AND t.name = o.y` just filters the seeked row.
20762078
let mut on_expr = on;
20772079
while let sql::ast::Expr::Paren(inner) = on_expr {
20782080
on_expr = inner;
20792081
}
2080-
let (l, r) = match on_expr {
2081-
sql::ast::Expr::Binary {
2082-
op: sql::ast::BinaryOp::Eq,
2083-
left,
2084-
right,
2085-
} => (left.as_ref(), right.as_ref()),
2086-
_ => break 'seek,
2087-
};
20882082
// A plain (optionally parenthesized) unqualified-schema column ref.
20892083
fn col_ref(mut e: &sql::ast::Expr) -> Option<(Option<&str>, &str)> {
20902084
while let sql::ast::Expr::Paren(i) = e {
@@ -2146,18 +2140,53 @@ impl Connection {
21462140
}
21472141
Some(matches[0])
21482142
};
2149-
let oc = if is_inner_ipk(l) {
2150-
match outer_col_index(r) {
2151-
Some(oc) => oc,
2152-
None => break 'seek,
2143+
// Collect the top-level `AND` conjuncts of the `ON` (parens
2144+
// stripped), then find the first that is an `Eq` binding the inner
2145+
// ipk to an outer column — that conjunct drives the rowid seek.
2146+
fn and_conjuncts<'a>(e: &'a sql::ast::Expr, out: &mut Vec<&'a sql::ast::Expr>) {
2147+
let mut e = e;
2148+
while let sql::ast::Expr::Paren(i) = e {
2149+
e = i;
21532150
}
2154-
} else if is_inner_ipk(r) {
2155-
match outer_col_index(l) {
2156-
Some(oc) => oc,
2157-
None => break 'seek,
2151+
if let sql::ast::Expr::Binary {
2152+
op: sql::ast::BinaryOp::And,
2153+
left,
2154+
right,
2155+
} = e
2156+
{
2157+
and_conjuncts(left, out);
2158+
and_conjuncts(right, out);
2159+
} else {
2160+
out.push(e);
21582161
}
2159-
} else {
2160-
break 'seek;
2162+
}
2163+
let mut conjuncts: Vec<&sql::ast::Expr> = Vec::new();
2164+
and_conjuncts(on_expr, &mut conjuncts);
2165+
let seek_key = conjuncts.iter().find_map(|c| {
2166+
let mut c = *c;
2167+
while let sql::ast::Expr::Paren(i) = c {
2168+
c = i;
2169+
}
2170+
let sql::ast::Expr::Binary {
2171+
op: sql::ast::BinaryOp::Eq,
2172+
left,
2173+
right,
2174+
} = c
2175+
else {
2176+
return None;
2177+
};
2178+
let (l, r) = (left.as_ref(), right.as_ref());
2179+
if is_inner_ipk(l) {
2180+
outer_col_index(r)
2181+
} else if is_inner_ipk(r) {
2182+
outer_col_index(l)
2183+
} else {
2184+
None
2185+
}
2186+
});
2187+
let oc = match seek_key {
2188+
Some(oc) => oc,
2189+
None => break 'seek,
21612190
};
21622191
// Combined schema (outer cols ++ inner cols), leftmost outermost.
21632192
let mut c_cols = o_cols.clone();

tests/vdbe_live_cursor.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ fn sqlite3_available() -> bool {
2323

2424
const SCHEMA: &str = "CREATE TABLE o(x, y TEXT);\
2525
INSERT INTO o VALUES(1,'a'),(2,'b'),(3,'c'),(NULL,'n'),(2,'b2'),(2.0,'real'),\
26-
('2','txt'),('2abc','junk'),(x'32','blob'),(2.5,'half'),(99,'miss'),(-1,'neg');\
26+
('2','txt'),('2abc','junk'),(x'32','blob'),(2.5,'half'),(99,'miss'),(-1,'neg'),\
27+
(2,'two'),(5,'five');\
2728
CREATE TABLE t(id INTEGER PRIMARY KEY, name TEXT);\
2829
INSERT INTO t VALUES(1,'one'),(2,'two'),(5,'five');";
2930

@@ -49,6 +50,14 @@ const VDBE_QUERIES: &[&str] = &[
4950
"SELECT o.y FROM o LEFT JOIN t ON o.x = t.id WHERE t.name IS NULL ORDER BY o.y",
5051
"SELECT count(*), count(t.id) FROM o LEFT JOIN t ON o.x = t.id",
5152
"SELECT o.x, t.name FROM o LEFT JOIN t ON o.x = t.id ORDER BY o.y LIMIT 3 OFFSET 2",
53+
// Compound `ON` (B5b-2c): the ipk equality is one `AND` conjunct — it drives
54+
// the rowid seek, the whole `ON` is re-checked so the extra conjunct just
55+
// filters the seeked row. The ipk-eq may be either conjunct.
56+
"SELECT o.x, t.name FROM o JOIN t ON o.x = t.id AND t.name = o.y ORDER BY o.y",
57+
"SELECT o.x, t.name FROM o JOIN t ON t.name = o.y AND o.x = t.id ORDER BY o.y",
58+
"SELECT o.x, t.name FROM o JOIN t ON o.x = t.id AND t.name <> 'one' ORDER BY o.y",
59+
"SELECT o.x, t.name FROM o LEFT JOIN t ON o.x = t.id AND t.name = o.y ORDER BY o.y",
60+
"SELECT count(*) FROM o JOIN t ON o.x = t.id AND o.y = t.name",
5261
];
5362

5463
fn setup() -> Connection {

0 commit comments

Comments
 (0)