Skip to content

Commit 5d77db1

Browse files
committed
test(document): split index tests into sub-modules and add partial/fanout coverage
Split sql_index_naming.rs into tests/sql_index/ (naming, planner, backfill, partial, helpers) to respect the 500-line hard limit and group tests by concern. The file is now a thin entry point that includes each sub-module via #[path = ...] so they compile in one test crate. New tests cover partial-index predicate filtering (rows outside the WHERE scope are not indexed, not UNIQUE-checked), the conjunct-aware planner rewrite (equality nested in AND still picks up the index), and cluster backfill fan-out (every node must execute BackfillIndex, not merely replicate Raft state). Harness gains document_index_backfill_count() on NodeHandle (reads the new SystemMetrics atomic) and wires system_metrics into the CoreLoop in lifecycle.rs so per-node counters are observable from tests.
1 parent 03de4e9 commit 5d77db1

9 files changed

Lines changed: 802 additions & 321 deletions

File tree

nodedb/tests/common/cluster_harness/node/inspect.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,23 @@ impl TestClusterNode {
3737
.unwrap_or(0)
3838
}
3939

40+
/// Count of `DocumentOp::BackfillIndex` handler invocations on
41+
/// this node's Data Plane since startup. A CREATE INDEX against a
42+
/// cluster must fan out backfill to every node — this counter
43+
/// exposes whether the local core actually executed the primitive
44+
/// (a positive value) versus merely replicating Raft state from
45+
/// the coordinator (counter stays 0).
46+
pub fn document_index_backfill_count(&self) -> u64 {
47+
self.shared
48+
.system_metrics
49+
.as_ref()
50+
.map(|m| {
51+
m.document_index_backfills
52+
.load(std::sync::atomic::Ordering::Relaxed)
53+
})
54+
.unwrap_or(0)
55+
}
56+
4057
/// Number of active collections visible on this node (read through
4158
/// the local `SystemCatalog` redb — populated by the
4259
/// `MetadataCommitApplier` on every node via

nodedb/tests/common/cluster_harness/node/lifecycle.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,12 +153,16 @@ impl TestClusterNode {
153153
let data_side = data_sides.into_iter().next().expect("data side");
154154
let event_producer = event_producers.into_iter().next().expect("event producer");
155155
let core_dir = data_dir_path.clone();
156+
let core_metrics = shared.system_metrics.clone();
156157
let (core_stop_tx, core_stop_rx) = std::sync::mpsc::channel::<()>();
157158
let core_handle = tokio::task::spawn_blocking(move || {
158159
let mut core =
159160
CoreLoop::open(0, data_side.request_rx, data_side.response_tx, &core_dir)
160161
.expect("core open");
161162
core.set_event_producer(event_producer);
163+
if let Some(m) = core_metrics {
164+
core.set_metrics(m);
165+
}
162166
// Continue ticking only while the channel is Empty.
163167
// `Ok(())` means we got an explicit stop signal;
164168
// `Disconnected` means the sender was dropped (e.g. the

nodedb/tests/sql_ddl_cluster.rs

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,67 @@ async fn ddl_create_drop_trigger_replicates() {
221221
cluster.shutdown().await;
222222
}
223223

224+
// ── Index backfill fan-out ───────────────────────────────────────
225+
226+
#[tokio::test(flavor = "multi_thread", worker_threads = 6)]
227+
async fn create_index_backfill_runs_on_every_node() {
228+
let cluster = TestCluster::spawn_three().await.expect("cluster");
229+
cluster
230+
.exec_ddl_on_any_leader("CREATE COLLECTION idx_fanout_coll")
231+
.await
232+
.expect("create coll");
233+
wait_for(
234+
"collection visible on all nodes",
235+
Duration::from_secs(10),
236+
Duration::from_millis(50),
237+
|| {
238+
cluster
239+
.nodes
240+
.iter()
241+
.all(|n| n.cached_collection_count() >= 1)
242+
},
243+
)
244+
.await;
245+
246+
cluster
247+
.exec_ddl_on_any_leader("INSERT INTO idx_fanout_coll (id, email) VALUES ('u1', 'a@b.com')")
248+
.await
249+
.expect("insert row");
250+
251+
cluster
252+
.exec_ddl_on_any_leader("CREATE INDEX idx_fanout_email ON idx_fanout_coll(email)")
253+
.await
254+
.expect("create index");
255+
256+
// Every node's Data Plane must have executed BackfillIndex at
257+
// least once — a distributed CREATE INDEX that runs the backfill
258+
// only on the coordinator silently under-populates the index on
259+
// rows hosted by other vShards. The counter distinguishes local
260+
// handler invocation from Raft-replicated catalog state. Poll
261+
// briefly to let any async applier post-effects settle before
262+
// asserting final counts.
263+
let deadline = std::time::Instant::now() + Duration::from_secs(5);
264+
while std::time::Instant::now() < deadline
265+
&& !cluster
266+
.nodes
267+
.iter()
268+
.all(|n| n.document_index_backfill_count() >= 1)
269+
{
270+
tokio::time::sleep(Duration::from_millis(50)).await;
271+
}
272+
let counts: Vec<u64> = cluster
273+
.nodes
274+
.iter()
275+
.map(|n| n.document_index_backfill_count())
276+
.collect();
277+
assert!(
278+
counts.iter().all(|&c| c >= 1),
279+
"backfill must run on every node; got {:?}",
280+
counts
281+
);
282+
cluster.shutdown().await;
283+
}
284+
224285
// ── Schedule ─────────────────────────────────────────────────────
225286

226287
#[tokio::test(flavor = "multi_thread", worker_threads = 6)]

nodedb/tests/sql_index/backfill.rs

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
//! Two-phase Building→Ready backfill and UNIQUE enforcement.
2+
3+
use super::common::pgwire_harness::TestServer;
4+
5+
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
6+
async fn create_unique_index_rejects_duplicate_insert() {
7+
let server = TestServer::start().await;
8+
9+
server
10+
.exec("CREATE COLLECTION idx_unique_enforce")
11+
.await
12+
.unwrap();
13+
server
14+
.exec("CREATE UNIQUE INDEX ON idx_unique_enforce(email)")
15+
.await
16+
.unwrap();
17+
18+
// First insert must succeed.
19+
server
20+
.exec("INSERT INTO idx_unique_enforce { id: 'a', email: 'x@y.z' }")
21+
.await
22+
.unwrap();
23+
24+
// Second insert with the same indexed value must be rejected. Today the
25+
// UNIQUE keyword is parsed (`is_unique`) but never persisted anywhere,
26+
// so duplicates succeed silently — a correctness bug that is part of
27+
// the same design flaw as the reporter's point-lookup issue: CREATE
28+
// INDEX DDL modifiers are parsed but not dispatched to the config or
29+
// enforcement layer.
30+
server
31+
.expect_error(
32+
"INSERT INTO idx_unique_enforce { id: 'b', email: 'x@y.z' }",
33+
"unique",
34+
)
35+
.await;
36+
}
37+
38+
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
39+
async fn create_index_backfills_existing_rows() {
40+
let server = TestServer::start().await;
41+
42+
server.exec("CREATE COLLECTION bf_col").await.unwrap();
43+
server
44+
.exec("INSERT INTO bf_col { id: 'a', email: 'one@x.com' }")
45+
.await
46+
.unwrap();
47+
server
48+
.exec("INSERT INTO bf_col { id: 'b', email: 'two@x.com' }")
49+
.await
50+
.unwrap();
51+
52+
// CREATE INDEX runs AFTER the rows exist. The two-phase
53+
// Building→Ready backfill pipeline must populate the index from the
54+
// pre-existing documents before flipping to Ready; otherwise a
55+
// subsequent lookup against the index would miss the rows (same
56+
// silent-miss class as the original reporter's bug).
57+
server.exec("CREATE INDEX ON bf_col(email)").await.unwrap();
58+
59+
let rows = server
60+
.query_text("SELECT id FROM bf_col WHERE email = 'one@x.com'")
61+
.await
62+
.expect("indexed SELECT must succeed");
63+
assert_eq!(
64+
rows.len(),
65+
1,
66+
"indexed SELECT must return exactly one row, got: {rows:?}"
67+
);
68+
assert!(
69+
rows[0].contains("\"a\""),
70+
"indexed SELECT row must reference doc id 'a', got: {}",
71+
rows[0]
72+
);
73+
}
74+
75+
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
76+
async fn create_unique_index_rejects_existing_duplicates() {
77+
let server = TestServer::start().await;
78+
79+
server.exec("CREATE COLLECTION bf_unique").await.unwrap();
80+
server
81+
.exec("INSERT INTO bf_unique { id: 'a', code: 'ABC' }")
82+
.await
83+
.unwrap();
84+
server
85+
.exec("INSERT INTO bf_unique { id: 'b', code: 'ABC' }")
86+
.await
87+
.unwrap();
88+
89+
// CREATE UNIQUE INDEX on a collection that already contains
90+
// duplicates must fail at the backfill phase — detecting the
91+
// violation before the Ready flip so the catalog never advertises
92+
// an index that doesn't actually enforce uniqueness.
93+
server
94+
.expect_error("CREATE UNIQUE INDEX ON bf_unique(code)", "unique")
95+
.await;
96+
}

nodedb/tests/sql_index/helpers.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
//! Shared helpers for the `sql_index_naming` test binary.
2+
3+
#![allow(dead_code)] // Not every sub-file uses every helper.
4+
5+
use super::common::pgwire_harness::TestServer;
6+
7+
/// Return all rows of EXPLAIN <sql> concatenated, lowercased.
8+
///
9+
/// Used to assert that the chosen physical plan references an index lookup
10+
/// rather than a full scan when a WHERE predicate lands on an indexed field.
11+
pub async fn explain_lower(server: &TestServer, sql: &str) -> String {
12+
let rows = server
13+
.query_text(&format!("EXPLAIN {sql}"))
14+
.await
15+
.expect("EXPLAIN must succeed");
16+
rows.join("\n").to_lowercase()
17+
}
18+
19+
/// Return all rows of EXPLAIN <sql> concatenated, preserving case.
20+
///
21+
/// The pgwire EXPLAIN handler emits the `PhysicalPlan` via `{:?}` debug
22+
/// format, so structural fields like `filters: [..]`, `projection: [..]`,
23+
/// `limit: N`, etc. appear verbatim. Plan-shape assertions use this
24+
/// form; `explain_lower` is only for case-insensitive plan-variant
25+
/// name checks.
26+
pub async fn explain_raw(server: &TestServer, sql: &str) -> String {
27+
let rows = server
28+
.query_text(&format!("EXPLAIN {sql}"))
29+
.await
30+
.expect("EXPLAIN must succeed");
31+
rows.join("\n")
32+
}

nodedb/tests/sql_index/naming.rs

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
//! CREATE INDEX naming, SHOW INDEXES, and parse-time shape tests.
2+
//!
3+
//! The "silent no-op" regression mode (DDL parses, audit records
4+
//! ownership, but the secondary index is never wired into the
5+
//! collection config) is guarded here at the registration surface.
6+
//! Deeper behavioral checks (planner, backfill, partial) live in
7+
//! sibling files.
8+
9+
use super::common::pgwire_harness::TestServer;
10+
11+
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
12+
async fn create_index_named() {
13+
let server = TestServer::start().await;
14+
15+
server.exec("CREATE COLLECTION idx_named").await.unwrap();
16+
server
17+
.exec("INSERT INTO idx_named { id: 'a', role: 'admin' }")
18+
.await
19+
.unwrap();
20+
21+
// Named index — standard SQL form.
22+
server
23+
.exec("CREATE INDEX my_idx ON idx_named(role)")
24+
.await
25+
.unwrap();
26+
}
27+
28+
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
29+
async fn create_index_unnamed_auto_name() {
30+
let server = TestServer::start().await;
31+
32+
server.exec("CREATE COLLECTION idx_unnamed").await.unwrap();
33+
server
34+
.exec("INSERT INTO idx_unnamed { id: 'a', email: 'a@b.com' }")
35+
.await
36+
.unwrap();
37+
38+
// No name — should auto-generate name and succeed.
39+
server
40+
.exec("CREATE INDEX ON idx_unnamed(email)")
41+
.await
42+
.unwrap();
43+
}
44+
45+
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
46+
async fn create_index_fields_keyword() {
47+
let server = TestServer::start().await;
48+
49+
server.exec("CREATE COLLECTION idx_fields").await.unwrap();
50+
server
51+
.exec("INSERT INTO idx_fields { id: 'a', tag: 'rust' }")
52+
.await
53+
.unwrap();
54+
55+
// FIELDS keyword form — should succeed.
56+
server
57+
.exec("CREATE INDEX ON idx_fields FIELDS tag")
58+
.await
59+
.unwrap();
60+
}
61+
62+
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
63+
async fn create_unique_index_unnamed() {
64+
let server = TestServer::start().await;
65+
66+
server.exec("CREATE COLLECTION idx_unique").await.unwrap();
67+
server
68+
.exec("INSERT INTO idx_unique { id: 'a', code: 'ABC' }")
69+
.await
70+
.unwrap();
71+
72+
// Unnamed UNIQUE index.
73+
server
74+
.exec("CREATE UNIQUE INDEX ON idx_unique(code)")
75+
.await
76+
.unwrap();
77+
}
78+
79+
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
80+
async fn show_indexes_lists_created_index() {
81+
let server = TestServer::start().await;
82+
83+
server.exec("CREATE COLLECTION idx_show").await.unwrap();
84+
server
85+
.exec("CREATE INDEX idx_show_role ON idx_show(role)")
86+
.await
87+
.unwrap();
88+
89+
// Positive lock-in: SHOW INDEXES must list a freshly created index. This
90+
// is the user-visible confirmation that creation succeeded; without it,
91+
// operators have no feedback channel. (First-column read of `query_text`
92+
// returns the index name.)
93+
let names = server
94+
.query_text("SHOW INDEXES")
95+
.await
96+
.expect("SHOW INDEXES must succeed");
97+
assert!(
98+
names.iter().any(|n| n == "idx_show_role"),
99+
"SHOW INDEXES must list created index, got: {names:?}"
100+
);
101+
}

0 commit comments

Comments
 (0)