Skip to content

Commit bc4a281

Browse files
MagicalTuxclaude
andcommitted
feat(eqp): emit the GROUP BY/DISTINCT temp-b-tree over a rowid range seek (B9d subset)
The `USE TEMP B-TREE FOR GROUP BY` / `FOR DISTINCT` node now also materializes over an unambiguous rowid range seek (`WHERE a>? GROUP BY c` → `SEARCH t USING INTEGER PRIMARY KEY (rowid>?)#USE TEMP B-TREE FOR GROUP BY`), not just a bare SCAN. The rowid is the table's own clustered key, so there is no secondary-index choice and thus no cost-model divergence, and the seek returns rows in rowid order — never the group/distinct-key order — so the b-tree is always needed. Gated on a rowid *range* seek line (an equality seek is a single row and stays excluded), reusing group_distinct_btree's existing secondary-index-leads guard. The secondary-index-seek variant, and all single-table index *choice* (B9h) and non-default-collation index selection (B9j), are deferred by design: they depend on SQLite's stat4-less cost model, which the pinned oracle makes un-differential-testable (same class as B1b/B4). Documented precisely in ROADMAP.md and the planner-index-seeks memory. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent a358e80 commit bc4a281

3 files changed

Lines changed: 148 additions & 14 deletions

File tree

ROADMAP.md

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1204,10 +1204,19 @@ tie/representative order), so they are perf/EQP-fidelity work, not correctness:
12041204
over a `LIMIT` body still declines — SQLite full-sorts the materialized `LIMIT` rows,
12051205
whereas pushing the ORDER BY down would render a partial `LAST TERM` index walk when
12061206
the leading prefix is indexed. Boundary rules in the `eqp-derived-coroutine` memory.
1207-
- **B9d — `SEARCH` + `GROUP BY`/`DISTINCT` temp-b-tree node.** graphite emits the
1208-
grouping b-tree only over a bare `SCAN`; under a `WHERE` seek it omits the node.
1209-
Entangled with B9h (SQLite often picks a *different* index whose walk serves the
1210-
grouping), so it should land together with the index-choice work.
1207+
- **B9d — `SEARCH` + `GROUP BY`/`DISTINCT` temp-b-tree node. ◐ Unambiguous subset
1208+
done.** The grouping b-tree now also materializes over a **rowid *range* seek**
1209+
(`WHERE a>? GROUP BY c` → `SEARCH t USING INTEGER PRIMARY KEY (rowid>?)#USE TEMP
1210+
B-TREE FOR GROUP BY`, and the `DISTINCT` analogue) — the rowid is the table's own
1211+
clustered key, so there is no secondary-index *choice* and thus no cost-model
1212+
divergence; the seek returns rows in rowid order, never the group/distinct-key
1213+
order, so the b-tree is always needed. Gated in `eqp_select` on a
1214+
`SEARCH … INTEGER PRIMARY KEY (rowid>?/<?)` access line (a rowid *equality* seek is a
1215+
single row → excluded), reusing `group_distinct_btree`'s existing "a secondary index
1216+
leads the first key column → decline" guard. *Still open (folded into B9h):* the
1217+
same node under a **secondary-index** seek (`WHERE b=? GROUP BY c`), where SQLite may
1218+
pick a *different* composite index `(b,c)` whose walk serves the grouping — a
1219+
cost-model index-choice decision.
12111220
- **B9e — `col = (scalar subquery)` seek. ✅ Done (SELECT).** `WHERE b = (SELECT …)`
12121221
(and `>`/`<`/etc.) against a non-correlated scalar subquery now seeks — the executor
12131222
folds the subquery to its value before the seek (`scan_source` single-table fast path),
@@ -1245,9 +1254,13 @@ tie/representative order), so they are perf/EQP-fidelity work, not correctness:
12451254
longest-equality-prefix only. It also decides *whether* a no-WHERE query uses a
12461255
covering scan at all (SQLite: narrow index beats a wide-row table scan, plain scan
12471256
beats it on a 2-column table) — so the covering-scan row-order parity (formerly B9i)
1248-
rides here too. This changes the chosen access path, so it risks regressing the EQP
1249-
corpus and must be rolled out shape-by-shape with the full differential suite — the
1250-
single-table analogue of B1b.
1257+
rides here too, as does the **secondary-index** `SEARCH` + `GROUP BY`/`DISTINCT`
1258+
b-tree left open by B9d. This changes the chosen access path, so it risks regressing
1259+
the EQP corpus and must be rolled out shape-by-shape with the full differential suite
1260+
— the single-table analogue of B1b. **Confirmed deferred by design (2026-07-04):** the
1261+
pinned `sqlite3 3.50.4` oracle has no stat4, so its choices depend on row-width /
1262+
index-width / index-count heuristics graphite can't reproduce without diverging the
1263+
EQP corpus — same class as B1b/B4. Needs a stat4-enabled oracle to become tractable.
12511264
- **B9i — covering-scan no-`ORDER BY` row order → subsumed by B9h (investigated
12521265
2026-07-04, nothing to fix in isolation).** The original premise was wrong: graphite's
12531266
covering read is *already* in index-walk order, and whenever graphite and SQLite pick
@@ -1261,12 +1274,22 @@ tie/representative order), so they are perf/EQP-fidelity work, not correctness:
12611274
with two, so the plan **and** the emitted order differ. Reproducing it is pure cost
12621275
modelling (row width, index width, number of indexes) — the same B9h/B4 problem, so
12631276
the row-order parity rides on B9h, not a separate execution-order change.
1264-
- **B9j — collation-aware index *selection* for a non-default-collation index.**
1265-
`collect_eq_constraints`/`collect_range_constraints` now compare an explicit
1266-
`COLLATE` to the *column's* collation (fixes the common bug); a `CREATE INDEX …
1267-
(b COLLATE NOCASE)` on a BINARY column plus a matching `COLLATE NOCASE` predicate
1268-
could seek that index, but graphite scans. Fully correct needs per-candidate-index
1269-
collation comparison threaded through every seek site.
1277+
- **B9j — collation-aware index *selection* for a non-default-collation index.
1278+
Deferred (entangled, rows already correct).** `collect_eq_constraints` /
1279+
`collect_range_constraints` compare an explicit `COLLATE` to the *column's*
1280+
collation. When an index carries a *non-default* collation (`CREATE INDEX ib ON
1281+
t(b COLLATE NOCASE)` on a BINARY column), graphite is wrong in **both** directions vs
1282+
sqlite: `b='x' COLLATE NOCASE` should seek `ib` (graphite scans), and `b='x'` (BINARY
1283+
comparison) should *not* seek the NOCASE `ib` (graphite over-seeks it — rows still
1284+
correct via the WHERE re-apply, only EQP/perf). The correct model — an index serves a
1285+
comparison iff its per-column collation equals the comparison's *effective* collation
1286+
(explicit `COLLATE`, else the column's) — must be threaded into the index *selection*
1287+
at every one of the ~9 `collect_eq_constraints` call sites (the seek fast paths,
1288+
`eqp_access`, and `seek_order_prefix`'s ORDER-BY credit) in lockstep. The current
1289+
column-collation gate in the collector is itself an earlier ORDER-BY-ordering
1290+
correctness fix, so moving the check risks the extensive collation/seek/order suite
1291+
for a niche pattern with rows already correct. Deferred; a careful cross-cutting
1292+
refactor, not a quick slice.
12701293

12711294
**Blocked / deferred by design:**
12721295
- **B1b — cost-based join reordering.** graphite's per-cursor seek/bloom-filter

src/exec/mod.rs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14593,9 +14593,24 @@ impl Connection {
1459314593
// clean single-table bare-`SCAN` case, placed after the SCAN line and
1459414594
// before any ORDER BY node — matching sqlite's node order.
1459514595
let mut group_btree_suppresses_order = false;
14596+
// The GROUP BY / DISTINCT temp-b-tree also materializes over an *unambiguous*
14597+
// access path that doesn't already yield the grouping order: a bare `SCAN`, or
14598+
// a rowid RANGE seek (`SEARCH … INTEGER PRIMARY KEY (rowid>?…)`), which returns
14599+
// rows in rowid order — never the group/distinct-key order — and, being the
14600+
// table's own clustered key, involves no secondary-index *choice* (so no
14601+
// cost-model divergence, unlike a secondary-index seek — see roadmap B9h). A
14602+
// rowid *equality* seek is a single row (grouping is a no-op), so it is
14603+
// excluded. `group_distinct_btree`'s own "a secondary index leads the first key
14604+
// column" guard still declines the shapes where sqlite would walk an index.
14605+
let rowid_range_seek = single_scan_detail.as_deref().is_some_and(|d| {
14606+
d.starts_with(&alloc::format!(
14607+
"SEARCH {label} USING INTEGER PRIMARY KEY (rowid"
14608+
)) && (d.contains('>') || d.contains('<'))
14609+
});
1459614610
// A no-op `DISTINCT` (its projection pins the rowid/IPK, so it removes
1459714611
// nothing) is planned by sqlite as if absent — no `FOR DISTINCT` node.
14598-
if single_scan_detail.as_deref() == Some(alloc::format!("SCAN {label}").as_str())
14612+
if (single_scan_detail.as_deref() == Some(alloc::format!("SCAN {label}").as_str())
14613+
|| rowid_range_seek)
1459914614
&& !self.distinct_is_noop(sel, &meta, &label)
1460014615
{
1460114616
if let Some((kind, suppress)) =

tests/eqp_seek_group_btree.rs

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
//! B9d (unambiguous subset) — the `USE TEMP B-TREE FOR GROUP BY` / `FOR DISTINCT`
2+
//! node materializes over a rowid *range* seek, not just a bare `SCAN`. A
3+
//! `WHERE a>? GROUP BY c` (a the INTEGER PRIMARY KEY, c not the seek key) seeks the
4+
//! rowid range — an unambiguous access path with no secondary-index *choice* — and
5+
//! returns rows in rowid order, which is never the grouping order, so SQLite (and now
6+
//! graphite) emit the grouping b-tree: `SEARCH t USING INTEGER PRIMARY KEY (rowid>?)#
7+
//! USE TEMP B-TREE FOR GROUP BY`. graphite previously omitted the node under any seek.
8+
//!
9+
//! A rowid *equality* seek (single row → grouping is a no-op), a GROUP BY on the rowid
10+
//! itself, and a secondary-index seek (where SQLite may pick a different composite
11+
//! index — cost-model, roadmap B9h) all stay as before. Verified vs sqlite3 3.50.4.
12+
13+
#![cfg(feature = "std")]
14+
15+
use std::process::Command;
16+
17+
fn sqlite3_available() -> bool {
18+
Command::new("sqlite3").arg("--version").output().is_ok()
19+
}
20+
21+
fn plan(bin: &str, base: &str, sql: &str) -> String {
22+
let full = format!("{base} EXPLAIN QUERY PLAN {sql}");
23+
let out = Command::new(bin)
24+
.arg(":memory:")
25+
.arg(&full)
26+
.output()
27+
.unwrap();
28+
String::from_utf8_lossy(&out.stdout)
29+
.lines()
30+
.filter(|l| !l.trim().is_empty() && !l.starts_with("QUERY PLAN"))
31+
.map(|l| l.trim_start_matches(|c: char| " |`*+_-".contains(c)))
32+
.collect::<Vec<_>>()
33+
.join("#")
34+
}
35+
36+
fn rows(bin: &str, base: &str, sql: &str) -> String {
37+
let full = format!("{base} {sql}");
38+
let out = Command::new(bin)
39+
.arg(":memory:")
40+
.arg(&full)
41+
.output()
42+
.unwrap();
43+
String::from_utf8_lossy(&out.stdout).trim_end().to_string()
44+
}
45+
46+
// A single secondary index (on b) so the rowid-seek cases are unambiguous; c is
47+
// deliberately un-indexed so no index serves the GROUP BY / DISTINCT.
48+
const SCHEMA: &str = "CREATE TABLE t(a INTEGER PRIMARY KEY, b, c); CREATE INDEX ib ON t(b);";
49+
50+
#[test]
51+
fn rowid_range_seek_group_distinct_btree_matches_sqlite() {
52+
if !sqlite3_available() {
53+
eprintln!("sqlite3 CLI not found; skipping");
54+
return;
55+
}
56+
let g = env!("CARGO_BIN_EXE_graphitesql");
57+
for q in [
58+
// Now emit the b-tree over the rowid range seek.
59+
"SELECT c FROM t WHERE a>3 GROUP BY c",
60+
"SELECT c FROM t WHERE a>3 AND a<9 GROUP BY c",
61+
"SELECT DISTINCT c FROM t WHERE a>3",
62+
"SELECT c, count(*) FROM t WHERE a>=2 GROUP BY c",
63+
"SELECT c FROM t WHERE a>3 GROUP BY c ORDER BY c", // ORDER BY folded into the b-tree
64+
// Unchanged: rowid equality (single row → no b-tree), GROUP BY the rowid,
65+
// bare scan, and a secondary-index-served grouping.
66+
"SELECT c FROM t WHERE a=3 GROUP BY c",
67+
"SELECT c FROM t WHERE a>3 GROUP BY a",
68+
"SELECT c FROM t GROUP BY c",
69+
"SELECT DISTINCT c FROM t",
70+
] {
71+
assert_eq!(
72+
plan("sqlite3", SCHEMA, q),
73+
plan(g, SCHEMA, q),
74+
"plan for {q}"
75+
);
76+
}
77+
}
78+
79+
#[test]
80+
fn rowid_range_seek_group_rows_match() {
81+
if !sqlite3_available() {
82+
eprintln!("sqlite3 CLI not found; skipping");
83+
return;
84+
}
85+
let g = env!("CARGO_BIN_EXE_graphitesql");
86+
let base = format!(
87+
"{SCHEMA} INSERT INTO t VALUES(1,10,5),(2,20,5),(3,30,7),(4,40,5),(5,50,7),(6,60,9);"
88+
);
89+
for q in [
90+
"SELECT c FROM t WHERE a>3 GROUP BY c ORDER BY c",
91+
"SELECT DISTINCT c FROM t WHERE a>2 ORDER BY c",
92+
"SELECT c, count(*) FROM t WHERE a>1 GROUP BY c ORDER BY c",
93+
] {
94+
assert_eq!(rows("sqlite3", &base, q), rows(g, &base, q), "rows for {q}");
95+
}
96+
}

0 commit comments

Comments
 (0)