Skip to content

Commit ac5b10f

Browse files
MagicalTuxclaude
andcommitted
feat(planner): seek a parenthesized column like the bare column
`(a) = 2`, `(b) > 10`, `(a) IN (1,2)`, and `(b) IS NULL` now seek their index exactly as the unparenthesized forms do, instead of falling back to a full scan. SQLite ignores the redundant parentheses; graphite failed to see the column through the `Paren` wrapper. The fix unwraps `Expr::Paren` in the shared `col_index`, so every seek path (equality, range, IN, IS NULL) and the `eqp_access` label pick it up together — EXPLAIN QUERY PLAN and the executor stay in lockstep, and rows are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 8f007ee commit ac5b10f

3 files changed

Lines changed: 94 additions & 0 deletions

File tree

ROADMAP.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,10 @@ byte-exact vs the pinned `sqlite3` 3.50.4 oracle. Capability summary:
473473
SCANned. The fix is one arm in the shared `collect_eq_constraints` (a NULL operand
474474
is excluded, staying the `col IS NULL` NULL-key seek), so the executor seek and the
475475
`eqp_access` label move in lockstep; `col IS NOT <const>` (a `!=`) stays a scan.
476+
A *parenthesized* column — `(a) = 2`, `(b) > 10`, `(a) IN (1,2)`, `(b) IS NULL`
477+
now seeks exactly as the bare column does (SQLite ignores the redundant parens);
478+
the fix unwraps `Expr::Paren` in the shared `col_index`, so every seek path and
479+
`eqp_access` pick it up together.
476480
The `NOT INDEXED` planner hint is now honored in the *plan* (the executor already
477481
honored it, so rows were always correct): SQLite forbids every *secondary* index on
478482
that table, so a `WHERE` seek, covering scan, ORDER-BY index walk, and MULTI-INDEX OR

src/exec/mod.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30380,6 +30380,12 @@ fn collect_range_constraints(
3038030380

3038130381
/// The column index a bare/qualified column expression resolves to, if any.
3038230382
fn col_index(e: &Expr, columns: &[ColumnInfo]) -> Option<usize> {
30383+
// A parenthesized column (`(a) = 2`) is the same column for seek purposes, so
30384+
// unwrap any `Paren` wrappers first — SQLite seeks it exactly as the bare form.
30385+
let mut e = e;
30386+
while let Expr::Paren(inner) = e {
30387+
e = inner;
30388+
}
3038330389
if let Expr::Column { table, column, .. } = e {
3038430390
columns.iter().position(|c| {
3038530391
c.name.eq_ignore_ascii_case(column)

tests/seek_parenthesized_column.rs

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
//! A parenthesized column in a `WHERE` constraint — `(a) = 2`, `(b) > 10`,
2+
//! `(a) IN (1,2)`, `(b) IS NULL` — seeks its index exactly as the bare column does.
3+
//! SQLite ignores the redundant parentheses; graphite previously failed to recognize
4+
//! the column through the `Paren` wrapper and fell back to a full scan, so
5+
//! `EXPLAIN QUERY PLAN` diverged (`SCAN` vs `SEARCH`). The fix lives in the shared
6+
//! `col_index`, so the executor seek and the EQP label move in lockstep. Verified vs
7+
//! the sqlite3 3.50.4 CLI.
8+
9+
#![cfg(feature = "std")]
10+
11+
use std::process::Command;
12+
13+
fn sqlite3_available() -> bool {
14+
Command::new("sqlite3").arg("--version").output().is_ok()
15+
}
16+
17+
fn plan(bin: &str, base: &str, sql: &str) -> String {
18+
let full = format!("{base} EXPLAIN QUERY PLAN {sql}");
19+
let out = Command::new(bin)
20+
.arg(":memory:")
21+
.arg(&full)
22+
.output()
23+
.unwrap();
24+
String::from_utf8_lossy(&out.stdout)
25+
.lines()
26+
.filter(|l| !l.trim().is_empty() && !l.starts_with("QUERY PLAN"))
27+
.map(|l| l.trim_start_matches(|c: char| " |`*+_-".contains(c)))
28+
.collect::<Vec<_>>()
29+
.join("#")
30+
}
31+
32+
fn rows(bin: &str, base: &str, sql: &str) -> String {
33+
let full = format!("{base} {sql}");
34+
let out = Command::new(bin)
35+
.arg(":memory:")
36+
.arg(&full)
37+
.output()
38+
.unwrap();
39+
String::from_utf8_lossy(&out.stdout).trim_end().to_string()
40+
}
41+
42+
const SCHEMA: &str = "CREATE TABLE t(a INTEGER PRIMARY KEY, b, c); CREATE INDEX tb ON t(b);";
43+
44+
#[test]
45+
fn parenthesized_column_seeks_like_bare() {
46+
if !sqlite3_available() {
47+
eprintln!("sqlite3 CLI not found; skipping");
48+
return;
49+
}
50+
let g = env!("CARGO_BIN_EXE_graphitesql");
51+
for q in [
52+
"SELECT * FROM t WHERE (a)=2", // INTEGER PRIMARY KEY
53+
"SELECT * FROM t WHERE ((a))=2", // nested parens
54+
"SELECT * FROM t WHERE 2=(a)", // flipped operands
55+
"SELECT * FROM t WHERE (b)=10", // secondary index equality
56+
"SELECT * FROM t WHERE (b)>10", // secondary index range
57+
"SELECT * FROM t WHERE (a) IN (1,2)",
58+
"SELECT * FROM t WHERE (b) IS NULL",
59+
] {
60+
assert_eq!(
61+
plan("sqlite3", SCHEMA, q),
62+
plan(g, SCHEMA, q),
63+
"plan for {q}"
64+
);
65+
}
66+
}
67+
68+
#[test]
69+
fn parenthesized_column_rows_match() {
70+
if !sqlite3_available() {
71+
eprintln!("sqlite3 CLI not found; skipping");
72+
return;
73+
}
74+
let g = env!("CARGO_BIN_EXE_graphitesql");
75+
let base = format!("{SCHEMA} INSERT INTO t VALUES(1,10,5),(2,20,5),(3,NULL,9),(4,40,1);");
76+
for q in [
77+
"SELECT a FROM t WHERE (a)=2",
78+
"SELECT a FROM t WHERE (b)>10 ORDER BY a",
79+
"SELECT a FROM t WHERE (a) IN (1,3) ORDER BY a",
80+
"SELECT a FROM t WHERE (b) IS NULL",
81+
] {
82+
assert_eq!(rows("sqlite3", &base, q), rows(g, &base, q), "rows for {q}");
83+
}
84+
}

0 commit comments

Comments
 (0)