diff --git a/Cargo.lock b/Cargo.lock index 53ab4b8cd..0d9526127 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2149,7 +2149,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -4430,11 +4430,14 @@ dependencies = [ "nodedb-raft", "nodedb-test-support", "nodedb-types", + "prost", "quinn", "rdkafka", "redb", + "reqwest", "rustls", "serde_json", + "snap", "sonic-rs", "tempfile", "tokio", @@ -5702,9 +5705,9 @@ dependencies = [ [[package]] name = "quinn-proto" -version = "0.11.17" +version = "0.11.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "04759210543be93709136e28212294a659ef5001836ff4eab4d663e4529bba83" +checksum = "0e750cca55fe4f0439a15d0bb529da9651e79993e8e72c61a899a36d462befbe" dependencies = [ "aws-lc-rs", "bytes", @@ -5736,7 +5739,7 @@ dependencies = [ "once_cell", "socket2", "tracing", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -6281,7 +6284,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -6349,7 +6352,7 @@ dependencies = [ "security-framework", "security-framework-sys", "webpki-root-certs", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -6390,9 +6393,9 @@ dependencies = [ [[package]] name = "ruzstd" -version = "0.9.0" +version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a252f5e20f038fe7b4ea53e073e65398d652c864cc162fc77c56c2f13717b888" +checksum = "69e4ce4a039f42bf25920b95667e4d789e98749ab11bbb8d2c2936f788421bb5" dependencies = [ "twox-hash", ] @@ -7047,7 +7050,7 @@ dependencies = [ "getrandom 0.3.4", "once_cell", "rustix", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -8155,7 +8158,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] diff --git a/deny.toml b/deny.toml index 5140f5989..0aae31003 100644 --- a/deny.toml +++ b/deny.toml @@ -44,6 +44,12 @@ ignore = [ "RUSTSEC-2026-0247", # bitmaps; review-by: 2027-05-01 "RUSTSEC-2026-0248", # im; review-by: 2027-05-01 "RUSTSEC-2026-0251", # sized-chunks; review-by: 2027-05-01 + # lindera reads its tokenizer config through serde_yaml_ng, which pulls + # unsafe-libyaml. The crate was archived upstream with no fixed version + # and no known security impact. lindera parses YAML only from the config + # file the operator names in LINDERA_CONFIG_PATH, never client input. + # Revisit on each lindera upgrade. + "RUSTSEC-2024-0449", # unsafe-libyaml; review-by: 2027-05-01 ] [licenses] diff --git a/docs/ai/evaluation-tracking.md b/docs/ai/evaluation-tracking.md index 154e978e2..e251ecd31 100644 --- a/docs/ai/evaluation-tracking.md +++ b/docs/ai/evaluation-tracking.md @@ -96,25 +96,20 @@ CREATE COLLECTION model_perf ( ) WITH (engine='timeseries', partition_by='1h', retention='90d'); -- Continuous aggregation: 5-minute rollups for dashboards -CREATE CONTINUOUS AGGREGATE model_perf_5m -ON model_perf -AS - SELECT time_bucket('5 minutes', ts) AS bucket, - model_name, - endpoint, - AVG(latency_ms) AS avg_latency, - AVG(score) AS avg_score, - SUM(CASE WHEN error THEN 1 ELSE 0 END) AS error_count, - COUNT(*) AS request_count - FROM model_perf - GROUP BY bucket, model_name, endpoint; +CREATE CONTINUOUS AGGREGATE model_perf_5m ON model_perf + BUCKET '5m' + AGGREGATE avg(latency_ms) AS avg_latency, + avg(score) AS avg_score, + max(latency_ms) AS max_latency, + count(*) AS request_count + GROUP BY model_name, endpoint; -- Dashboard query: latency trend for the last 24 hours -SELECT ts, model_name, avg_latency, avg_score, error_count, request_count +SELECT bucket, model_name, avg_latency, avg_score, max_latency, request_count FROM model_perf_5m WHERE model_name = 'retriever-v3' - AND ts > NOW() - INTERVAL '24 hours' -ORDER BY ts; + AND bucket > NOW() - INTERVAL '24 hours' +ORDER BY bucket; -- Drift detection: compare this week's average to last week's SELECT diff --git a/docs/query-language.md b/docs/query-language.md index 711f09d87..570c5db60 100644 --- a/docs/query-language.md +++ b/docs/query-language.md @@ -766,18 +766,14 @@ SHOW MATERIALIZED VIEWS; ### Continuous Aggregates -Continuous aggregates are incrementally maintained views over timeseries collections. Unlike `REFRESH MATERIALIZED VIEW` (full re-scan), continuous aggregates update only the new watermark window. +Continuous aggregates are incrementally maintained views over timeseries collections. Unlike `REFRESH MATERIALIZED VIEW` (full re-scan), continuous aggregates fold only new data into per-bucket state. ```sql -CREATE CONTINUOUS AGGREGATE cpu_hourly -ON cpu_metrics -AS - SELECT time_bucket('1 hour', ts) AS hour, - host, - AVG(cpu) AS avg_cpu - FROM cpu_metrics - GROUP BY hour, host -WITH (refresh_interval = '1m'); +CREATE CONTINUOUS AGGREGATE cpu_hourly ON cpu_metrics + BUCKET '1h' + AGGREGATE avg(cpu) AS avg_cpu, count(*) AS samples + GROUP BY host + WITH (refresh_interval = '1m', retention = '365d'); -- Manually trigger a refresh REFRESH CONTINUOUS AGGREGATE cpu_hourly; @@ -787,6 +783,23 @@ SHOW CONTINUOUS AGGREGATES; DROP CONTINUOUS AGGREGATE cpu_hourly; ``` +`BUCKET` sets the bucket width. `AGGREGATE` functions: `count`, `sum`, `avg`, `min`, `max`, `first`, `last`, `count_distinct`, `approx_count_distinct`, `percentile(col, q)` (also `ts_percentile`, `approx_percentile`), `topk(col, k)` (also `approx_topk`). `GROUP BY` is optional. + +`WITH` options: + +| Option | Value | +| ------ | ----- | +| `refresh_policy` | `on_flush` (default), `on_seal`, `manual`, or an interval such as `'5m'` (folds periodically) | +| `refresh_interval` | Interval such as `'1m'`. Folds periodically on the maintenance tick. | +| `retention` | Interval after which the aggregate drops old buckets | + +- `on_flush` and `on_seal` aggregates fold each flushed partition once. +- An interval `refresh_policy` or `refresh_interval` folds on the maintenance tick at that interval. +- `manual` aggregates fold only on `REFRESH CONTINUOUS AGGREGATE`. A read serves the last refresh. +- Aggregate state survives restart and keeps its history after raw retention removes the source rows. +- `SELECT * FROM cpu_hourly` returns one row per bucket and group: `bucket`, the `GROUP BY` columns, then the outputs. +- The aggregate is read-only. + ## Engine-Specific SQL ### Vector Search @@ -1257,7 +1270,7 @@ SHOW USERS; `NOW()`, `CURRENT_TIMESTAMP()`, `EXTRACT(field FROM ts)`, `DATE_TRUNC(unit, ts)`, `DATE_FORMAT(ts, fmt)`, `time_bucket(interval, ts)` -`time_bucket(interval, ts)` — truncates `ts` to the nearest `interval` boundary. Registered as a DataFusion ScalarUDF. Accepts interval literals (`'5m'`, `'1h'`, `'1d'`) and ISO 8601 durations (`'PT5M'`, `'PT1H'`). Used in timeseries aggregations and continuous aggregate definitions. +`time_bucket(interval, ts)` — truncates `ts` to the nearest `interval` boundary. Registered as a DataFusion ScalarUDF. Accepts interval literals (`'5m'`, `'1h'`, `'1d'`) and ISO 8601 durations (`'PT5M'`, `'PT1H'`). Used in timeseries aggregations. Continuous aggregate definitions do not use it. They set the bucket with `BUCKET`. ### Type diff --git a/docs/timeseries.md b/docs/timeseries.md index 7b5f2386c..8bc8ab046 100644 --- a/docs/timeseries.md +++ b/docs/timeseries.md @@ -14,7 +14,7 @@ Timeseries is a **columnar profile**. It extends the [Columnar engine](columnar. - **Cascading compression** — ALP + FastLanes + FSST + Pcodec + rANS + LZ4. Achieves 20-40x compression on typical telemetry, compared to 3-5x from single-pass approaches. - **ILP ingest** — InfluxDB Line Protocol for high-throughput metric ingestion with adaptive batching and per-series core routing. -- **Continuous aggregation** — Incrementally maintained materialized views with watermarks, multi-tier chaining, and out-of-order handling. No full re-scan on refresh. +- **Continuous aggregation** — Incrementally maintained per-bucket state, folded from flushed partitions. No full re-scan on refresh. - **Retention policies** — Automatic expiry of old data by time partition. - **Approximate aggregation** — HyperLogLog (cardinality), t-digest (percentiles), SpaceSaving (top-K), Count-Min Sketch (frequency). All mergeable across shards and usable in continuous aggregation. - **Block-level skip** — Sparse primary index with min/max statistics per block. Queries skip irrelevant time ranges without decompression. @@ -53,11 +53,10 @@ GROUP BY bucket, host ORDER BY bucket DESC; -- Continuous aggregation (incremental, no full re-scan) -CREATE CONTINUOUS AGGREGATE cpu_hourly -ON cpu_metrics -BUCKET '1 hour' -AGGREGATE AVG(cpu_usage) AS avg_cpu, ts_percentile(cpu_usage, 0.99) AS p99_cpu -GROUP BY time_bucket('1 hour', ts) AS hour, host +CREATE CONTINUOUS AGGREGATE cpu_hourly ON cpu_metrics +BUCKET '1h' +AGGREGATE avg(cpu_usage) AS avg_cpu, ts_percentile(cpu_usage, 0.99) AS p99_cpu +GROUP BY host WITH (refresh_interval = '1m'); -- Approximate aggregation @@ -107,16 +106,15 @@ GROUP BY bucket; ## Continuous Aggregation DDL -Continuous aggregates are incrementally maintained views over a timeseries collection. They refresh automatically as new data arrives — no full re-scan on each refresh. +A continuous aggregate folds new rows of a timeseries collection into per-bucket state. It never re-scans the source. ```sql -- Create a continuous aggregate -CREATE CONTINUOUS AGGREGATE cpu_hourly -ON cpu_metrics -BUCKET '1 hour' -AGGREGATE AVG(cpu_usage) AS avg_cpu, ts_percentile(cpu_usage, 0.99) AS p99_cpu -GROUP BY time_bucket('1 hour', ts) AS hour, host -WITH (refresh_interval = '1m'); +CREATE CONTINUOUS AGGREGATE cpu_hourly ON cpu_metrics +BUCKET '1h' +AGGREGATE avg(cpu_usage) AS avg_cpu, ts_percentile(cpu_usage, 0.99) AS p99_cpu +GROUP BY host +WITH (refresh_interval = '1m', retention = '365d'); -- Manually trigger a refresh REFRESH CONTINUOUS AGGREGATE cpu_hourly; @@ -128,7 +126,25 @@ SHOW CONTINUOUS AGGREGATES; DROP CONTINUOUS AGGREGATE cpu_hourly; ``` -The `refresh_interval` controls how often the engine checks for new data since the last watermark. Out-of-order data within the watermark window is handled automatically. +`BUCKET` sets the bucket width. Do not use `time_bucket(...)` in the definition. + +| Clause | Meaning | +| ------ | ------- | +| `AGGREGATE f(col) [AS alias], ...` | Outputs. Functions: `count`, `sum`, `avg`, `min`, `max`, `first`, `last`, `count_distinct`, `approx_count_distinct`, `percentile(col, q)` (also `ts_percentile`, `approx_percentile`), `topk(col, k)` (also `approx_topk`). `q` is in `[0, 1]`. `k` is at least 1. Use `count(*)` to count rows. | +| `GROUP BY col, ...` | Optional. Adds group columns to each bucket. | +| `refresh_policy` | `on_flush` (default), `on_seal`, `manual`, or an interval such as `'5m'` (folds periodically). | +| `refresh_interval` | Interval such as `'1m'`. Folds periodically on the maintenance tick. | +| `retention` | Interval after which the aggregate drops old buckets. | + +Refresh behavior: + +- `on_flush` and `on_seal` aggregates fold each flushed partition once. +- `refresh_interval` makes the aggregate periodic. It folds on the maintenance tick at that interval. +- `manual` aggregates fold only on `REFRESH CONTINUOUS AGGREGATE`. A read serves the last refresh. + +Aggregate state survives restart. It keeps its history after raw retention removes the source rows. + +`SELECT * FROM cpu_hourly` returns one row per bucket and group. Columns are `bucket`, the `GROUP BY` columns, then the outputs. The aggregate is read-only. Continuous aggregate refreshes can also be triggered from the cron scheduler: @@ -140,7 +156,7 @@ AS BEGIN END; ``` -This is useful when you want deterministic refresh timing rather than relying on the `refresh_interval` background timer. See [Real-Time Features](real-time.md#cron-scheduler) for the full cron scheduler reference. +This is useful when you want deterministic refresh timing rather than relying on the `refresh_interval` maintenance tick or the flush-time fold. See [Real-Time Features](real-time.md#cron-scheduler) for the full cron scheduler reference. ## ILP Ingest (InfluxDB Line Protocol) diff --git a/docs/vectors.md b/docs/vectors.md index 2a1d43b69..db22dcfca 100644 --- a/docs/vectors.md +++ b/docs/vectors.md @@ -113,7 +113,7 @@ CREATE COLLECTION corpus ( tenant_id UUID, created_at TIMESTAMP DEFAULT now() ) WITH ( - primary='vector', + engine='vector', vector_field='embedding', dim=384, metric='cosine', @@ -125,7 +125,7 @@ CREATE COLLECTION corpus ( ); ``` -`primary='vector'` is purely an access-path hint — not a different engine. Cross-engine queries, CRDT sync, SQL semantics all keep working. `payload_indexes` are per-field equality / range / boolean indexes over the metadata sidecar for filtered ANN (replaces Pinecone metadata filters). +`engine='vector'` implies `primary='vector'`. It requires `vector_field` and `dim`; a missing one, or a contradicting `primary`, is refused with SQLSTATE `42601`. A document collection searched through a vector index uses `engine='document_schemaless'`. Cross-engine queries, CRDT sync, SQL semantics all keep working. `payload_indexes` are per-field equality / range / boolean indexes over the metadata sidecar for filtered ANN (replaces Pinecone metadata filters). `storage_dtype` controls the precision of raw HNSW vector storage (independent of quantization). Accepted values: `F32` (default, 4 bytes/dimension), `F16` (2 bytes/dimension, IEEE half), `BF16` (2 bytes/dimension, bfloat16 — same exponent range as F32, better dynamic range for embeddings, preferred). Using `BF16` or `F16` halves resident vector bytes, reducing memory pressure on large indexes with a small accuracy cost. The HNSW graph structure and search quality remain unaffected. diff --git a/nodedb-client-tests/tests/document_declared_key_round_trip.rs b/nodedb-client-tests/tests/document_declared_key_round_trip.rs index 87c575274..075c7307e 100644 --- a/nodedb-client-tests/tests/document_declared_key_round_trip.rs +++ b/nodedb-client-tests/tests/document_declared_key_round_trip.rs @@ -191,7 +191,7 @@ async fn vector_search_restricts_allowed_ids_on_a_declared_key() { &remote, "CREATE COLLECTION sku_vecs \ FIELDS (sku TEXT PRIMARY KEY, embedding VECTOR(2)) \ - WITH (engine='vector', m=8, ef_construction=50)", + WITH (engine='document_schemaless', m=8, ef_construction=50)", ) .await; for (sku, x) in [ diff --git a/nodedb-client-tests/tests/document_typeguard_round_trip.rs b/nodedb-client-tests/tests/document_typeguard_round_trip.rs new file mode 100644 index 000000000..3d772b87b --- /dev/null +++ b/nodedb-client-tests/tests/document_typeguard_round_trip.rs @@ -0,0 +1,91 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! A native document write is held to the collection's typeguards and +//! generated columns, as a SQL write is. +//! +//! A native put reaches the Data Plane without the SQL planner. The typeguard +//! hook on the Data Plane judges its body all the same, and a body that +//! supplies a generated column is refused before dispatch. + +use nodedb_client::native::pool::PoolConfig; +use nodedb_client::{Document, NativeClient, NodeDb, Value}; +use nodedb_test_support::pgwire_harness::TestServer; + +fn native(server: &TestServer) -> NativeClient { + NativeClient::new(PoolConfig::new( + format!("127.0.0.1:{}", server.native_port), + nodedb_types::protocol::AuthMethod::Trust { + username: "nodedb".into(), + }, + )) +} + +fn document(id: &str, fields: &[(&str, &str)]) -> Document { + let mut doc = Document::new(id); + for (name, value) in fields { + doc.set(*name, Value::String((*value).into())); + } + doc +} + +#[tokio::test] +async fn native_put_enforces_typeguards() { + let server = TestServer::start().await; + server.exec("CREATE COLLECTION tg_native").await.unwrap(); + server + .exec( + "CREATE TYPEGUARD ON tg_native (\ + email STRING REQUIRED, \ + slug STRING VALUE LOWER(title)\ + )", + ) + .await + .unwrap(); + let native = native(&server); + + native + .document_put("tg_native", document("n1", &[("title", "Hi")])) + .await + .expect_err("a put without the required email must be refused"); + + native + .document_put( + "tg_native", + document("n2", &[("title", "Native Row"), ("email", "a@b.c")]), + ) + .await + .expect("a valid put lands"); + let read = native + .document_get("tg_native", "n2") + .await + .expect("document_get") + .expect("the written document exists"); + assert_eq!( + read.get("slug"), + Some(&Value::String("native row".into())), + "the guard VALUE must inject: {read:?}" + ); +} + +#[tokio::test] +async fn native_put_supplying_a_generated_column_is_refused() { + let server = TestServer::start().await; + server + .exec( + "CREATE COLLECTION gen_native (\ + id TEXT PRIMARY KEY, price TEXT, \ + label TEXT GENERATED ALWAYS AS (LOWER(price))) \ + WITH (engine='document_strict')", + ) + .await + .unwrap(); + let native = native(&server); + + native + .document_put( + "gen_native", + document("g1", &[("price", "TEN"), ("label", "mine")]), + ) + .await + .expect_err("a put that supplies a generated column must be refused"); +} diff --git a/nodedb-client-tests/tests/vector_insert_field_round_trip.rs b/nodedb-client-tests/tests/vector_insert_field_round_trip.rs index 5bea1ce49..8d912143a 100644 --- a/nodedb-client-tests/tests/vector_insert_field_round_trip.rs +++ b/nodedb-client-tests/tests/vector_insert_field_round_trip.rs @@ -28,9 +28,8 @@ async fn vector_insert_field_must_not_silently_delegate_to_unfielded() { // `primary='vector'` is the server contract that routes per-row // INSERT through `VectorOp::DirectUpsert`, which binds the vector // under the named-field HNSW key — exactly the path - // `vector_insert_field` must target. Plain `engine='vector'` - // without `primary='vector'` is a schemaless collection that - // happens to have a VECTOR column: the data lands in the document + // `vector_insert_field` must target. A `document_schemaless` + // collection with a VECTOR column keeps the data in the document // body, not in a per-field HNSW index, so a field-aware insert is // unverifiable in that mode. remote diff --git a/nodedb-client-tests/tests/vector_search_allowed_ids_round_trip.rs b/nodedb-client-tests/tests/vector_search_allowed_ids_round_trip.rs index 42ae1b2c8..519afc734 100644 --- a/nodedb-client-tests/tests/vector_search_allowed_ids_round_trip.rs +++ b/nodedb-client-tests/tests/vector_search_allowed_ids_round_trip.rs @@ -21,7 +21,7 @@ async fn seed(remote: &NodeDbRemote) { .execute_sql( "CREATE COLLECTION allowed_ids_vecs \ FIELDS (id TEXT, embedding VECTOR(2)) \ - WITH (engine='vector', m=8, ef_construction=50)", + WITH (engine='document_schemaless', m=8, ef_construction=50)", &[], ) .await diff --git a/nodedb-client-tests/tests/vector_search_filter_round_trip.rs b/nodedb-client-tests/tests/vector_search_filter_round_trip.rs index 4de37caf0..ba210bf36 100644 --- a/nodedb-client-tests/tests/vector_search_filter_round_trip.rs +++ b/nodedb-client-tests/tests/vector_search_filter_round_trip.rs @@ -26,15 +26,16 @@ async fn vector_search_with_metadata_filter_round_trips_through_pgwire() { .await .expect("pgwire connect to harness must succeed"); - // Provision a vector collection with an explicit `category` - // metadata column so the WHERE predicate has a real column to bind - // against. Vector engine requires the engine='vector' option plus - // explicit FIELDS; the planner does not auto-promote. + // Provision a document collection with a vector column and an + // explicit `category` metadata column, so the WHERE predicate has a + // real column to bind against. Vector search over a document + // collection uses engine='document_schemaless'; engine='vector' is a + // vector-primary collection. remote .execute_sql( "CREATE COLLECTION embeddings \ FIELDS (id TEXT, embedding VECTOR(3), category TEXT) \ - WITH (engine='vector', m=8, ef_construction=50)", + WITH (engine='document_schemaless', m=8, ef_construction=50)", &[], ) .await diff --git a/nodedb-client-tests/tests/vector_search_metadata_filter_window.rs b/nodedb-client-tests/tests/vector_search_metadata_filter_window.rs index 134921ece..7c6b139d4 100644 --- a/nodedb-client-tests/tests/vector_search_metadata_filter_window.rs +++ b/nodedb-client-tests/tests/vector_search_metadata_filter_window.rs @@ -24,7 +24,7 @@ async fn seed(remote: &NodeDbRemote) { .execute_sql( "CREATE COLLECTION meta_filter_vecs \ FIELDS (id TEXT, embedding VECTOR(2), category TEXT) \ - WITH (engine='vector', m=8, ef_construction=50)", + WITH (engine='document_schemaless', m=8, ef_construction=50)", &[], ) .await diff --git a/nodedb-cluster-tests/Cargo.toml b/nodedb-cluster-tests/Cargo.toml index bf5bc827f..2ddb736c0 100644 --- a/nodedb-cluster-tests/Cargo.toml +++ b/nodedb-cluster-tests/Cargo.toml @@ -42,11 +42,14 @@ futures = { workspace = true } hex = { workspace = true } loro = { workspace = true } nexar = { workspace = true } +prost = { workspace = true } quinn = { workspace = true } rdkafka = { workspace = true } redb = { workspace = true } +reqwest = { workspace = true } rustls = { workspace = true } serde_json = { workspace = true } +snap = { workspace = true } sonic-rs = { workspace = true } tracing = { workspace = true } tracing-subscriber = { workspace = true } diff --git a/nodedb-cluster-tests/tests/common_suite/cases/calvin_backup_cut_order.rs b/nodedb-cluster-tests/tests/common_suite/cases/calvin_backup_cut_order.rs index 78831172e..a9472c3c8 100644 --- a/nodedb-cluster-tests/tests/common_suite/cases/calvin_backup_cut_order.rs +++ b/nodedb-cluster-tests/tests/common_suite/cases/calvin_backup_cut_order.rs @@ -60,6 +60,17 @@ const MAX_WRITES: u32 = 40; /// The backup object, under the shared backup root. const OBJECT: &str = "calvin_cut/default.ndbb"; +/// How long one awaited step after the gate's release gets. It stays under +/// the test runner's kill timeout, so a hang names its step. +const AWAIT_DEADLINE: Duration = Duration::from_secs(60); + +/// Await `fut`, or panic naming `step` once [`AWAIT_DEADLINE`] passes. +async fn within(step: &str, fut: impl std::future::Future) -> T { + tokio::time::timeout(AWAIT_DEADLINE, fut) + .await + .unwrap_or_else(|_| panic!("{step} did not finish within {AWAIT_DEADLINE:?}")) +} + /// Every row `sql` returns through `client`, each column as text. async fn rows(client: &tokio_postgres::Client, sql: &str) -> Vec> { client @@ -281,27 +292,41 @@ async fn a_backup_during_calvin_commits_restores_whole_transactions() { ); std::fs::write(&release, b"release").expect("release the held redo"); - held.await + within("the held transaction after the gate's release", held) + .await .expect("held transaction task") .unwrap_or_else(|e| panic!("the held transaction: {e}")); - backup + within("BACKUP DATABASE after the gate's release", backup) .await .expect("backup task") .unwrap_or_else(|e| panic!("BACKUP DATABASE: {e}")); stop.store(true, Ordering::Relaxed); - for writer in writers { - let written = writer.await.expect("writer task"); + for (idx, writer) in writers.into_iter().enumerate() { + let written = within(&format!("writer {idx} after the stop"), writer) + .await + .expect("writer task"); assert!(written > 0, "every writer committed while the backup ran"); } drop(gate); assert!(root.join(OBJECT).exists(), "the backup wrote its object"); - fx.cluster.shutdown().await; + within("the source cluster's shutdown", fx.cluster.shutdown()).await; - let target = TestCluster::spawn_three_with_backup_root(root.clone()) - .await - .expect("target cluster"); - exec(&target, &format!("RESTORE DATABASE default FROM '{uri}'")).await; + let target = within( + "the target cluster's start", + TestCluster::spawn_three_with_backup_root(root.clone()), + ) + .await + .expect("target cluster"); + within( + "RESTORE DATABASE", + exec(&target, &format!("RESTORE DATABASE default FROM '{uri}'")), + ) + .await; target.wait_for_full_apply_convergence(STEP_DEADLINE).await; - assert_whole_transactions(&target, &accts, &entries).await; - target.shutdown().await; + within( + "reading the restored collections", + assert_whole_transactions(&target, &accts, &entries), + ) + .await; + within("the target cluster's shutdown", target.shutdown()).await; } diff --git a/nodedb-cluster-tests/tests/common_suite/cases/cluster_execute_request.rs b/nodedb-cluster-tests/tests/common_suite/cases/cluster_execute_request.rs index 42c04b8d1..539d0f2b4 100644 --- a/nodedb-cluster-tests/tests/common_suite/cases/cluster_execute_request.rs +++ b/nodedb-cluster-tests/tests/common_suite/cases/cluster_execute_request.rs @@ -60,6 +60,7 @@ fn make_kv_put_request( txn_id: None, vshard_id: None, read_groups: Vec::new(), + user_roles: Vec::new(), } } @@ -172,6 +173,7 @@ async fn execute_request_read_carries_watermark_lsn() { txn_id: None, vshard_id: None, read_groups: Vec::new(), + user_roles: Vec::new(), }; let resp = send_execute_request(transport, node1.listen_addr, req).await; @@ -306,6 +308,7 @@ async fn execute_request_cross_node_dispatch() { txn_id: None, vshard_id: None, read_groups: Vec::new(), + user_roles: Vec::new(), }; let resp = send_execute_request(sender_transport, target_addr, req).await; diff --git a/nodedb-cluster-tests/tests/common_suite/cases/constraint_delivery.rs b/nodedb-cluster-tests/tests/common_suite/cases/constraint_delivery.rs index cfc3f5b8b..a743b43a3 100644 --- a/nodedb-cluster-tests/tests/common_suite/cases/constraint_delivery.rs +++ b/nodedb-cluster-tests/tests/common_suite/cases/constraint_delivery.rs @@ -5,10 +5,11 @@ //! //! A collection's UNIQUE / NOT NULL constraints are derived from the catalog //! and must be installed into every data-group replica's CRDT validator — the -//! component that rejects constraint-violating peer deltas at commit. The -//! metadata leader's reconcile loop re-derives each collection's constraint set -//! and replicates it via a `ConstraintChange` entry on the collection's vshard -//! data Raft log; each replica installs it, fenced by `constraint_version`. +//! component that rejects constraint-violating peer deltas at commit. The DDL +//! that changes a collection's constraint set proposes a `ConstraintChange` +//! entry on the collection's vshard data Raft log before it returns, and the +//! metadata leader's reconcile loop re-delivers the set as a safety net. Each +//! replica installs it, fenced by `constraint_version`. //! //! Reading the catalog row on a follower would only prove the catalog //! replicated — NOT that the validator installed. These tests read the @@ -20,9 +21,9 @@ //! 2. alter — a later schema mutation grows the set on every replica. //! 3. crash recovery — killing the metadata leader leaves the set installed //! on every survivor; a new metadata leader takes over reconciliation. -//! This is what justifies a recurring reconcile loop over a one-shot -//! create-time hook: leadership can move, and the surviving cluster must -//! still converge without the original proposer alive. +//! This is what keeps the recurring reconcile loop beside the DDL-time +//! install: leadership can move, and the surviving cluster must still +//! converge without the original proposer alive. use crate::common; use common::cluster_harness::{TestCluster, TestClusterNode, wait_for_async}; diff --git a/nodedb-cluster-tests/tests/common_suite/cases/continuous_aggregate_serving.rs b/nodedb-cluster-tests/tests/common_suite/cases/continuous_aggregate_serving.rs new file mode 100644 index 000000000..61d251814 --- /dev/null +++ b/nodedb-cluster-tests/tests/common_suite/cases/continuous_aggregate_serving.rs @@ -0,0 +1,142 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! A continuous aggregate read as a relation answers the same rows on every +//! node, and after its root source's leader dies. +//! +//! The aggregate is `Manual`. `REFRESH CONTINUOUS AGGREGATE` is a replicated +//! cut on the source's data group: every replica flushes the source at the +//! cut's log position and folds the same rows. A read on any node fetches +//! the finalized rows from the replica that owns the source's vShard. So a +//! read through a node that does not lead the source, and a read after the +//! leader is killed, both return the rows of the last cut. + +use crate::common; +use common::cluster_harness::shared_steps::{db_detail, kill_node, leader_index_of}; +use common::cluster_harness::{TestCluster, TestClusterNode, wait_for_async}; + +use std::time::Duration; + +const SRC: &str = "cagg_cluster_src"; +const VIEW: &str = "cagg_cluster_view"; + +const CONVERGE: Duration = Duration::from_secs(30); +const STEP: Duration = Duration::from_millis(100); + +/// `(host, s, c)` rows of the aggregate read through `node`, or `None` while +/// the read fails. A failed read prints the server's error, so a timed-out +/// wait shows why the read never succeeded. +async fn view_rows(node: &TestClusterNode) -> Option> { + let messages = match node + .client + .simple_query(&format!("SELECT host, s, c FROM {VIEW} ORDER BY host")) + .await + { + Ok(messages) => messages, + Err(e) => { + eprintln!("read {VIEW}: {}", db_detail(&e)); + return None; + } + }; + Some( + messages + .iter() + .filter_map(|msg| match msg { + tokio_postgres::SimpleQueryMessage::Row(row) => Some(( + row.get(0).unwrap_or_default().to_string(), + row.get(1).unwrap_or_default().to_string(), + row.get(2).unwrap_or_default().to_string(), + )), + _ => None, + }) + .collect(), + ) +} + +/// Whether `rows` are the buckets of the two seeded hosts. +fn are_seeded(rows: &[(String, String, String)]) -> bool { + let num = |s: &str| s.parse::().ok(); + let seeded = rows.len() == 2 + && rows[0].0 == "a" + && num(&rows[0].1) == Some(4.0) + && rows[0].2 == "2" + && rows[1].0 == "b" + && num(&rows[1].1) == Some(30.0) + && rows[1].2 == "2"; + if !seeded { + eprintln!("read {VIEW}: unexpected rows {rows:?}"); + } + seeded +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn continuous_aggregate_reads_from_source_leader() { + let mut cluster = TestCluster::spawn_three() + .await + .expect("spawn 3-node cluster"); + cluster + .exec_ddl_on_any_leader(&format!( + "CREATE COLLECTION {SRC} (ts BIGINT TIME_KEY, host TEXT, value FLOAT) \ + WITH (engine='timeseries')" + )) + .await + .expect("create the source"); + cluster + .exec_ddl_on_any_leader(&format!( + "CREATE CONTINUOUS AGGREGATE {VIEW} ON {SRC} BUCKET '1m' \ + AGGREGATE SUM(value) AS s, COUNT(*) AS c GROUP BY host \ + WITH (refresh_policy = 'manual')" + )) + .await + .expect("create the aggregate"); + + let leader = leader_index_of(&cluster, SRC); + let reader = (leader + 1) % cluster.nodes.len(); + for (ts, host, value) in [ + (1_000, "a", 1.5), + (2_000, "a", 2.5), + (3_000, "b", 10.0), + (4_000, "b", 20.0), + ] { + cluster.nodes[reader] + .client + .simple_query(&format!( + "INSERT INTO {SRC} (ts, host, value) VALUES ({ts}, '{host}', {value})" + )) + .await + .unwrap_or_else(|e| panic!("insert row at {ts}: {}", db_detail(&e))); + } + cluster.nodes[reader] + .client + .simple_query(&format!("REFRESH CONTINUOUS AGGREGATE {VIEW}")) + .await + .unwrap_or_else(|e| panic!("refresh: {}", db_detail(&e))); + + let through_reader = &cluster.nodes[reader]; + wait_for_async( + "a node that does not lead the source reads the refreshed buckets", + CONVERGE, + STEP, + || async { + view_rows(through_reader) + .await + .is_some_and(|rows| are_seeded(&rows)) + }, + ) + .await; + + kill_node(&mut cluster, leader).await; + let survivor = &cluster.nodes[0]; + wait_for_async( + "after the source's leader dies, the new leader serves the same cut", + CONVERGE, + STEP, + || async { + view_rows(survivor) + .await + .is_some_and(|rows| are_seeded(&rows)) + }, + ) + .await; + + cluster.shutdown().await; +} diff --git a/nodedb-cluster-tests/tests/common_suite/cases/install_snapshot_crdt_constraints_cluster.rs b/nodedb-cluster-tests/tests/common_suite/cases/install_snapshot_crdt_constraints_cluster.rs index a7820ce3c..06b012ecc 100644 --- a/nodedb-cluster-tests/tests/common_suite/cases/install_snapshot_crdt_constraints_cluster.rs +++ b/nodedb-cluster-tests/tests/common_suite/cases/install_snapshot_crdt_constraints_cluster.rs @@ -7,7 +7,7 @@ //! ## What this guards //! //! `CREATE UNIQUE INDEX` bumps the collection's `constraint_version` in the -//! catalog and (once the metadata-leader reconcile loop runs) installs the +//! catalog and, before it returns, installs the //! translated `Constraint` into the hosting node's per-core CRDT validator //! for that collection. U6 made `MetaOp::CreateTenantSnapshot` (`create.rs`) //! carry the validator's installed constraint set into the snapshot's @@ -27,8 +27,9 @@ //! Instead this test drives the two U6-changed handlers directly, on one //! node, deterministically: //! -//! 1. Install a UNIQUE(email) constraint on `users` (via one on-demand -//! reconcile pass, with the background reconcile loop held off). +//! 1. Install a UNIQUE(email) constraint on `users` (via on-demand reconcile +//! passes until one delivers, with the background reconcile loop held +//! off), then wait until every node applied every committed entry. //! 2. CAPTURE a tenant snapshot (`create_tenant_snapshot` -> //! `MetaOp::CreateTenantSnapshot`) and assert its `crdt_constraints` //! field carries the installed constraint — proving `create.rs`. @@ -83,11 +84,26 @@ async fn crdt_constraint_survives_snapshot_capture_and_restore() { .await .expect("CREATE UNIQUE INDEX users_email_uniq"); - // Install the constraint deterministically: drive one reconcile pass on - // every node (only the metadata leader actually proposes; the rest - // no-op), then poll node 0's validator until UNIQUE(email) is installed. - for node in &cluster.nodes { - node.run_constraint_reconcile_once().await; + // Install the constraint deterministically: drive a reconcile pass on + // every node until one delivers. Only the metadata leader proposes; the + // rest no-op. A pass that delivers nothing leaves its proposal free to + // commit later, after the drop below, so the test waits for a delivery. + let reconcile_deadline = Instant::now() + Duration::from_secs(15); + loop { + let mut delivered = 0usize; + for node in &cluster.nodes { + delivered += node.run_constraint_reconcile_once().await; + } + if delivered > 0 { + break; + } + if Instant::now() >= reconcile_deadline { + panic!( + "no node delivered the constraint set of '{COLLECTION}' through a reconcile \ + pass within 15s" + ); + } + tokio::time::sleep(Duration::from_millis(200)).await; } let deadline = Instant::now() + Duration::from_secs(15); loop { @@ -139,6 +155,16 @@ async fn crdt_constraint_survives_snapshot_capture_and_restore() { // CLEAR: drop the validator's constraint set directly so restore is the // only possible source of reinstallation. + // + // The drop is local to node 0 and rides no Raft log. Every `SetConstraints` + // at version 1 that the DDL and the reconcile pass committed must apply on + // node 0 first. One applied later passes the `>=` version fence and + // reinstalls the set. The delivery above returned once the group's leader + // applied it, so its entry is committed, and so is every entry before it. + // The drop stays at version 1, the version restore installs at. + cluster + .wait_for_full_apply_convergence(Duration::from_secs(15)) + .await; assert!( cluster.nodes[0] .crdt_drop_constraints(TenantId::new(TENANT), COLLECTION, 1) diff --git a/nodedb-cluster-tests/tests/common_suite/cases/mod.rs b/nodedb-cluster-tests/tests/common_suite/cases/mod.rs index be7dd1fbf..bfb83a2c5 100644 --- a/nodedb-cluster-tests/tests/common_suite/cases/mod.rs +++ b/nodedb-cluster-tests/tests/common_suite/cases/mod.rs @@ -76,6 +76,7 @@ mod column_stats_cross_node; mod committed_publish_failover; mod committed_publish_metadata_lag; mod constraint_delivery; +mod continuous_aggregate_serving; mod crdt_compact_cross_node; mod cross_node_pk_lookup; mod cross_node_pk_write; @@ -128,6 +129,7 @@ mod owner_read_on_non_replica; mod pgwire_gateway_migration; mod planner_local_only; mod prepared_cache_invalidation; +mod promql_remote_storage; mod proposal_committed_twice_applies_once; mod redo_stream_held_replica; mod resp_gateway_migration; diff --git a/nodedb-cluster-tests/tests/common_suite/cases/promql_remote_storage.rs b/nodedb-cluster-tests/tests/common_suite/cases/promql_remote_storage.rs new file mode 100644 index 000000000..8c66b6f06 --- /dev/null +++ b/nodedb-cluster-tests/tests/common_suite/cases/promql_remote_storage.rs @@ -0,0 +1,215 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! PromQL reads over remote-written data on a 3-node cluster. +//! +//! Remote write on one node registers each metric collection through the +//! replicated catalog and stores the samples on the metric's owning data +//! group. Remote read and `/query` on every node then return the same +//! series: discovery reads each node's replicated catalog, and every scan +//! is routed to the group that owns the metric. + +use crate::common; + +use std::sync::Arc; +use std::time::Duration; + +use nodedb::config::auth::AuthMode; +use nodedb::control::promql::remote_proto::{ + Label, LabelMatcher, MatchType, Query, ReadRequest, ReadResponse, Sample, TimeSeries, + WriteRequest, +}; +use prost::Message; + +use common::cluster_harness::{TestCluster, TestClusterNode, wait_for_async}; + +/// An HTTP listener serving `node`'s state in trust mode. +async fn start_http(node: &TestClusterNode) -> (String, tokio::task::JoinHandle<()>) { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind http listener"); + let addr = listener.local_addr().expect("http local addr"); + let shared = Arc::clone(&node.shared); + let (bus, _) = nodedb::control::shutdown::ShutdownBus::new(Arc::clone(&shared.shutdown)); + let handle = tokio::spawn(async move { + let _ = nodedb::control::server::http::server::run_with_listener( + listener, + shared, + AuthMode::Trust, + None, + bus, + ) + .await; + }); + (format!("http://{addr}/v1/obsv/api/v1"), handle) +} + +fn base_ms() -> i64 { + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("clock after epoch") + .as_millis() as i64; + (now / 1000 - 600) * 1000 +} + +fn series(metric: &str, job: &str, samples: &[(i64, f64)]) -> TimeSeries { + TimeSeries { + labels: vec![ + Label { + name: "__name__".into(), + value: metric.into(), + }, + Label { + name: "job".into(), + value: job.into(), + }, + ], + samples: samples + .iter() + .map(|&(timestamp, value)| Sample { value, timestamp }) + .collect(), + exemplars: vec![], + } +} + +async fn remote_write(base: &str, timeseries: Vec) -> (reqwest::StatusCode, String) { + let mut buf = Vec::new(); + WriteRequest { timeseries } + .encode(&mut buf) + .expect("encode write request"); + let body = snap::raw::Encoder::new() + .compress_vec(&buf) + .expect("snappy compress"); + let response = reqwest::Client::new() + .post(format!("{base}/write")) + .header("Content-Encoding", "snappy") + .body(body) + .send() + .await + .expect("POST remote write"); + let status = response.status(); + (status, response.text().await.expect("remote write body")) +} + +/// `(metric, job, values)` of every series a remote read returns, sorted. +async fn remote_read(base: &str, start_ms: i64, end_ms: i64) -> Option)>> { + let mut buf = Vec::new(); + ReadRequest { + queries: vec![Query { + start_timestamp_ms: start_ms, + end_timestamp_ms: end_ms, + matchers: vec![LabelMatcher { + match_type: MatchType::Re as i32, + name: "__name__".into(), + value: "promql_cluster_m1|promql_cluster_m2".into(), + }], + }], + } + .encode(&mut buf) + .expect("encode read request"); + let body = snap::raw::Encoder::new() + .compress_vec(&buf) + .expect("snappy compress"); + let response = reqwest::Client::new() + .post(format!("{base}/read")) + .header("Content-Encoding", "snappy") + .body(body) + .send() + .await + .ok()?; + if !response.status().is_success() { + return None; + } + let bytes = response.bytes().await.ok()?; + let raw = snap::raw::Decoder::new().decompress_vec(&bytes).ok()?; + let decoded = ReadResponse::decode(&raw[..]).ok()?; + let mut out: Vec<(String, Vec)> = decoded + .results + .first()? + .timeseries + .iter() + .map(|ts| { + let label = |name: &str| { + ts.labels + .iter() + .find(|l| l.name == name) + .map(|l| l.value.clone()) + .unwrap_or_default() + }; + ( + format!("{}:{}", label("__name__"), label("job")), + ts.samples.iter().map(|s| s.value).collect(), + ) + }) + .collect(); + out.sort_by(|a, b| a.0.cmp(&b.0)); + Some(out) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn remote_write_on_one_node_reads_back_on_every_node() { + let cluster = TestCluster::spawn_three().await.expect("3-node cluster"); + let mut endpoints = Vec::new(); + for node in &cluster.nodes { + endpoints.push(start_http(node).await); + } + tokio::time::sleep(Duration::from_millis(50)).await; + + let t0 = base_ms(); + let (status, body) = remote_write( + &endpoints[0].0, + vec![ + series("promql_cluster_m1", "api", &[(t0, 1.0), (t0 + 15_000, 2.0)]), + series("promql_cluster_m2", "web", &[(t0, 5.0)]), + ], + ) + .await; + assert_eq!( + status, + reqwest::StatusCode::NO_CONTENT, + "remote write: {body}" + ); + + let expected = vec![ + ("promql_cluster_m1:api".to_owned(), vec![1.0, 2.0]), + ("promql_cluster_m2:web".to_owned(), vec![5.0]), + ]; + for (index, (base, _)) in endpoints.iter().enumerate() { + let base = base.clone(); + let want = expected.clone(); + wait_for_async( + &format!("node {index} reads both metrics back"), + Duration::from_secs(15), + Duration::from_millis(100), + || { + let base = base.clone(); + let want = want.clone(); + async move { remote_read(&base, t0, t0 + 15_000).await == Some(want) } + }, + ) + .await; + + let response = reqwest::Client::new() + .get(format!("{base}/query")) + .query(&[ + ("query", "sum(promql_cluster_m1)".to_owned()), + ("time", format!("{}", (t0 + 15_000) as f64 / 1000.0)), + ]) + .send() + .await + .expect("GET /query"); + assert_eq!(response.status(), reqwest::StatusCode::OK); + let text = response.text().await.expect("query body"); + let json: serde_json::Value = + sonic_rs::from_str(&text).unwrap_or_else(|e| panic!("query JSON {e}: {text}")); + let value = json + .pointer("/data/result/0/value/1") + .and_then(|v| v.as_str()) + .unwrap_or_else(|| panic!("node {index}: no value in {json}")); + assert_eq!(value, "2", "node {index}: {json}"); + } + + for (_, handle) in endpoints { + handle.abort(); + } + cluster.shutdown().await; +} diff --git a/nodedb-cluster-tests/tests/common_suite/cases/shuffle_aggregate_cross_node.rs b/nodedb-cluster-tests/tests/common_suite/cases/shuffle_aggregate_cross_node.rs index df1cb124c..fa4166003 100644 --- a/nodedb-cluster-tests/tests/common_suite/cases/shuffle_aggregate_cross_node.rs +++ b/nodedb-cluster-tests/tests/common_suite/cases/shuffle_aggregate_cross_node.rs @@ -89,6 +89,7 @@ fn agg_specs() -> Vec { user_alias: Some(user_alias.to_string()), field: field.to_string(), expr: None, + param: None, }; vec![ spec("count", "*", "cnt"), @@ -105,6 +106,7 @@ fn producer_plan(rows: &[&Row]) -> Vec { let scan = PhysicalPlan::Query(QueryOp::ProviderScan { provider: None, rows: msgpack_array(rows), + sources: Vec::new(), filters: Vec::new(), projection: Vec::new(), computed_columns: Vec::new(), diff --git a/nodedb-cluster-tests/tests/common_suite/cases/shuffle_consume_cross_node.rs b/nodedb-cluster-tests/tests/common_suite/cases/shuffle_consume_cross_node.rs index 4085e4d72..63ba3058f 100644 --- a/nodedb-cluster-tests/tests/common_suite/cases/shuffle_consume_cross_node.rs +++ b/nodedb-cluster-tests/tests/common_suite/cases/shuffle_consume_cross_node.rs @@ -102,6 +102,7 @@ fn provider_scan_plan(rows: &[&Row]) -> Vec { let plan = PhysicalPlan::Query(QueryOp::ProviderScan { provider: None, rows: msgpack_array(rows), + sources: Vec::new(), filters: Vec::new(), projection: Vec::new(), computed_columns: Vec::new(), diff --git a/nodedb-cluster-tests/tests/common_suite/cases/shuffle_produce_cross_node.rs b/nodedb-cluster-tests/tests/common_suite/cases/shuffle_produce_cross_node.rs index f4351274d..317ebef42 100644 --- a/nodedb-cluster-tests/tests/common_suite/cases/shuffle_produce_cross_node.rs +++ b/nodedb-cluster-tests/tests/common_suite/cases/shuffle_produce_cross_node.rs @@ -106,6 +106,7 @@ fn provider_scan_plan(rows: &[Vec]) -> Vec { let plan = PhysicalPlan::Query(QueryOp::ProviderScan { provider: None, rows: msgpack_array(rows), + sources: Vec::new(), filters: Vec::new(), projection: Vec::new(), computed_columns: Vec::new(), diff --git a/nodedb-cluster-tests/tests/common_suite/cases/surrogate_raft.rs b/nodedb-cluster-tests/tests/common_suite/cases/surrogate_raft.rs index 6722ac738..80195b5de 100644 --- a/nodedb-cluster-tests/tests/common_suite/cases/surrogate_raft.rs +++ b/nodedb-cluster-tests/tests/common_suite/cases/surrogate_raft.rs @@ -6,7 +6,7 @@ //! - `SurrogateAlloc` (the hwm) is Raft-replicated via //! `MetadataEntry::SurrogateAlloc`. Every node persists the hwm //! to `_system.surrogate_hwm` in its local redb catalog on apply. -//! - `SurrogateBind` (individual pk↔surrogate mappings) is NOT +//! - `SurrogateBindBatch` (pk↔surrogate mapping WAL records) is NOT //! Raft-replicated; each node re-derives bindings from the //! engine-write WAL replay path. //! diff --git a/nodedb-cluster-tests/tests/common_suite/cases/sync_constraint_version_fence.rs b/nodedb-cluster-tests/tests/common_suite/cases/sync_constraint_version_fence.rs index 7ace48563..2932a6d1c 100644 --- a/nodedb-cluster-tests/tests/common_suite/cases/sync_constraint_version_fence.rs +++ b/nodedb-cluster-tests/tests/common_suite/cases/sync_constraint_version_fence.rs @@ -1,133 +1,66 @@ // SPDX-License-Identifier: BUSL-1.1 -//! A peer CRDT delta admitted against a `constraint_version` the accepting -//! replica has not installed yet is fenced — refused retryably and left -//! unimported — rather than merged ahead of the constraint that is supposed to -//! police it. Once the constraint installs, the delta re-pushed at its original -//! seq is accepted normally. +//! A DDL that changes a collection's CRDT constraint set installs it before it +//! returns, so the constraint-version fence never holds the first synced delta +//! after the DDL. //! //! ## What this guards //! -//! `CREATE UNIQUE INDEX` bumps the collection's `constraint_version` in the -//! catalog immediately, but the constraint itself is delivered to each -//! data-group's per-core CRDT validator asynchronously by the metadata-leader -//! reconcile loop (`nodedb::bootstrap::constraint_reconcile`). If a peer -//! delta whose admitted `constraint_version_required` is newer than what a -//! replica has installed were allowed to import, that replica would -//! (temporarily, until reconcile catches up) accept rows a stricter -//! constraint would have rejected — a real, if narrow, correctness gap. -//! The fence closes it: such a delta is refused and never merged into the -//! tenant document. +//! `CREATE COLLECTION` with a `PRIMARY KEY` and `CREATE UNIQUE INDEX` each bump +//! the collection's `constraint_version` in the catalog. A synced delta is +//! admitted against that version, and its data-group apply is fenced until the +//! version is installed on the applying replica. The DDL proposes the +//! matching `ConstraintChange` on the collection's data Raft log before it +//! returns. A delta proposed later on the same log therefore applies after the +//! install on every replica, wherever it entered the cluster. //! -//! The refusal is **retryable**, not terminal: the same bytes at the same seq -//! succeed the moment reconcile installs the constraint. So it must reach the -//! edge as `DeltaAck { status: Gap }` and Origin must hold the producer -//! high-water-mark, keeping the re-push admissible instead of deduplicating it -//! to `Duplicate`. A terminal `DeltaReject` here would make the edge retire the -//! write while Origin waits forever for a re-push — see -//! `sync_retryable_delta_refusal.rs` for the sibling retryable refusal. +//! The DDL runs on one node and the delta enters through the sync listener of +//! another node, right after the DDL returns. The delta must apply on its +//! first push, never answer `Gap`. //! -//! ## Why this test is deterministic, not racy +//! ## Why the install can only come from the DDL //! -//! The reconcile loop normally fires once a second, so a naive test that -//! pushes a delta right after `CREATE UNIQUE INDEX` would race the loop — -//! sometimes the constraint installs before the delta arrives, sometimes -//! after. To make the fence branch observable every run, this test sets -//! `NODEDB_CONSTRAINT_RECONCILE_INTERVAL_MS` to an hour before spawning the -//! cluster (the loop's immediate startup tick fires before the collection -//! even exists, so the next auto-tick is an hour away — effectively never, -//! for the test's lifetime) and then drives exactly one reconcile pass on -//! demand via `TestClusterNode::run_constraint_reconcile_once()` once the -//! fenced-rejection assertion has already been made. +//! The test sets `NODEDB_CONSTRAINT_RECONCILE_INTERVAL_MS` to an hour before it +//! spawns the cluster. The reconcile loop's startup tick fires before the +//! collection exists, and its next tick is an hour away. No reconcile pass +//! runs during the test, so only the DDL can have installed the version. +//! +//! The fence itself, for a delta that does outrun its install, is covered by +//! the Data Plane unit test +//! `a_delta_ahead_of_the_installed_constraint_version_is_fenced`. use crate::common; use common::cluster_harness::TestCluster; -use std::time::{Duration, Instant}; - use nodedb::control::server::sync::listener::{SyncListenerConfig, start_sync_listener}; use nodedb::control::shutdown::{ShutdownBus, ShutdownWatch}; -use nodedb_crdt::{Constraint, ConstraintKind}; use nodedb_test_support::sync_client::{DeltaOutcome, SyncTestClient}; -use nodedb_types::TenantId; use nodedb_types::sync::wire::AckStatus; const COLL: &str = "users"; const DOC: &str = "doc1"; -const PEER_ID: u64 = 7; -/// The stream seq both pushes use. An edge client assigns a seq once and -/// reuses it across re-sends, so the re-push must be admissible at the seq the -/// fenced first send already claimed. const SEQ: u64 = 1; -/// Build a Loro snapshot delta inserting one row `row_id` with a single -/// `email` field, mirroring the pattern used by the sibling CRDT sync tests. -fn row_delta(collection: &str, row_id: &str, email: &str) -> Vec { +/// A Loro delta inserting row `row_id` with `id` and `email`, and the peer +/// id that wrote it. +fn row_delta(collection: &str, row_id: &str, email: &str) -> (Vec, u64) { let doc = loro::LoroDoc::new(); - let coll = doc.get_map(collection); - let row = coll + let row = doc + .get_map(collection) .insert_container(row_id, loro::LoroMap::new()) .expect("row container"); - row.insert("email", email).expect("field"); + row.insert("id", row_id).expect("id field"); + row.insert("email", email).expect("email field"); doc.commit(); - doc.export(loro::ExportMode::Snapshot) - .expect("export loro snapshot") -} - -/// Read `crdt_state('users','doc1')` on `client`. Returns `Ok(Some(payload))` -/// when the document exists, `Ok(None)` when it is absent — the CRDT read path -/// answers a missing document with `ErrorCode::NotFound`, which pgwire renders -/// as SQLSTATE `NO_DATA` (`02000`), so absence is a deterministic, terminal -/// outcome rather than a transient error. Any OTHER error is treated as a -/// transient catch-up hiccup and retried until `timeout`. -async fn read_crdt_doc( - client: &tokio_postgres::Client, - timeout: Duration, -) -> Result, String> { - let deadline = Instant::now() + timeout; - loop { - match client - .simple_query(&format!("SELECT crdt_state('{COLL}', '{DOC}')")) - .await - { - Ok(rows) => { - for msg in rows { - if let tokio_postgres::SimpleQueryMessage::Row(r) = msg { - return Ok(Some(r.get(0).unwrap_or("").to_string())); - } - } - return Ok(Some(String::new())); - } - // A missing document is terminal, not transient: the CRDT read path - // answers `NotFound`, which the `crdt_state` scalar function surfaces - // as SQLSTATE `02000` (no_data). That is exactly the "not imported" - // state the fence assertion expects. - Err(e) - if e.as_db_error() - .is_some_and(|d| d.code() == &tokio_postgres::error::SqlState::NO_DATA) => - { - return Ok(None); - } - Err(e) => { - if Instant::now() < deadline { - tokio::time::sleep(Duration::from_millis(150)).await; - continue; - } - return Err(format!( - "crdt_state query failed: code={:?} msg={:?}", - e.code().map(|c| c.code()), - e.as_db_error().map(|d| d.message()), - )); - } - } - } + let delta = doc + .export(loro::ExportMode::Snapshot) + .expect("export loro snapshot"); + (delta, doc.peer_id()) } #[tokio::test(flavor = "multi_thread", worker_threads = 4)] -async fn peer_delta_fenced_until_constraint_installed() { +async fn a_delta_pushed_through_another_node_right_after_ddl_applies() { // Hold the background constraint-reconcile loop off for the lifetime of - // this test: its only auto-tick fires before `users` even exists, and - // the next one is an hour away, so `installed` for `users` stays at 0 - // until this test drives `run_constraint_reconcile_once()` explicitly. + // this test, so only the DDL can install the constraint version. unsafe { std::env::set_var("NODEDB_CONSTRAINT_RECONCILE_INTERVAL_MS", "3600000"); } @@ -136,9 +69,9 @@ async fn peer_delta_fenced_until_constraint_installed() { .await .expect("spawn three-node cluster"); - cluster + let ddl_node = cluster .exec_ddl_on_any_leader(&format!( - "CREATE COLLECTION {COLL} WITH (engine='document_schemaless')" + "CREATE COLLECTION {COLL} (id TEXT PRIMARY KEY, email TEXT) WITH (crdt='true')" )) .await .expect("CREATE COLLECTION users"); @@ -149,25 +82,8 @@ async fn peer_delta_fenced_until_constraint_installed() { .await .expect("CREATE UNIQUE INDEX users_email_uniq"); - // Prove the constraint has NOT been installed into the validator yet - // (auto-reconcile is held off, so this must stay empty — no waiting for - // convergence, because it will never converge on its own here). - let immediate = cluster.nodes[0] - .crdt_constraints(TenantId::new(1), COLL) - .await; - assert!( - immediate.is_empty(), - "expected no constraints installed before reconcile runs, got: {immediate:?}" - ); - tokio::time::sleep(Duration::from_millis(500)).await; - let after_wait = cluster.nodes[0] - .crdt_constraints(TenantId::new(1), COLL) - .await; - assert!( - after_wait.is_empty(), - "expected constraints to remain uninstalled with reconcile held off, got: {after_wait:?}" - ); - + // The delta enters through a node other than the one that ran CREATE. + let push_node = (ddl_node + 1) % cluster.nodes.len(); let cfg = SyncListenerConfig { listen_addr: std::net::SocketAddr::from(([127, 0, 0, 1], 0)), ..Default::default() @@ -176,111 +92,35 @@ async fn peer_delta_fenced_until_constraint_installed() { ShutdownBus::new(std::sync::Arc::new(ShutdownWatch::new())); let state = start_sync_listener( cfg, - Some(std::sync::Arc::clone(&cluster.nodes[0].shared)), + Some(std::sync::Arc::clone(&cluster.nodes[push_node].shared)), shutdown_bus, ) .await .expect("start sync listener"); - let addr = state.config.listen_addr; - - let mut client = SyncTestClient::connect(addr) + let mut client = SyncTestClient::connect(state.config.listen_addr) .await - .expect("sync handshake with node 0"); + .expect("sync handshake"); - // Delta A (seq 1): admitted against constraint_version=1 (the catalog - // already bumped it at CREATE UNIQUE INDEX), but the replica has 0 - // installed — the fence must refuse it, not merge it. - let outcome_a = client - .push_delta_at_seq(COLL, DOC, PEER_ID, 1, SEQ, row_delta(COLL, DOC, "a@b.com")) + let (delta, peer_id) = row_delta(COLL, DOC, "a@b.com"); + let outcome = client + .push_delta_at_seq(COLL, DOC, peer_id, 1, SEQ, delta) .await - .expect("push delta A"); - let ack_a = match outcome_a { + .expect("push delta"); + let ack = match outcome { DeltaOutcome::Ack(ack) => ack, DeltaOutcome::Reject(reject) => panic!( - "the version fence is a retryable refusal and must be acked as Gap, but Origin sent a \ - terminal DeltaReject (reason={:?}, compensation={:?}) — the edge rolls the write back \ - and never re-pushes, while Origin holds the high-water-mark for it", + "the first delta after the DDL must apply, got DeltaReject (reason={:?}, \ + compensation={:?})", reject.reason, reject.compensation ), }; assert_eq!( - ack_a.status, - AckStatus::Gap { expected: SEQ }, - "the fenced delta applied nothing; the ack must report the gap at the seq Origin still \ - expects, got {:?}", - ack_a.status - ); - - // The fenced delta must NOT have been imported — the document is absent. - let landed = read_crdt_doc(&cluster.nodes[0].client, Duration::from_secs(5)) - .await - .expect("crdt_state query after fenced delta"); - assert!( - landed.is_none(), - "fenced delta A must not be imported, but crdt_state returned: {landed:?}" - ); - - // Install the constraint on demand: a reconcile pass proposes only on the - // metadata leader, and leadership can move under load. Re-drive it on - // every node each round until node 0's validator holds UNIQUE(email). - let deadline = Instant::now() + Duration::from_secs(15); - let mut accepted = 0usize; - loop { - let constraints = cluster.nodes[0] - .crdt_constraints(TenantId::new(1), COLL) - .await; - if constraints - .iter() - .any(|c: &Constraint| c.kind == ConstraintKind::Unique && c.field == "email") - { - break; - } - if Instant::now() >= deadline { - panic!( - "UNIQUE(email) constraint on '{COLL}' did not install within 15s of re-driving \ - reconcile on every node ({accepted} proposals accepted); observed: \ - {constraints:?}; groups:\n{}", - cluster.group_positions_report() - ); - } - for node in &cluster.nodes { - accepted += node.run_constraint_reconcile_once().await; - } - tokio::time::sleep(Duration::from_millis(200)).await; - } - - // Re-push at the SAME seq, exactly as an edge client does: the seq is - // stored on the durable pending-delta record at first send and reused - // verbatim. required(1) <= installed(1) now and a single fresh row has no - // unique conflict, so it must apply. A `Duplicate` here would mean the - // fenced delta advanced the high-water-mark after all and the write is lost. - let outcome_b = client - .push_delta_at_seq(COLL, DOC, PEER_ID, 1, SEQ, row_delta(COLL, DOC, "a@b.com")) - .await - .expect("push delta B"); - let ack_b = match outcome_b { - DeltaOutcome::Ack(ack) => ack, - DeltaOutcome::Reject(reject) => panic!( - "delta B must apply once the constraint is installed, got DeltaReject \ - (reason={:?}, compensation={:?})", - reject.reason, reject.compensation - ), - }; - assert_eq!( - ack_b.status, + ack.status, AckStatus::Applied, - "the held high-water-mark must admit the same-seq re-push rather than deduplicate it, \ - got {:?}", - ack_b.status - ); - - // Now it must be imported — the document exists with a non-empty payload. - let landed_after = read_crdt_doc(&cluster.nodes[0].client, Duration::from_secs(10)) - .await - .expect("crdt_state query after accepted delta"); - assert!( - matches!(landed_after, Some(ref s) if !s.is_empty()), - "accepted delta B must be imported, but crdt_state on node 0 returned: {landed_after:?}" + "the DDL installed the constraint version before it returned, so the first push \ + through node {push_node} must apply, got {:?}; groups:\n{}", + ack.status, + cluster.group_positions_report() ); cluster.shutdown().await; diff --git a/nodedb-cluster-tests/tests/common_suite/cases/sync_retryable_delta_refusal.rs b/nodedb-cluster-tests/tests/common_suite/cases/sync_retryable_delta_refusal.rs index b918206c7..f02c5caf2 100644 --- a/nodedb-cluster-tests/tests/common_suite/cases/sync_retryable_delta_refusal.rs +++ b/nodedb-cluster-tests/tests/common_suite/cases/sync_retryable_delta_refusal.rs @@ -33,8 +33,8 @@ //! retryable rather than terminal. //! //! The sibling retryable refusal, a delta admitted against a constraint version -//! the accepting replica has not installed yet, is covered by -//! `sync_constraint_version_fence.rs`. +//! the accepting replica has not installed yet, is covered by the Data Plane +//! unit test `a_delta_ahead_of_the_installed_constraint_version_is_fenced`. use crate::common; use common::cluster_harness::TestCluster; diff --git a/nodedb-cluster-tests/tests/common_suite/cases/vector_index_dispatch_cross_node.rs b/nodedb-cluster-tests/tests/common_suite/cases/vector_index_dispatch_cross_node.rs index 50773c9b9..474b2d5c9 100644 --- a/nodedb-cluster-tests/tests/common_suite/cases/vector_index_dispatch_cross_node.rs +++ b/nodedb-cluster-tests/tests/common_suite/cases/vector_index_dispatch_cross_node.rs @@ -123,6 +123,7 @@ async fn execute_on_node(node: &TestClusterNode, plan: PhysicalPlan) -> ExecuteR txn_id: None, vshard_id: None, read_groups: Vec::new(), + user_roles: Vec::new(), }; match transport .send_rpc_to_addr(node.listen_addr, RaftRpc::ExecuteRequest(request)) diff --git a/nodedb-cluster/src/calvin/types/transaction.rs b/nodedb-cluster/src/calvin/types/transaction.rs index 4585a1d38..c81238a3e 100644 --- a/nodedb-cluster/src/calvin/types/transaction.rs +++ b/nodedb-cluster/src/calvin/types/transaction.rs @@ -146,6 +146,13 @@ pub struct TxClass { #[serde(default)] #[msgpack(default)] pub restore_id: u64, + /// The roles of the identity whose writes the transaction commits. Every + /// participant's redo record carries them, so every replica judges a + /// role-guarded state transition against the same set. Empty for a + /// transaction no user wrote. + #[serde(default)] + #[msgpack(default)] + pub user_roles: Vec, /// Cached participating-vshard set. Re-derived on decode; not serialized. #[serde(skip)] #[msgpack(ignore)] @@ -296,6 +303,7 @@ impl TxClass { incarnations: Vec::new(), multi_part: None, restore_id: 0, + user_roles: Vec::new(), participating_vshards: Vec::new(), } } @@ -413,6 +421,11 @@ impl TxClass { self.event_source = code; } + /// Set the roles of the identity whose writes the transaction commits. + pub fn set_user_roles(&mut self, user_roles: Vec) { + self.user_roles = user_roles; + } + /// Mark the transaction as the re-issue of RESTORE `restore_id`. pub fn set_restore_id(&mut self, restore_id: u64) { self.restore_id = restore_id; diff --git a/nodedb-cluster/src/raft_loop/loop_core.rs b/nodedb-cluster/src/raft_loop/loop_core.rs index 20d6c1ca9..fbc78605c 100644 --- a/nodedb-cluster/src/raft_loop/loop_core.rs +++ b/nodedb-cluster/src/raft_loop/loop_core.rs @@ -580,25 +580,31 @@ mod tests { rl.run(shutdown_rx).await; }); - tokio::time::sleep(Duration::from_millis(50)).await; - - assert!( - raft_loop.applier.count() >= 1, - "expected at least 1 applied entry (no-op), got {}", - raft_loop.applier.count() - ); + // Poll until the leader commits its no-op (election done). The log + // fsyncs to disk, so a loaded host can take far longer than one tick. + let deadline = tokio::time::Instant::now() + Duration::from_secs(5); + while raft_loop.applier.count() < 1 { + assert!( + tokio::time::Instant::now() < deadline, + "expected at least 1 applied entry (no-op), got {}", + raft_loop.applier.count() + ); + tokio::time::sleep(Duration::from_millis(10)).await; + } // vshard 0 maps to data group 1 (not metadata group 0). let (_gid, idx) = raft_loop.propose(0, b"hello".to_vec()).unwrap(); assert!(idx >= 2); - tokio::time::sleep(Duration::from_millis(50)).await; - - assert!( - raft_loop.applier.count() >= 2, - "expected at least 2 applied entries, got {}", - raft_loop.applier.count() - ); + let deadline = tokio::time::Instant::now() + Duration::from_secs(5); + while raft_loop.applier.count() < 2 { + assert!( + tokio::time::Instant::now() < deadline, + "expected at least 2 applied entries, got {}", + raft_loop.applier.count() + ); + tokio::time::sleep(Duration::from_millis(10)).await; + } shutdown_tx.send(true).unwrap(); run_handle.abort(); diff --git a/nodedb-cluster/src/rpc_codec/execute/codec.rs b/nodedb-cluster/src/rpc_codec/execute/codec.rs index 4ea76e09d..4066c2ac3 100644 --- a/nodedb-cluster/src/rpc_codec/execute/codec.rs +++ b/nodedb-cluster/src/rpc_codec/execute/codec.rs @@ -167,6 +167,7 @@ mod tests { txn_id: None, vshard_id: None, read_groups: Vec::new(), + user_roles: Vec::new(), }; let decoded = roundtrip_req(req.clone()); assert_eq!(decoded.plan_bytes, req.plan_bytes); @@ -193,6 +194,7 @@ mod tests { txn_id: None, vshard_id: None, read_groups: Vec::new(), + user_roles: Vec::new(), }; let decoded = roundtrip_req(req); assert!(decoded.descriptor_versions.is_empty()); @@ -210,6 +212,7 @@ mod tests { txn_id: Some(nodedb_types::id::TxnId::new(9)), vshard_id: Some(nodedb_types::id::VShardId::new(513)), read_groups: vec![7, 9], + user_roles: vec!["approver".into()], }; let decoded = roundtrip_req(req); assert_eq!( @@ -218,6 +221,7 @@ mod tests { ); assert_eq!(decoded.txn_id, Some(nodedb_types::id::TxnId::new(9))); assert_eq!(decoded.read_groups, vec![7, 9]); + assert_eq!(decoded.user_roles, vec!["approver".to_string()]); } #[test] @@ -434,6 +438,7 @@ mod tests { txn_id: None, vshard_id: None, read_groups: Vec::new(), + user_roles: Vec::new(), }; let rpc = RaftRpc::ExecuteStreamRequest(req.clone()); let encoded = diff --git a/nodedb-cluster/src/rpc_codec/execute/types.rs b/nodedb-cluster/src/rpc_codec/execute/types.rs index c52225b3f..efc0267d6 100644 --- a/nodedb-cluster/src/rpc_codec/execute/types.rs +++ b/nodedb-cluster/src/rpc_codec/execute/types.rs @@ -50,6 +50,10 @@ pub struct ExecuteRequest { /// index for each and applies through it before it reads. Empty for a /// write or a read that accepts this replica as it is. pub read_groups: Vec, + /// The roles of the identity authenticated on the originating node. A + /// write carries them to every replica, which judges a role-guarded + /// state transition against them. Empty for a read or a system write. + pub user_roles: Vec, } /// Response to an `ExecuteRequest`. diff --git a/nodedb-cluster/src/transport/client/transport.rs b/nodedb-cluster/src/transport/client/transport.rs index 0e4608cb0..092f0a976 100644 --- a/nodedb-cluster/src/transport/client/transport.rs +++ b/nodedb-cluster/src/transport/client/transport.rs @@ -757,6 +757,7 @@ pub(super) mod tests { txn_id: None, vshard_id: None, read_groups: Vec::new(), + user_roles: Vec::new(), }); let stream = client.send_rpc_stream(1, req).await.unwrap(); diff --git a/nodedb-crdt/src/state/preview.rs b/nodedb-crdt/src/state/preview.rs index 3add2be6a..df9fb4574 100644 --- a/nodedb-crdt/src/state/preview.rs +++ b/nodedb-crdt/src/state/preview.rs @@ -62,6 +62,11 @@ pub struct CrdtDeltaPreview { pub post_image: Option, /// Deterministic canonical MessagePack representation of [`post_image`](Self::post_image). pub post_image_msgpack: Vec, + /// The target row as the authoritative state holds it before the delta, + /// in the same canonical encoding as `post_image_msgpack`. Encodes `None` + /// when the row is absent. A constraint on how a row may change judges + /// this image against the post-image. + pub pre_image_msgpack: Vec, /// SHA-256 over the post-import sorted `(peer, counter)` frontier entries. pub resulting_frontier_digest: [u8; 32], /// Number of new operations contributed beyond the fork's prior oplog @@ -182,6 +187,22 @@ impl CrdtState { }); } + // `read_row` opens the collection's root map, and opening an absent + // root registers it in the authoritative state. The pre-image reads + // the row only when its collection already exists, so the preview + // stays non-mutating. + let collection_exists = match self.doc.get_value() { + LoroValue::Map(roots) => roots.contains_key(expected_collection), + _ => false, + }; + let raw_pre_image = if collection_exists { + self.read_row(expected_collection, expected_row) + } else { + None + }; + let pre_image_msgpack = + zerompk::to_msgpack_vec(&raw_pre_image.as_ref().map(CanonicalLoroValue::from)) + .map_err(|error| CrdtError::Loro(format!("preview pre-image encode: {error}")))?; let raw_post_image = fork_state.read_row(expected_collection, expected_row); let post_image = raw_post_image.as_ref().map(loro_to_value); let post_image_msgpack = @@ -198,6 +219,7 @@ impl CrdtState { write_set, post_image, post_image_msgpack, + pre_image_msgpack, resulting_frontier_digest: CrdtState::frontier_digest_from(&resulting_frontier), imported_ops, trimmed_ops: admission.trimmed_operations(), diff --git a/nodedb-physical/src/physical_plan/aggregate_functions.rs b/nodedb-physical/src/physical_plan/aggregate_functions.rs new file mode 100644 index 000000000..054d795f1 --- /dev/null +++ b/nodedb-physical/src/physical_plan/aggregate_functions.rs @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: Apache-2.0 + +//! The aggregate functions the generic aggregate accumulates. +//! +//! An [`AggregateSpec`](super::AggregateSpec) sent to the generic aggregate +//! names one of these functions, or the `grouping` pseudo-aggregate. The +//! planner refuses any other function before it builds the spec. + +/// Function names with a generic accumulator. +pub const GENERIC_AGGREGATE_FUNCTIONS: &[&str] = &[ + "count", + "sum", + "avg", + "sum_distinct", + "avg_distinct", + "min", + "max", + "count_distinct", + "stddev", + "stddev_pop", + "stddev_samp", + "variance", + "var_pop", + "var_samp", + "ts_stddev", + "approx_count_distinct", + "approx_percentile", + "approx_topk", + "array_agg", + "array_agg_distinct", + "percentile_cont", + "string_agg", + "group_concat", +]; + +/// Generic aggregates that take a constant parameter beside their input: +/// the `k` of `approx_topk`, the percentile of `approx_percentile` and +/// `percentile_cont`. Each spec carries it in +/// [`AggregateSpec::param`](super::AggregateSpec::param), and a spec +/// without it is refused. +pub const PARAMETERIZED_AGGREGATE_FUNCTIONS: &[&str] = + &["approx_topk", "approx_percentile", "percentile_cont"]; + +/// The constant parameter of a parameterized aggregate. +#[derive( + Debug, + Clone, + Copy, + PartialEq, + serde::Serialize, + serde::Deserialize, + zerompk::ToMessagePack, + zerompk::FromMessagePack, +)] +pub enum AggregateParam { + /// The fraction of `approx_percentile` / `percentile_cont`, in `[0, 1]`. + Percentile { fraction: f64 }, + /// The `k` of `approx_topk`, at least 1. + TopK { k: u32 }, +} diff --git a/nodedb-physical/src/physical_plan/collection.rs b/nodedb-physical/src/physical_plan/collection.rs index 02a6c8598..220213162 100644 --- a/nodedb-physical/src/physical_plan/collection.rs +++ b/nodedb-physical/src/physical_plan/collection.rs @@ -31,6 +31,7 @@ impl PhysicalPlan { | PhysicalPlan::Vector(VectorOp::DirectUpsert { collection, .. }) | PhysicalPlan::Vector(VectorOp::DirectInsert { collection, .. }) | PhysicalPlan::Vector(VectorOp::DirectInsertIfAbsent { collection, .. }) + | PhysicalPlan::Vector(VectorOp::DirectBatchWrite { collection, .. }) | PhysicalPlan::Vector(VectorOp::DirectDelete { collection, .. }) | PhysicalPlan::Vector(VectorOp::DirectTruncate { collection, .. }) | PhysicalPlan::Vector(VectorOp::DirectUpdate { collection, .. }) @@ -66,6 +67,7 @@ impl PhysicalPlan { | PhysicalPlan::Document(DocumentOp::BulkUpdate { collection, .. }) | PhysicalPlan::Document(DocumentOp::BulkDelete { collection, .. }) | PhysicalPlan::Document(DocumentOp::Upsert { collection, .. }) + | PhysicalPlan::Document(DocumentOp::BatchUpsert { collection, .. }) | PhysicalPlan::Document(DocumentOp::InsertSelect { target_collection: collection, .. @@ -82,6 +84,9 @@ impl PhysicalPlan { | PhysicalPlan::Timeseries(TimeseriesOp::Scan { collection, .. }) | PhysicalPlan::Timeseries(TimeseriesOp::Ingest { collection, .. }) | PhysicalPlan::Timeseries(TimeseriesOp::Truncate { collection, .. }) + | PhysicalPlan::Timeseries(TimeseriesOp::RefreshAggregate { collection, .. }) + | PhysicalPlan::Timeseries(TimeseriesOp::Partitions { collection, .. }) + | PhysicalPlan::Timeseries(TimeseriesOp::ReadAggregate { collection, .. }) | PhysicalPlan::Spatial(SpatialOp::Scan { collection, .. }) | PhysicalPlan::Document(DocumentOp::Register { collection, .. }) | PhysicalPlan::Document(DocumentOp::IndexLookup { collection, .. }) @@ -94,7 +99,10 @@ impl PhysicalPlan { PhysicalPlan::Timeseries(TimeseriesOp::ResolveIngest(inner)) => match &inner.ingest { TimeseriesOp::Scan { collection, .. } | TimeseriesOp::Ingest { collection, .. } - | TimeseriesOp::Truncate { collection, .. } => Some(collection.as_str()), + | TimeseriesOp::Truncate { collection, .. } + | TimeseriesOp::RefreshAggregate { collection, .. } + | TimeseriesOp::Partitions { collection, .. } + | TimeseriesOp::ReadAggregate { collection, .. } => Some(collection.as_str()), TimeseriesOp::ResolveIngest(_) => None, }, // Same shape on the graph side, and `EdgeDelete` itself reports diff --git a/nodedb-physical/src/physical_plan/document/mod.rs b/nodedb-physical/src/physical_plan/document/mod.rs index 39ee94ba1..bd1d0115c 100644 --- a/nodedb-physical/src/physical_plan/document/mod.rs +++ b/nodedb-physical/src/physical_plan/document/mod.rs @@ -8,6 +8,7 @@ pub mod merge_types; pub mod ollp_edge; pub mod op; pub mod resolved_mutation; +pub mod row_source; pub mod sum_target; pub mod timeseries_schema; pub mod types; @@ -21,6 +22,7 @@ pub use merge_types::{MergeActionOp, MergeClauseKind as MergeClauseKindOp, Merge pub use ollp_edge::OllpPredictedEdge; pub use op::DocumentOp; pub use resolved_mutation::{DocumentResolveOutcome, DocumentResolvedMutation}; +pub use row_source::{DocumentPageSource, QuerySource, RowSource}; pub use sum_target::{RedoSumTargets, ResolvedSumTarget, SumTargetKey, resolved_sum_surrogate}; pub use timeseries_schema::TimeseriesSchema; pub use types::{ diff --git a/nodedb-physical/src/physical_plan/document/op.rs b/nodedb-physical/src/physical_plan/document/op.rs index 6af3676cf..a37f3041b 100644 --- a/nodedb-physical/src/physical_plan/document/op.rs +++ b/nodedb-physical/src/physical_plan/document/op.rs @@ -6,6 +6,7 @@ use nodedb_types::{ use super::merge_types::MergeClauseOp; use super::ollp_edge::OllpPredictedEdge; +use super::row_source::RowSource; use super::sum_target::ResolvedSumTarget; use super::timeseries_schema::TimeseriesSchema; use super::types::{EnforcementOptions, RegisteredIndex, ReturningSpec, StorageMode, UpdateValue}; @@ -200,10 +201,19 @@ pub enum DocumentOp { }, /// Batch insert documents in a single redb transaction. + /// + /// Every row is judged as a `PointInsert` judges it: a key already + /// stored, or one the page names twice, is refused with a `unique` + /// violation (23505) and the page writes nothing. With `if_absent` a + /// stored key skips its row instead, as `ON CONFLICT DO NOTHING` does. BatchInsert { collection: QualifiedCollection, /// (document_id, value_bytes) pairs. documents: Vec<(String, Vec)>, + /// `INSERT ... ON CONFLICT DO NOTHING`: a row whose key is already + /// stored is skipped, not refused. See `PointInsert::if_absent`. + #[serde(default)] + if_absent: bool, /// Per-row surrogates, parallel to `documents`: one bound surrogate /// per row. A batch with a `Surrogate::ZERO` entry is refused before /// any row is written. @@ -360,16 +370,13 @@ pub enum DocumentOp { field: String, }, - /// INSERT ... SELECT: copy documents from source to target. + /// INSERT ... SELECT: copy the rows of a source relation into the target. + /// The Control Plane orchestrates it: it reads `source`, gives each row + /// a fresh target surrogate, and writes the rows. It never reaches the + /// Data Plane as itself. InsertSelect { target_collection: QualifiedCollection, - source_collection: QualifiedCollection, - source_filters: Vec, - source_limit: usize, - /// zerompk-encoded `Vec`: one entry per target column, - /// its `alias` the target column name and its `expr` the source-row - /// expression. Empty means copy each source row unchanged. - column_map: Vec, + source: RowSource, }, /// Upsert: insert or merge. When `on_conflict_updates` is non-empty, @@ -401,12 +408,47 @@ pub enum DocumentOp { resolved_sum_targets: Vec, }, - /// Update target rows matched by a join with a source collection: scan - /// source, build a merged doc per matching target row (source fields - /// qualified `.`), evaluate `updates`, write back. + /// Multi-row `UPSERT` / `INSERT ... ON CONFLICT DO UPDATE`: every row of + /// one statement, applied as one page in one redb transaction. + /// + /// Each row is judged as an `Upsert` judges it: an absent key stores the + /// row, a stored key stores the merge. A key the page names twice merges + /// into the body its earlier row stored. Any row's refusal leaves the + /// whole page unwritten. Inside a transaction the page expands into one + /// `Upsert` per row. + BatchUpsert { + collection: QualifiedCollection, + /// `(document_id, value_bytes)` pairs, in statement order. + documents: Vec<(String, Vec)>, + /// Per-row surrogates, parallel to `documents`. Never + /// `Surrogate::ZERO`: a page that carries one is refused. + surrogates: Vec, + /// See `Upsert::on_conflict_updates`. One clause for every row. + on_conflict_updates: Vec<(String, UpdateValue)>, + /// See `Upsert::rls_write_check`. Decided against each row's body + /// that persists. + rls_write_check: RlsWriteCheck, + /// When `Some`, return one row per row of the page — the STORED + /// post-image of each, in `documents` order. + #[serde(default)] + returning: Option, + /// See `PointDelete::rls_filters`. + #[serde(default)] + rls_filters: Vec, + /// See `PointPut::resolved_sum_targets`. One entry per distinct pair + /// across `documents`. + #[serde(default)] + resolved_sum_targets: Vec, + }, + + /// Update target rows matched by a join with a source relation: build a + /// merged doc per matching target row (source fields qualified + /// `.`), evaluate `updates`, write back. UpdateFromJoin { target_collection: QualifiedCollection, - source_collection: QualifiedCollection, + /// The source relation. The Control Plane reads it and ships its rows + /// in `source_rows`; the Data Plane never reads it. + source: RowSource, /// Qualifier used for source columns in assignment expressions. source_alias: String, /// Field in the target used for the equi-join. @@ -419,13 +461,12 @@ pub enum DocumentOp { target_filters: Vec, #[serde(default)] returning: Option, - /// Control-Plane-shipped source rows for cross-core `UPDATE ... FROM`. - /// When `Some`, the join-map is built from these pre-scanned - /// `(source_doc_id, raw_stored_bytes)` rows instead of local storage — - /// needed because source and target can live on different - /// Data-Plane cores. `None` = legacy local-read path. + /// The source rows, read by the Control Plane, each a standard + /// msgpack map. The Data Plane builds the join map from them only. + /// `None` is the unresolved shape every Control-Plane entry point + /// intercepts; the Data Plane refuses it. #[serde(default)] - source_rows: Option)>>, + source_rows: Option>>, /// See `PointDelete::rls_filters`; every returned row is a target row. #[serde(default)] rls_filters: Vec, @@ -511,7 +552,8 @@ pub enum DocumentOp { /// with no source match applying `NotMatchedBySource`. Merge { target_collection: QualifiedCollection, - source_collection: QualifiedCollection, + /// See `UpdateFromJoin::source`. + source: RowSource, /// Qualifier used for source columns in assignment expressions. source_alias: String, target_join_col: String, @@ -520,7 +562,8 @@ pub enum DocumentOp { #[serde(default)] returning: Option, /// Control-Plane-pre-assigned surrogates for NOT-MATCHED insert rows, - /// keyed by source join value. When `Some`, the handler re-verifies + /// keyed by source row id (the row's position in `source_rows`, as + /// text). When `Some`, the handler re-verifies /// the insert set against these keys (`OllpRetryRequired` on drift, /// no write) and applies every arm in one redb transaction. `None` /// is the unresolved shape every Control-Plane entry point @@ -536,7 +579,7 @@ pub enum DocumentOp { resolved_insert_identities: Vec<(String, u32)>, /// See `UpdateFromJoin::source_rows`. #[serde(default)] - source_rows: Option)>>, + source_rows: Option>>, /// See `PointDelete::rls_filters`; every returned row is a target row. #[serde(default)] rls_filters: Vec, diff --git a/nodedb-physical/src/physical_plan/document/row_source.rs b/nodedb-physical/src/physical_plan/document/row_source.rs new file mode 100644 index 000000000..682aa502f --- /dev/null +++ b/nodedb-physical/src/physical_plan/document/row_source.rs @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: Apache-2.0 + +//! The relation a Control-Plane-orchestrated `INSERT ... SELECT`, `MERGE`, or +//! `UPDATE ... FROM` reads its source rows from. + +use nodedb_types::QualifiedCollection; + +use crate::physical_plan::PhysicalPlan; + +/// Source relation of an orchestrated copy or join write. +#[derive( + Debug, + Clone, + PartialEq, + serde::Serialize, + serde::Deserialize, + zerompk::ToMessagePack, + zerompk::FromMessagePack, +)] +pub enum RowSource { + /// A plain document scan, read page by page through + /// `DocumentOp::MaterializeScan`. The B-tree key cursor is stable under + /// concurrent writes, so the read has no row-count bound. The Control + /// Plane applies the WHERE filter, the offset, the limit, and the column + /// map to each page. + DocumentPages(DocumentPageSource), + /// Any other SELECT, lowered by the normal plan converter and read once + /// through the Data Plane dispatch funnel. It has exactly SELECT + /// semantics: engine, filters, ordering, distinct, windows, joins, `AS + /// OF`, and read policies all come from the normal pipeline, under the + /// normal result-byte budget. + Query(QuerySource), +} + +/// A plain document scan source, read page by page. +#[derive( + Debug, + Clone, + PartialEq, + serde::Serialize, + serde::Deserialize, + zerompk::ToMessagePack, + zerompk::FromMessagePack, +)] +pub struct DocumentPageSource { + pub collection: QualifiedCollection, + /// Serialized `Vec`: the WHERE clause, ANDed with the + /// collection's read policy by the RLS pass. + pub filters: Vec, + /// `FOR SYSTEM_TIME AS OF` cut, in epoch milliseconds. `None` reads the + /// current version. + pub system_as_of_ms: Option, + /// Matching rows skipped before the first row is read. + pub offset: usize, + /// Matching rows read after the offset. `None` reads every matching row. + pub limit: Option, + /// zerompk `Vec`: one entry per target column, its alias + /// the target column and its expression the source-row expression. + /// Empty passes each source row through unchanged. + pub column_map: Vec, +} + +/// A source SELECT read once as a whole relation. +#[derive( + Debug, + Clone, + PartialEq, + serde::Serialize, + serde::Deserialize, + zerompk::ToMessagePack, + zerompk::FromMessagePack, +)] +pub struct QuerySource { + /// The lowered SELECT. + pub plan: Box, + /// zerompk `Vec` that renames each output column to the + /// target column it writes. Empty passes each output row through + /// unchanged. + pub column_map: Vec, +} + +impl RowSource { + /// A page source that reads every current row of `collection` unchanged. + pub fn whole_collection(collection: QualifiedCollection) -> Self { + RowSource::DocumentPages(DocumentPageSource { + collection, + filters: Vec::new(), + system_as_of_ms: None, + offset: 0, + limit: None, + column_map: Vec::new(), + }) + } + + /// The collection a page source scans. `None` for a query source, whose + /// collections are the ones its plan reads. + pub fn paged_collection(&self) -> Option<&QualifiedCollection> { + match self { + RowSource::DocumentPages(page) => Some(&page.collection), + RowSource::Query(_) => None, + } + } + + /// The plan a query source runs. `None` for a page source. + pub fn query_plan(&self) -> Option<&PhysicalPlan> { + match self { + RowSource::DocumentPages(_) => None, + RowSource::Query(query) => Some(&query.plan), + } + } + + /// A short label for logs and error messages: the paged collection, or + /// the primary collection of the query plan. + pub fn label(&self) -> &str { + match self { + RowSource::DocumentPages(page) => page.collection.as_str(), + RowSource::Query(query) => query.plan.collection().unwrap_or(""), + } + } +} diff --git a/nodedb-physical/src/physical_plan/document/timeseries_schema.rs b/nodedb-physical/src/physical_plan/document/timeseries_schema.rs index 939dfd842..2e5a43ac9 100644 --- a/nodedb-physical/src/physical_plan/document/timeseries_schema.rs +++ b/nodedb-physical/src/physical_plan/document/timeseries_schema.rs @@ -27,6 +27,10 @@ pub struct TimeseriesSchema { pub time_key: String, /// Declared `(column_name, type_str)` pairs in declaration order. pub columns: Vec<(String, String)>, + /// The declared partition interval, as `partition_by` spells it (`1h`, + /// `1d`, `1M`, `1y`, `AUTO`, `UNBOUNDED`). A flush cuts one partition per + /// interval its rows fall in. + pub partition_interval: String, } impl TimeseriesSchema { diff --git a/nodedb-physical/src/physical_plan/document/types.rs b/nodedb-physical/src/physical_plan/document/types.rs index d32109d2c..f1ead77ee 100644 --- a/nodedb-physical/src/physical_plan/document/types.rs +++ b/nodedb-physical/src/physical_plan/document/types.rs @@ -81,6 +81,11 @@ pub struct EnforcementOptions { /// On UPDATE: re-evaluate if any `depends_on` column changed. #[serde(default)] pub generated_columns: Vec, + /// The collection's typeguards. Schemaless collections only; empty + /// otherwise. Every write injects their DEFAULT / VALUE into the stored + /// body and validates REQUIRED, type and CHECK. + #[serde(default)] + pub type_guards: Vec, } impl EnforcementOptions { diff --git a/nodedb-physical/src/physical_plan/gathered_source.rs b/nodedb-physical/src/physical_plan/gathered_source.rs new file mode 100644 index 000000000..09a5b7486 --- /dev/null +++ b/nodedb-physical/src/physical_plan/gathered_source.rs @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: Apache-2.0 + +//! Where the rows of a coordinator-gathered +//! [`crate::physical_plan::QueryOp::ProviderScan`] were read from. + +/// One resource the plan behind a gathered `ProviderScan` read. +/// +/// The coordinator replaces a gathered child plan (a join input, a +/// post-processed body, an aggregate input, a set-operation branch) with a +/// `ProviderScan` of its rows. The task is authorized again after that +/// replacement. Each source keeps the requirement the child had, so that +/// check demands what the check of the child demanded. +#[derive( + Debug, + Clone, + PartialEq, + Eq, + serde::Serialize, + serde::Deserialize, + zerompk::ToMessagePack, + zerompk::FromMessagePack, +)] +pub enum GatheredSource { + /// The child read this collection. It requires Read on it. + Collection(String), + /// The child read no collection at this point, such as constant rows. + /// It requires tenant-wide Read. + Tenant, +} diff --git a/nodedb-physical/src/physical_plan/kv/batch_intent.rs b/nodedb-physical/src/physical_plan/kv/batch_intent.rs new file mode 100644 index 000000000..7e19ca9c9 --- /dev/null +++ b/nodedb-physical/src/physical_plan/kv/batch_intent.rs @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: Apache-2.0 + +//! How a KV batch put treats a key that is already stored. + +use nodedb_types::RlsWriteCheck; + +use crate::physical_plan::document::UpdateValue; + +/// The rule `KvOp::BatchPut` applies to each entry's key. +/// +/// A key the batch names twice is judged against the batch's own earlier +/// entry, as one statement's rows are judged in row order. +#[derive( + Debug, + Clone, + PartialEq, + Default, + serde::Serialize, + serde::Deserialize, + zerompk::ToMessagePack, + zerompk::FromMessagePack, +)] +pub enum KvBatchIntent { + /// Overwrite every key. Reports the keys the batch created as + /// `inserted`. RESP `MSET` and the native batch put. + #[default] + Put, + /// SQL `UPSERT`: overwrite every key. Reports every entry written as + /// `affected`, as one `KvOp::Put` per row does. + Upsert, + /// SQL `INSERT`: a stored or repeated key refuses the whole batch with + /// `unique_violation` (SQLSTATE 23505). Nothing is written then. + Insert, + /// SQL `INSERT ... ON CONFLICT DO NOTHING`: a stored or repeated key is + /// skipped. Reports the entries written as `affected`. Which rows are + /// written depends on stored state, so the batch resolves to a mutation + /// list before it is proposed or journaled. + InsertIfAbsent, + /// SQL `INSERT ... ON CONFLICT (key) DO UPDATE SET ...`: an absent key + /// stores its entry, a stored or repeated key stores the merge of + /// `updates` into the stored body. `rls_write_check` decides each body + /// that persists. Resolves to a mutation list before it is proposed or + /// journaled, as `KvOp::InsertOnConflictUpdate` does under a policy. + Merge { + updates: Vec<(String, UpdateValue)>, + rls_write_check: RlsWriteCheck, + }, +} + +impl KvBatchIntent { + /// Whether the rows this intent writes depend on stored state. Such a + /// batch resolves to a mutation list first, so a replica and a WAL replay + /// store exactly the rows the leader decided. + pub fn depends_on_stored_rows(&self) -> bool { + match self { + KvBatchIntent::InsertIfAbsent | KvBatchIntent::Merge { .. } => true, + KvBatchIntent::Put | KvBatchIntent::Upsert | KvBatchIntent::Insert => false, + } + } +} diff --git a/nodedb-physical/src/physical_plan/kv/mod.rs b/nodedb-physical/src/physical_plan/kv/mod.rs index 3898a4298..6cdec694b 100644 --- a/nodedb-physical/src/physical_plan/kv/mod.rs +++ b/nodedb-physical/src/physical_plan/kv/mod.rs @@ -2,6 +2,7 @@ //! KV engine operations dispatched to the Data Plane. +pub mod batch_intent; pub mod collection; pub mod counter_shape; pub mod op; @@ -9,6 +10,7 @@ pub mod resolved_mutation; pub mod sorted_read; pub mod transfer_amount; +pub use batch_intent::KvBatchIntent; pub use counter_shape::KvCounterShape; pub use op::KvOp; pub use resolved_mutation::{KvResolveOutcome, KvResolvedMutation}; diff --git a/nodedb-physical/src/physical_plan/kv/op.rs b/nodedb-physical/src/physical_plan/kv/op.rs index 70490f4c8..7ef3ee09c 100644 --- a/nodedb-physical/src/physical_plan/kv/op.rs +++ b/nodedb-physical/src/physical_plan/kv/op.rs @@ -4,6 +4,7 @@ use nodedb_types::{QualifiedCollection, RlsWriteCheck, Surrogate}; +use super::batch_intent::KvBatchIntent; use super::counter_shape::KvCounterShape; use super::resolved_mutation::KvResolvedMutation; use super::sorted_read::{SortedIndexRead, SortedIndexSpec}; @@ -222,13 +223,18 @@ pub enum KvOp { rls_filters: Vec, }, - /// Batch put: insert/update multiple key-value pairs atomically. + /// Batch put: write multiple key-value pairs atomically, as one request + /// and one durable write. A multi-row SQL `INSERT` / `UPSERT` lowers to + /// it, with `intent` set to the statement's rule for a stored key. BatchPut { collection: QualifiedCollection, /// `(key, value)` pairs. entries: Vec<(Vec, Vec)>, /// Per-key TTL override in milliseconds. 0 = use collection default. ttl_ms: u64, + /// What a key already stored, or repeated in `entries`, does. + #[serde(default)] + intent: KvBatchIntent, /// Stable cross-engine identity for each entry, same order and /// length as `entries`, assigned by the CP-side `SurrogateAssigner` /// from `(collection, key)` -- the same mechanism `Put`/`Insert` @@ -256,6 +262,12 @@ pub enum KvOp { /// Position of the field in the schema column list. field_position: usize, /// Whether to backfill the index with existing entries. + /// + /// `false` over a collection that already holds rows gives an index + /// that lacks those rows. No equality probes such an index: a + /// `WHERE field = x` reads the whole collection, so results stay + /// correct and only the speed differs. An index registered before + /// the first row, or with backfill, answers every equality. backfill: bool, }, diff --git a/nodedb-physical/src/physical_plan/meta.rs b/nodedb-physical/src/physical_plan/meta.rs index 75853a614..5532e3b87 100644 --- a/nodedb-physical/src/physical_plan/meta.rs +++ b/nodedb-physical/src/physical_plan/meta.rs @@ -59,10 +59,27 @@ pub enum MetaOp { /// Remove a continuous aggregate from this core's manager. UnregisterContinuousAggregate { name: String }, - /// List all continuous aggregates from this core's manager. - /// Returns JSON-serialized `Vec`. + /// List the request tenant's continuous aggregates from this core's + /// manager. Returns MessagePack-encoded `Vec`. ListContinuousAggregates, + /// Boot registered every continuous aggregate the catalog holds on this + /// core. The core drops every persisted image entry of an aggregate + /// that is not registered, and writes the images. + ContinuousAggregatesRegistered, + + /// Wait until the refresh cut `refresh_id` of the named continuous + /// aggregate is fully folded. `collection` is the aggregate's root + /// source collection: the wait runs on the core that owns its vShard, + /// where the cut applied. The core holds the request and answers it + /// from its tick, so the fold runs under the per-tick budget while the + /// core serves other requests. The request deadline bounds the wait. + AwaitContinuousAggregateRefresh { + collection: QualifiedCollection, + name: String, + refresh_id: u64, + }, + /// Convert a collection's storage mode. /// /// Scans all documents in the collection and re-encodes them for the @@ -208,9 +225,16 @@ pub enum MetaOp { /// Enforce retention on a timeseries collection: drop segments older than /// the cutoff. Called by the retention policy enforcement loop. + /// + /// `aggregates` lists every continuous aggregate the catalog roots at + /// `collection`. Retention folds an expiring partition into those + /// aggregates before it removes it, so the core refuses the pass while + /// one of them is not registered on it at that incarnation. The loop + /// retries on its next pass. EnforceTimeseriesRetention { collection: QualifiedCollection, max_age_ms: i64, + aggregates: Vec, }, /// Bitemporal audit-retention purge for the graph edge store. @@ -245,10 +269,17 @@ pub enum MetaOp { /// column version has been superseded by a later partition is removed. /// Non-bitemporal columnar collections are a no-op (collection is /// expected to be flagged bitemporal by the caller). + /// + /// `aggregates` lists every continuous aggregate the catalog roots at + /// `collection`, as in `EnforceTimeseriesRetention`. A timeseries + /// collection's purge removes partitions, so the core refuses it while + /// one of them is not registered on it at that incarnation. A plain + /// columnar collection roots no aggregate, and its list is empty. TemporalPurgeColumnar { tenant_id: u64, collection: QualifiedCollection, cutoff_system_ms: i64, + aggregates: Vec, }, /// Bitemporal audit-retention purge for CRDT (Loro-backed) collections. diff --git a/nodedb-physical/src/physical_plan/mod.rs b/nodedb-physical/src/physical_plan/mod.rs index f403546ae..d2bcbe92d 100644 --- a/nodedb-physical/src/physical_plan/mod.rs +++ b/nodedb-physical/src/physical_plan/mod.rs @@ -6,6 +6,7 @@ //! each defined in its own module. This keeps each engine's operations //! isolated. +pub mod aggregate_functions; pub mod array; pub mod calvin_reply; pub mod cluster_array; @@ -15,6 +16,7 @@ pub mod columnar; pub mod crdt; pub mod document; pub mod exchange; +pub mod gathered_source; pub mod graph; pub mod kv; pub mod meta; @@ -32,11 +34,15 @@ pub mod sort_key; pub mod spatial; pub mod streaming; pub mod text; +pub mod tiering; pub mod timeseries; pub mod truncate_target; pub mod vector; pub mod wire; +pub use aggregate_functions::{ + AggregateParam, GENERIC_AGGREGATE_FUNCTIONS, PARAMETERIZED_AGGREGATE_FUNCTIONS, +}; pub use array::{ArrayBinaryOp, ArrayOp, ArrayReducer}; pub use calvin_reply::{ CalvinInstall, CalvinInstalledTimeseriesSpec, CalvinPostImagesSpec, CalvinReplyRow, @@ -47,20 +53,21 @@ pub use cluster_event::{ClusterEventOp, MAX_REMOTE_CDC_COMMITTED_OFFSETS}; pub use columnar::{ColumnarInsertIntent, ColumnarOp}; pub use crdt::{CrdtOp, CrdtWriteVerb}; pub use document::{ - BalancedDef, DeclaredColumn, DocumentOp, DocumentResolveOutcome, DocumentResolvedMutation, - EnforcementOptions, GeneratedColumnSpec, MaterializedSumBinding, OllpPredictedEdge, - PeriodLockConfig, RedoSumTargets, RegisteredIndex, RegisteredIndexState, ResolvedSumTarget, - ReturningColumns, ReturningItem, ReturningSpec, StorageMode, SumTargetKey, TimeseriesSchema, - UpdateValue, resolved_sum_surrogate, + BalancedDef, DeclaredColumn, DocumentOp, DocumentPageSource, DocumentResolveOutcome, + DocumentResolvedMutation, EnforcementOptions, GeneratedColumnSpec, MaterializedSumBinding, + OllpPredictedEdge, PeriodLockConfig, QuerySource, RedoSumTargets, RegisteredIndex, + RegisteredIndexState, ResolvedSumTarget, ReturningColumns, ReturningItem, ReturningSpec, + RowSource, StorageMode, SumTargetKey, TimeseriesSchema, UpdateValue, resolved_sum_surrogate, }; pub use exchange::{ExchangeMode, ExchangeOp}; +pub use gathered_source::GatheredSource; pub use graph::{ AlgoEdge, AlgoStage, BatchEdge, BspSuperstepPlan, BspSuperstepResult, GraphOp, RagBindingRow, RagLegs, RagStage, RagTextHit, RagVectorHit, WccSuperstepPlan, WccSuperstepResult, }; pub use kv::{ - KvCounterShape, KvOp, KvResolveOutcome, KvResolvedMutation, SortedIndexRead, SortedIndexSpec, - TransferAmount, + KvBatchIntent, KvCounterShape, KvOp, KvResolveOutcome, KvResolvedMutation, SortedIndexRead, + SortedIndexSpec, TransferAmount, }; pub use meta::MetaOp; pub use meta_home::{HomeAnswer, HomeVersion, HomeVersionProbe}; @@ -74,9 +81,13 @@ pub use set_op::SetOpKind; pub use sort_key::SortKeySpec; pub use spatial::{SpatialOp, SpatialPredicate}; pub use text::{ScoreScanBound, ScoreScanOrder, TextOp, TextScoreSpec}; -pub use timeseries::{TimeseriesOp, TimeseriesResolve, UNBOUNDED_TIME_RANGE}; +pub use tiering::{TierRead, TierSegment, TierSource, TieringError, TieringPlan}; +pub use timeseries::{ + AggregateIncarnation, PartitionsAction, RefreshCut, TimeseriesOp, TimeseriesResolve, + UNBOUNDED_TIME_RANGE, +}; pub use vector::{ - VectorDirectWriteIntent, VectorOp, VectorResolveOutcome, VectorResolvedMutation, - VectorWriteTargets, + VectorDirectRow, VectorDirectWriteIntent, VectorOp, VectorResolveOutcome, + VectorResolvedMutation, VectorWriteTargets, }; pub use wire::{decode, encode}; diff --git a/nodedb-physical/src/physical_plan/query.rs b/nodedb-physical/src/physical_plan/query.rs index 67e023498..317c90be5 100644 --- a/nodedb-physical/src/physical_plan/query.rs +++ b/nodedb-physical/src/physical_plan/query.rs @@ -25,6 +25,11 @@ pub struct AggregateSpec { pub field: String, /// Optional expression to evaluate per-document before aggregating. pub expr: Option, + /// The constant parameter of a parameterized aggregate (see + /// [`super::PARAMETERIZED_AGGREGATE_FUNCTIONS`]); `None` for every other + /// function. + #[serde(default)] + pub param: Option, } #[derive( @@ -55,23 +60,30 @@ pub enum QueryOp { /// Coordinator-resolved data-movement wrapper. Defensive error if it reaches a core. Exchange(crate::physical_plan::ExchangeOp), - /// Pre-materialized rows. `provider: Some(name)` => filled per-request - /// (identity-scoped catalog table); `None` => `rows` is final (constant - /// SELECT or gathered broadcast child). `rows` is a canonical msgpack array - /// of map values (the `encode_binary_rows` shape). + /// Pre-materialized rows. `provider: Some(name)` => a named relation (an + /// identity-scoped catalog table or a continuous aggregate) whose rows + /// the coordinator fills per request; `None` => `rows` is final + /// (constant SELECT or a gathered child). `rows` is a canonical msgpack + /// array of map values (the `encode_binary_rows` shape). /// /// The executor applies predicate, offset, sort, distinct, projection, and /// limit in that order over the rows before emitting the response. Empty /// `filters`/`projection` and `None` limit are no-ops (all rows emitted /// unchanged through those stages). ProviderScan { - /// Catalog provider name for deferred materialization by the coordinator. - /// `Some(name)` is replaced with `None` after `materialize_providers` - /// fills `rows`. Data-Plane cores must never see `Some`. + /// Named relation the coordinator materializes. The name stays after + /// the coordinator fills `rows`, so the authorization of the filled + /// plan demands Read on it. The Data Plane reads only `rows`. provider: Option, /// Msgpack array of plain map rows. Filled by the coordinator from the - /// catalog producer or from a gathered broadcast child. + /// catalog producer or from a gathered child. rows: Vec, + /// What a gathered child read to produce `rows`. The authorization + /// of the plan that embeds these rows demands each source's + /// requirement. Empty for a named provider and for constant rows, + /// which `provider` and the tenant-wide fallback cover. + #[serde(default)] + sources: Vec, /// Serialized `Vec` (MessagePack). Empty = no predicate. #[serde(default)] filters: Vec, @@ -252,10 +264,6 @@ pub enum QueryOp { on: Vec<(String, String)>, join_type: String, limit: usize, - /// Post-join GROUP BY columns (empty = no aggregation). - post_group_by: Vec, - /// Post-join aggregates: (op, field) pairs (empty = no aggregation). - post_aggregates: Vec<(String, String)>, /// Post-join projection: column names to keep (empty = all). projection: Vec, /// MessagePack-encoded computed projection expressions. When present, @@ -379,6 +387,13 @@ pub enum QueryOp { left_rls_filters: Vec, /// Row-level-security filters for rows scanned from `right_collection`. right_rls_filters: Vec, + /// Resolved rows for the left side, set by the coordinator inside a + /// transaction so the side reads its staged writes. `None` means the + /// side is scanned locally by `left_collection`. When set, the child + /// carries the side's read policy and `left_rls_filters` is empty. + left_input: Option>, + /// Resolved rows for the right side. Same semantics as `left_input`. + right_input: Option>, }, /// Sort-merge join: both sides pre-sorted by join key. @@ -396,6 +411,11 @@ pub enum QueryOp { left_rls_filters: Vec, /// Row-level-security filters for rows scanned from `right_collection`. right_rls_filters: Vec, + /// Resolved rows for the left side. Same semantics as + /// `QueryOp::NestedLoopJoin::left_input`. + left_input: Option>, + /// Resolved rows for the right side. Same semantics as `left_input`. + right_input: Option>, }, /// Multi-facet aggregation: compute facet counts for multiple fields diff --git a/nodedb-physical/src/physical_plan/rls_write_check_accessor.rs b/nodedb-physical/src/physical_plan/rls_write_check_accessor.rs index 729b85e59..9d9bb37b7 100644 --- a/nodedb-physical/src/physical_plan/rls_write_check_accessor.rs +++ b/nodedb-physical/src/physical_plan/rls_write_check_accessor.rs @@ -13,7 +13,7 @@ use super::PhysicalPlan; use super::columnar::ColumnarOp; use super::document::DocumentOp; use super::graph::GraphOp; -use super::kv::KvOp; +use super::kv::{KvBatchIntent, KvOp}; use super::timeseries::TimeseriesOp; use super::vector::VectorOp; @@ -56,6 +56,16 @@ impl PhysicalPlan { | PhysicalPlan::Document(DocumentOp::Upsert { rls_write_check, .. }) + | PhysicalPlan::Document(DocumentOp::BatchUpsert { + rls_write_check, .. + }) + | PhysicalPlan::Kv(KvOp::BatchPut { + intent: + KvBatchIntent::Merge { + rls_write_check, .. + }, + .. + }) | PhysicalPlan::Document(DocumentOp::UpdateFromJoin { rls_write_check, .. }) @@ -113,6 +123,9 @@ impl PhysicalPlan { | PhysicalPlan::Vector(VectorOp::DirectUpsert { rls_write_check, .. }) + | PhysicalPlan::Vector(VectorOp::DirectBatchWrite { + rls_write_check, .. + }) | PhysicalPlan::Vector(VectorOp::DirectDelete { rls_write_check, .. }) diff --git a/nodedb-physical/src/physical_plan/tiering.rs b/nodedb-physical/src/physical_plan/tiering.rs new file mode 100644 index 000000000..6b17afe66 --- /dev/null +++ b/nodedb-physical/src/physical_plan/tiering.rs @@ -0,0 +1,210 @@ +// SPDX-License-Identifier: Apache-2.0 + +//! AUTO_TIER routing of one timeseries scan across retention tiers. +//! +//! A tiered retention policy keeps raw rows for `raw_retain_ms`, then +//! downsampled buckets in one continuous aggregate per tier. A scan carries +//! the policy's tier layout; the core that owns the collection splits the +//! query range into segments and reads each segment from its tier. + +/// The tier layout of an AUTO_TIER retention policy. +#[derive( + Debug, + Clone, + PartialEq, + serde::Serialize, + serde::Deserialize, + zerompk::ToMessagePack, + zerompk::FromMessagePack, +)] +pub struct TieringPlan { + /// How long raw rows are kept, in milliseconds. 0 = forever. + pub raw_retain_ms: u64, + /// Downsample tiers, finest first. + pub tiers: Vec, +} + +/// One downsample tier of a [`TieringPlan`]. +#[derive( + Debug, + Clone, + PartialEq, + serde::Serialize, + serde::Deserialize, + zerompk::ToMessagePack, + zerompk::FromMessagePack, +)] +pub struct TierRead { + /// The policy's tier number (1 = first downsample tier). + pub tier_index: u32, + /// The continuous aggregate that holds this tier's buckets. + pub aggregate: String, + /// Bucket width in milliseconds. + pub resolution_ms: i64, + /// How long this tier keeps buckets, in milliseconds. 0 = forever. + pub retain_ms: u64, +} + +/// Which source serves one segment of a tiered scan. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum TierSource { + /// Raw rows of the collection. + Raw, + /// Buckets of `TieringPlan::tiers[index]`. + Tier(usize), +} + +/// One time segment of a tiered scan: `[start, end)` read from `source`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct TierSegment { + pub source: TierSource, + pub start: i64, + pub end: i64, +} + +/// Why a tier layout cannot split a range. +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +pub enum TieringError { + #[error( + "tier {tier_index} resolution {resolution_ms}ms is not a positive multiple of the \ + finer tier's {finer_ms}ms; a coarser bucket would straddle the finer tier's edge" + )] + UnalignedResolution { + tier_index: u32, + resolution_ms: i64, + finer_ms: i64, + }, +} + +impl TieringPlan { + /// Split `query` (an inclusive `(min, max)` millisecond range) into + /// segments, newest first. + /// + /// The edge between two sources is the retention boundary rounded up to + /// the coarser tier's resolution, so every bucket a tier serves lies + /// wholly before the edge and no row is read from two sources. + pub fn segments( + &self, + query: (i64, i64), + now_ms: i64, + ) -> Result, TieringError> { + let (query_start, query_end) = query; + let mut segments = Vec::new(); + // `upper` is the exclusive end of the source being placed. + let mut upper = query_end.saturating_add(1); + let mut source = TierSource::Raw; + let mut boundary = now_ms; + let mut retain = self.raw_retain_ms; + let mut finer_ms = 1i64; + for (index, tier) in self.tiers.iter().enumerate() { + if retain == 0 { + break; + } + if tier.resolution_ms <= 0 || tier.resolution_ms % finer_ms != 0 { + return Err(TieringError::UnalignedResolution { + tier_index: tier.tier_index, + resolution_ms: tier.resolution_ms, + finer_ms, + }); + } + boundary = boundary.saturating_sub(retain as i64); + let edge = align_up(boundary, tier.resolution_ms); + push_segment(&mut segments, source, edge.max(query_start), upper); + upper = upper.min(edge); + source = TierSource::Tier(index); + retain = tier.retain_ms; + finer_ms = tier.resolution_ms; + } + push_segment(&mut segments, source, query_start, upper); + Ok(segments) + } +} + +/// Append `[start, end)` when it is non-empty. +fn push_segment(out: &mut Vec, source: TierSource, start: i64, end: i64) { + if start < end { + out.push(TierSegment { source, start, end }); + } +} + +/// `ms` rounded up to a multiple of `step` (`step > 0`). +fn align_up(ms: i64, step: i64) -> i64 { + let floor = ms.div_euclid(step).saturating_mul(step); + if floor == ms { + floor + } else { + floor.saturating_add(step) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + const DAY: i64 = 86_400_000; + + fn plan() -> TieringPlan { + TieringPlan { + raw_retain_ms: 7 * DAY as u64, + tiers: vec![ + TierRead { + tier_index: 1, + aggregate: "_policy_p_tier1".into(), + resolution_ms: 60_000, + retain_ms: 90 * DAY as u64, + }, + TierRead { + tier_index: 2, + aggregate: "_policy_p_tier2".into(), + resolution_ms: 3_600_000, + retain_ms: 0, + }, + ], + } + } + + #[test] + fn recent_range_reads_raw_only() { + let now = 100_000 * DAY; + let segments = plan().segments((now - 3_600_000, now), now).expect("split"); + assert_eq!(segments.len(), 1); + assert_eq!(segments[0].source, TierSource::Raw); + assert_eq!(segments[0].end, now + 1); + } + + #[test] + fn range_spanning_tiers_splits_on_aligned_edges() { + let now = 100_000 * DAY + 12_345; + let segments = plan().segments((now - 200 * DAY, now), now).expect("split"); + assert_eq!(segments.len(), 3); + assert_eq!(segments[0].source, TierSource::Raw); + assert_eq!(segments[1].source, TierSource::Tier(0)); + assert_eq!(segments[2].source, TierSource::Tier(1)); + // Edges are aligned to the coarser source's resolution and contiguous. + assert_eq!(segments[0].start, segments[1].end); + assert_eq!(segments[1].start, segments[2].end); + assert_eq!(segments[0].start % 60_000, 0); + assert_eq!(segments[1].start % 3_600_000, 0); + } + + #[test] + fn forever_raw_retention_never_reads_a_tier() { + let mut plan = plan(); + plan.raw_retain_ms = 0; + let now = 100_000 * DAY; + let segments = plan.segments((0, now), now).expect("split"); + assert_eq!(segments.len(), 1); + assert_eq!(segments[0].source, TierSource::Raw); + } + + #[test] + fn unaligned_resolution_is_refused() { + let mut plan = plan(); + plan.tiers[1].resolution_ms = 90_000; + let now = 100_000 * DAY; + assert!(matches!( + plan.segments((0, now), now), + Err(TieringError::UnalignedResolution { tier_index: 2, .. }) + )); + } +} diff --git a/nodedb-physical/src/physical_plan/timeseries.rs b/nodedb-physical/src/physical_plan/timeseries.rs index c7cdbedba..fceb106a9 100644 --- a/nodedb-physical/src/physical_plan/timeseries.rs +++ b/nodedb-physical/src/physical_plan/timeseries.rs @@ -62,6 +62,15 @@ pub enum TimeseriesOp { /// `[_ts_valid_from, _ts_valid_until)` contains it are returned. #[serde(default)] valid_at_ms: Option, + /// HAVING predicates (MessagePack `Vec`) over the + /// finalized group rows, applied after gap-fill and before sort and + /// limit. Empty on a raw scan. + #[serde(default)] + having: Vec, + /// AUTO_TIER layout. `Some` routes each segment of the range to the + /// raw rows or to a downsample tier's buckets. + #[serde(default)] + tiering: Option, }, /// Write a batch of samples to the columnar memtable. @@ -116,6 +125,105 @@ pub enum TimeseriesOp { #[serde(default)] restart_identity: bool, }, + + /// A refresh cut of one continuous aggregate rooted at `collection`. + /// + /// The cut replicates on the collection's data group and applies on + /// every replica in log order. At apply the replica flushes the + /// collection's memtable, so every row of every record before the cut + /// sits in a partition and no row after it does. The aggregate then + /// folds exactly the partitions present at that moment, on the + /// maintenance tick, so every replica folds the same rows. + RefreshAggregate { + collection: QualifiedCollection, + cut: RefreshCut, + }, + + /// Partition maintenance on the core that owns `collection`: + /// `SHOW PARTITIONS FOR` lists its partitions, and `REWRITE PARTITIONS + /// FOR` rewrites each sealed one. Neither changes a row, so neither is + /// logged or replicated. + Partitions { + collection: QualifiedCollection, + action: PartitionsAction, + }, + + /// The finalized rows of continuous aggregate `aggregate`, read on the + /// core that owns its root source `collection`: one map per bucket and + /// group, holding `bucket`, the GROUP BY columns and the output columns. + /// The read completes the aggregate's backfill and pending refresh cuts + /// first, so it never answers from partial buckets. It changes no row, + /// so it is neither logged nor replicated. + ReadAggregate { + collection: QualifiedCollection, + aggregate: String, + }, +} + +/// What a [`TimeseriesOp::Partitions`] does. +#[derive( + Debug, + Clone, + Copy, + PartialEq, + Eq, + serde::Serialize, + serde::Deserialize, + zerompk::ToMessagePack, + zerompk::FromMessagePack, +)] +#[msgpack(c_enum)] +pub enum PartitionsAction { + /// List every queryable partition with its state, time range, row count, + /// size and interval. + List, + /// Rewrite every sealed or merged partition with the collection's current + /// schema and codecs. A column the schema no longer holds is dropped. + Rewrite, +} + +/// One continuous aggregate by name and incarnation. +#[derive( + Debug, + Clone, + PartialEq, + Eq, + serde::Serialize, + serde::Deserialize, + zerompk::ToMessagePack, + zerompk::FromMessagePack, +)] +pub struct AggregateIncarnation { + pub name: String, + pub incarnation: nodedb_types::Hlc, +} + +/// The cut a [`TimeseriesOp::RefreshAggregate`] makes in one continuous +/// aggregate. +#[derive( + Debug, + Clone, + PartialEq, + Eq, + serde::Serialize, + serde::Deserialize, + zerompk::ToMessagePack, + zerompk::FromMessagePack, +)] +pub struct RefreshCut { + /// Aggregate name. + pub aggregate: String, + /// Incarnation of the aggregate the cut was proposed for. A replica + /// whose aggregate of that name has another incarnation never applies + /// the cut to it. + pub incarnation: nodedb_types::Hlc, + /// Names the cut on every replica. A second apply of the same id is a + /// no-op, and a waiter asks for completion by it. For a periodic cut it + /// is the period slot: `now_ms / interval_ms`. + pub refresh_id: u64, + /// The periodic catch-up proposed for slot `refresh_id`. A replica + /// applies only slots above the last one it applied. + pub periodic: bool, } /// What a [`TimeseriesOp::ResolveIngest`] resolves. diff --git a/nodedb-physical/src/physical_plan/vector/collection.rs b/nodedb-physical/src/physical_plan/vector/collection.rs index 600238f46..786553299 100644 --- a/nodedb-physical/src/physical_plan/vector/collection.rs +++ b/nodedb-physical/src/physical_plan/vector/collection.rs @@ -17,6 +17,7 @@ impl VectorOp { VectorOp::DirectUpsert { collection, .. } | VectorOp::DirectInsert { collection, .. } | VectorOp::DirectInsertIfAbsent { collection, .. } + | VectorOp::DirectBatchWrite { collection, .. } | VectorOp::DirectDelete { collection, .. } | VectorOp::DirectTruncate { collection, .. } | VectorOp::DirectUpdate { collection, .. } diff --git a/nodedb-physical/src/physical_plan/vector/expand.rs b/nodedb-physical/src/physical_plan/vector/expand.rs new file mode 100644 index 000000000..1ff98c7c3 --- /dev/null +++ b/nodedb-physical/src/physical_plan/vector/expand.rs @@ -0,0 +1,149 @@ +// SPDX-License-Identifier: Apache-2.0 + +//! The point writes a vector-primary page expands to. +//! +//! A transaction stages each row of a page on its own, so each row gets its +//! own overlay image, undo entry and redo entry. A key the page names twice +//! is in the overlay when its later row stages, so that row is refused, +//! skipped or merged as the page judges it. + +use super::op::VectorOp; +use super::write::{VectorDirectRow, VectorDirectWriteIntent}; + +impl VectorOp { + /// One `DirectInsert` / `DirectInsertIfAbsent` / `DirectUpsert` per row + /// of a `DirectBatchWrite` page, in page order, by the page's intent. + /// `None` for every other op. + pub fn direct_batch_point_writes(&self) -> Option> { + let VectorOp::DirectBatchWrite { + collection, + field, + intent, + rows, + quantization, + storage_dtype, + payload_indexes, + returning, + rls_filters, + on_conflict_updates, + rls_write_check, + } = self + else { + return None; + }; + Some( + rows.iter() + .map(|row| { + let VectorDirectRow { + surrogate, + pk_bytes, + vector, + payload, + } = row.clone(); + match intent { + VectorDirectWriteIntent::Insert => VectorOp::DirectInsert { + collection: collection.clone(), + field: field.clone(), + surrogate, + pk_bytes, + vector, + payload, + quantization: *quantization, + storage_dtype: *storage_dtype, + payload_indexes: payload_indexes.clone(), + returning: returning.clone(), + rls_filters: rls_filters.clone(), + }, + VectorDirectWriteIntent::InsertIfAbsent => VectorOp::DirectInsertIfAbsent { + collection: collection.clone(), + field: field.clone(), + surrogate, + pk_bytes, + vector, + payload, + quantization: *quantization, + storage_dtype: *storage_dtype, + payload_indexes: payload_indexes.clone(), + returning: returning.clone(), + rls_filters: rls_filters.clone(), + }, + VectorDirectWriteIntent::Upsert => VectorOp::DirectUpsert { + collection: collection.clone(), + field: field.clone(), + surrogate, + pk_bytes, + vector, + payload, + quantization: *quantization, + storage_dtype: *storage_dtype, + payload_indexes: payload_indexes.clone(), + returning: returning.clone(), + rls_filters: rls_filters.clone(), + on_conflict_updates: on_conflict_updates.clone(), + rls_write_check: rls_write_check.clone(), + }, + } + }) + .collect(), + ) + } +} + +#[cfg(test)] +mod tests { + use nodedb_types::{DatabaseId, QualifiedCollection, RlsWriteCheck, Surrogate}; + + use super::*; + + fn page(intent: VectorDirectWriteIntent) -> VectorOp { + let row = |s: u32, pk: &str| VectorDirectRow { + surrogate: Surrogate::new(s), + pk_bytes: pk.as_bytes().to_vec(), + vector: vec![0.5, 0.25], + payload: Vec::new(), + }; + VectorOp::DirectBatchWrite { + collection: QualifiedCollection::new(DatabaseId::DEFAULT, "vecs"), + field: "emb".into(), + intent, + rows: vec![row(1, "a"), row(2, "b")], + quantization: nodedb_types::VectorQuantization::None, + storage_dtype: nodedb_types::VectorStorageDtype::F32, + payload_indexes: Vec::new(), + returning: None, + rls_filters: Vec::new(), + on_conflict_updates: Vec::new(), + rls_write_check: RlsWriteCheck::NoPolicyApplies, + } + } + + #[test] + fn a_page_expands_to_one_point_write_per_row_in_order() { + let writes = page(VectorDirectWriteIntent::InsertIfAbsent) + .direct_batch_point_writes() + .expect("a page expands"); + let keys: Vec> = writes + .iter() + .map(|write| match write { + VectorOp::DirectInsertIfAbsent { pk_bytes, .. } => pk_bytes.clone(), + other => panic!("expected DirectInsertIfAbsent, got {other:?}"), + }) + .collect(); + assert_eq!(keys, vec![b"a".to_vec(), b"b".to_vec()]); + assert!(matches!( + page(VectorDirectWriteIntent::Upsert) + .direct_batch_point_writes() + .as_deref(), + Some([VectorOp::DirectUpsert { .. }, VectorOp::DirectUpsert { .. }]) + )); + } + + #[test] + fn a_point_write_does_not_expand() { + let point = page(VectorDirectWriteIntent::Insert) + .direct_batch_point_writes() + .and_then(|writes| writes.into_iter().next()) + .expect("a row"); + assert!(point.direct_batch_point_writes().is_none()); + } +} diff --git a/nodedb-physical/src/physical_plan/vector/mod.rs b/nodedb-physical/src/physical_plan/vector/mod.rs index fd851e2bc..7b9661dde 100644 --- a/nodedb-physical/src/physical_plan/vector/mod.rs +++ b/nodedb-physical/src/physical_plan/vector/mod.rs @@ -3,10 +3,11 @@ //! Vector engine operations dispatched to the Data Plane. pub mod collection; +pub mod expand; pub mod op; pub mod resolved_mutation; pub mod write; pub use op::VectorOp; pub use resolved_mutation::{VectorResolveOutcome, VectorResolvedMutation}; -pub use write::{VectorDirectWriteIntent, VectorWriteTargets}; +pub use write::{VectorDirectRow, VectorDirectWriteIntent, VectorWriteTargets}; diff --git a/nodedb-physical/src/physical_plan/vector/op.rs b/nodedb-physical/src/physical_plan/vector/op.rs index 2f619365d..abd5df3c2 100644 --- a/nodedb-physical/src/physical_plan/vector/op.rs +++ b/nodedb-physical/src/physical_plan/vector/op.rs @@ -399,6 +399,44 @@ pub enum VectorOp { rls_filters: Vec, }, + /// Multi-row vector-primary `INSERT` / `INSERT ... ON CONFLICT` / + /// `UPSERT`: one statement, one page. Every row is judged before any + /// row lands, so the page lands whole or not at all. + /// + /// Each row means what one `DirectInsert` / `DirectInsertIfAbsent` / + /// `DirectUpsert` of the same `intent` means, applied in page order. A + /// key the page names twice is judged against the body its earlier row + /// produced, and is stored once, with the final body. + /// + /// The page always resolves to a [`VectorOp::ResolvedDirectWrite`] + /// before it is logged or replicated: the record names the rows the page + /// stores, so replay never judges existence again. + DirectBatchWrite { + collection: QualifiedCollection, + /// Vector column name; keys the HNSW index. + field: String, + /// What an existing key means for every row of the page. + intent: super::write::VectorDirectWriteIntent, + /// The rows, in statement order. + rows: Vec, + quantization: nodedb_types::VectorQuantization, + storage_dtype: nodedb_types::VectorStorageDtype, + payload_indexes: Vec<(String, nodedb_types::PayloadIndexKind)>, + /// When `Some`, return one row per row the page writes, each its + /// key's stored post-image. + #[serde(default)] + returning: Option, + /// Read filters gating the rows `returning` emits. + #[serde(default)] + rls_filters: Vec, + /// See `DirectUpsert::on_conflict_updates`. Only an `Upsert` page + /// carries one; one clause for every row. + #[serde(default)] + on_conflict_updates: Vec<(String, UpdateValue)>, + /// See `DirectUpsert::rls_write_check`. + rls_write_check: nodedb_types::RlsWriteCheck, + }, + /// Vector-primary `DELETE`. Removes the HNSW node, its payload bitmap /// entries, and the payload sidecar row of every targeted surrogate. /// Reports the number of rows that existed. @@ -459,8 +497,9 @@ pub enum VectorOp { /// would apply, and the response payload it would return, without /// applying anything. /// - /// Wraps a [`VectorOp::DirectDelete`], [`VectorOp::DirectUpdate`], or - /// [`VectorOp::DirectUpsert`] verbatim, live write predicate included — + /// Wraps a [`VectorOp::DirectDelete`], [`VectorOp::DirectUpdate`], + /// [`VectorOp::DirectUpsert`], or [`VectorOp::DirectBatchWrite`] + /// verbatim, live write predicate included — /// the handler resolves the targets, reads each row's sidecar, computes /// the post-image, and decides the policy here, where the writing /// identity is still available. A follower has none, so the predicate diff --git a/nodedb-physical/src/physical_plan/vector/write.rs b/nodedb-physical/src/physical_plan/vector/write.rs index ab14e1fbc..d86899562 100644 --- a/nodedb-physical/src/physical_plan/vector/write.rs +++ b/nodedb-physical/src/physical_plan/vector/write.rs @@ -30,6 +30,29 @@ pub enum VectorDirectWriteIntent { Upsert, } +/// One row of a [`VectorOp::DirectBatchWrite`](super::VectorOp::DirectBatchWrite) +/// page. The fields mean what the same-named fields of +/// [`VectorOp::DirectUpsert`](super::VectorOp::DirectUpsert) mean. +#[derive( + Debug, + Clone, + PartialEq, + serde::Serialize, + serde::Deserialize, + zerompk::ToMessagePack, + zerompk::FromMessagePack, +)] +pub struct VectorDirectRow { + /// Global surrogate the Control Plane bound to `pk_bytes`. + pub surrogate: Surrogate, + /// UTF-8 of the declared primary-key value. + pub pk_bytes: Vec, + /// FP32 vector values. + pub vector: Vec, + /// Pre-encoded MessagePack of every non-vector column the row supplied. + pub payload: Vec, +} + /// The rows a vector-primary `DELETE` / `UPDATE` targets. #[derive( Debug, diff --git a/nodedb-physical/src/physical_plan/wire.rs b/nodedb-physical/src/physical_plan/wire.rs index e3c37969a..21385209d 100644 --- a/nodedb-physical/src/physical_plan/wire.rs +++ b/nodedb-physical/src/physical_plan/wire.rs @@ -117,8 +117,6 @@ mod tests { on: vec![("id".into(), "id".into())], join_type: "left".into(), limit: 10, - post_group_by: Vec::new(), - post_aggregates: Vec::new(), projection: vec![JoinProjection { source: "l.id".into(), output: "id".into(), diff --git a/nodedb-query/src/expr_parse/mod.rs b/nodedb-query/src/expr_parse/mod.rs index 138e946e0..a06584eab 100644 --- a/nodedb-query/src/expr_parse/mod.rs +++ b/nodedb-query/src/expr_parse/mod.rs @@ -2,7 +2,9 @@ pub mod error; pub mod parser; +mod predicate; mod tokenizer; +mod validate; pub use error::ExprParseError; pub use parser::parse_generated_expr; diff --git a/nodedb-query/src/expr_parse/parser.rs b/nodedb-query/src/expr_parse/parser.rs index a312765d5..0521dbbfb 100644 --- a/nodedb-query/src/expr_parse/parser.rs +++ b/nodedb-query/src/expr_parse/parser.rs @@ -10,6 +10,8 @@ //! - Binary operators: `+`, `-`, `*`, `/`, `%` //! - Comparison: `=`, `!=`, `<>`, `<`, `>`, `<=`, `>=` //! - Logical: `AND`, `OR`, `NOT` +//! - Predicates: `IS [NOT] NULL`, `[NOT] LIKE` / `ILIKE` (with `ESCAPE`), +//! `[NOT] IN (...)`, `[NOT] BETWEEN ... AND ...` //! - Parenthesized sub-expressions: `(a + b) * c` //! - Function calls: `ROUND(price * 1.08, 2)`, `CONCAT(a, ' ', b)` //! - COALESCE: `COALESCE(a, b, '')` @@ -19,7 +21,9 @@ //! Determinism validation: rejects `NOW()`, `RANDOM()`, `NEXTVAL()`, `UUID()`. use super::error::ExprParseError; +use super::predicate::parse_postfix_predicate; use super::tokenizer::{Token, TokenKind, tokenize}; +use super::validate::{collect_columns, validate_deterministic}; use crate::expr::{BinaryOp, SqlExpr}; use nodedb_types::Value; @@ -86,10 +90,10 @@ fn parse_and( pos: &mut usize, depth: &mut usize, ) -> Result { - let mut left = parse_comparison(tokens, pos, depth)?; + let mut left = parse_not(tokens, pos, depth)?; while peek_keyword(tokens, *pos, "AND") { *pos += 1; - let right = parse_comparison(tokens, pos, depth)?; + let right = parse_not(tokens, pos, depth)?; left = SqlExpr::BinaryOp { left: Box::new(left), op: BinaryOp::And, @@ -99,12 +103,27 @@ fn parse_and( Ok(left) } +/// `NOT` binds looser than a comparison: `NOT a = b` is `NOT (a = b)`. +fn parse_not( + tokens: &[Token], + pos: &mut usize, + depth: &mut usize, +) -> Result { + if peek_keyword(tokens, *pos, "NOT") { + *pos += 1; + let inner = parse_not(tokens, pos, depth)?; + return Ok(SqlExpr::Negate(Box::new(inner))); + } + parse_comparison(tokens, pos, depth) +} + fn parse_comparison( tokens: &[Token], pos: &mut usize, depth: &mut usize, ) -> Result { - let left = parse_additive(tokens, pos, depth)?; + let operand = parse_additive(tokens, pos, depth)?; + let left = parse_postfix_predicate(operand, tokens, pos, depth)?; if *pos < tokens.len() && tokens[*pos].kind == TokenKind::Op { let op = match tokens[*pos].text.as_str() { "=" => BinaryOp::Eq, @@ -126,7 +145,7 @@ fn parse_comparison( Ok(left) } -fn parse_additive( +pub(super) fn parse_additive( tokens: &[Token], pos: &mut usize, depth: &mut usize, @@ -315,7 +334,7 @@ fn parse_case( }) } -fn parse_arg_list( +pub(super) fn parse_arg_list( tokens: &[Token], pos: &mut usize, depth: &mut usize, @@ -340,13 +359,17 @@ fn parse_arg_list( // ── Helpers ─────────────────────────────────────────────────────────── -fn peek_keyword(tokens: &[Token], pos: usize, keyword: &str) -> bool { +pub(super) fn peek_keyword(tokens: &[Token], pos: usize, keyword: &str) -> bool { pos < tokens.len() && tokens[pos].kind == TokenKind::Ident && tokens[pos].text.eq_ignore_ascii_case(keyword) } -fn expect_keyword(tokens: &[Token], pos: &mut usize, keyword: &str) -> Result<(), ExprParseError> { +pub(super) fn expect_keyword( + tokens: &[Token], + pos: &mut usize, + keyword: &str, +) -> Result<(), ExprParseError> { if peek_keyword(tokens, *pos, keyword) { *pos += 1; Ok(()) @@ -381,122 +404,6 @@ fn expect_token( } } -// ── Validation ──────────────────────────────────────────────────────── - -const NON_DETERMINISTIC: &[&str] = &[ - "now", - "current_timestamp", - "random", - "nextval", - "uuid", - "uuid_v4", - "uuid_v7", - "gen_random_uuid", - "ulid", - "cuid2", - "nanoid", -]; - -fn validate_deterministic(expr: &SqlExpr) -> Result<(), ExprParseError> { - match expr { - SqlExpr::Function { name, args } => { - if NON_DETERMINISTIC.contains(&name.as_str()) { - return Err(ExprParseError::InvalidLiteral { - detail: format!( - "non-deterministic function '{name}()' not allowed in GENERATED ALWAYS AS" - ), - }); - } - for arg in args { - validate_deterministic(arg)?; - } - Ok(()) - } - SqlExpr::BinaryOp { left, right, .. } => { - validate_deterministic(left)?; - validate_deterministic(right) - } - SqlExpr::Negate(inner) => validate_deterministic(inner), - SqlExpr::Coalesce(args) => { - for arg in args { - validate_deterministic(arg)?; - } - Ok(()) - } - SqlExpr::Case { - operand, - when_thens, - else_expr, - } => { - if let Some(op) = operand { - validate_deterministic(op)?; - } - for (cond, then) in when_thens { - validate_deterministic(cond)?; - validate_deterministic(then)?; - } - if let Some(e) = else_expr { - validate_deterministic(e)?; - } - Ok(()) - } - SqlExpr::Cast { expr, .. } => validate_deterministic(expr), - SqlExpr::NullIf(a, b) => { - validate_deterministic(a)?; - validate_deterministic(b) - } - SqlExpr::IsNull { expr, .. } => validate_deterministic(expr), - SqlExpr::Column(_) - | SqlExpr::Literal(_) - | SqlExpr::OldColumn(_) - | SqlExpr::ExcludedColumn(_) => Ok(()), - } -} - -pub(crate) fn collect_columns(expr: &SqlExpr, deps: &mut Vec) { - match expr { - SqlExpr::Column(name) => deps.push(name.clone()), - SqlExpr::BinaryOp { left, right, .. } => { - collect_columns(left, deps); - collect_columns(right, deps); - } - SqlExpr::Negate(inner) => collect_columns(inner, deps), - SqlExpr::Function { args, .. } => { - for arg in args { - collect_columns(arg, deps); - } - } - SqlExpr::Coalesce(args) => { - for arg in args { - collect_columns(arg, deps); - } - } - SqlExpr::Case { - operand, - when_thens, - else_expr, - } => { - if let Some(op) = operand { - collect_columns(op, deps); - } - for (cond, then) in when_thens { - collect_columns(cond, deps); - collect_columns(then, deps); - } - if let Some(e) = else_expr { - collect_columns(e, deps); - } - } - SqlExpr::Cast { expr, .. } => collect_columns(expr, deps), - SqlExpr::NullIf(a, b) => { - collect_columns(a, deps); - collect_columns(b, deps); - } - SqlExpr::IsNull { expr, .. } => collect_columns(expr, deps), - SqlExpr::Literal(_) | SqlExpr::OldColumn(_) | SqlExpr::ExcludedColumn(_) => {} - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/nodedb-query/src/expr_parse/predicate.rs b/nodedb-query/src/expr_parse/predicate.rs new file mode 100644 index 000000000..562cf8f89 --- /dev/null +++ b/nodedb-query/src/expr_parse/predicate.rs @@ -0,0 +1,228 @@ +// SPDX-License-Identifier: Apache-2.0 + +//! Postfix predicates on an operand: `IS [NOT] NULL`, `[NOT] LIKE`, +//! `[NOT] ILIKE`, `[NOT] IN (...)` and `[NOT] BETWEEN ... AND ...`. +//! +//! Each lowers onto a form the evaluator already has, the same way the SQL +//! planner lowers it. LIKE and ILIKE become the `like` and `ilike` functions. +//! IN becomes an OR of equalities. BETWEEN becomes two comparisons. + +use super::error::ExprParseError; +use super::parser::{expect_keyword, parse_additive, parse_arg_list, peek_keyword}; +use super::tokenizer::Token; +use crate::expr::{BinaryOp, SqlExpr}; +use nodedb_types::Value; + +/// Parse the predicate that follows `operand`, if one does. Returns +/// `operand` unchanged when the next token starts no predicate. +pub(super) fn parse_postfix_predicate( + operand: SqlExpr, + tokens: &[Token], + pos: &mut usize, + depth: &mut usize, +) -> Result { + if peek_keyword(tokens, *pos, "IS") { + *pos += 1; + let negated = peek_keyword(tokens, *pos, "NOT"); + if negated { + *pos += 1; + } + expect_keyword(tokens, pos, "NULL")?; + return Ok(SqlExpr::IsNull { + expr: Box::new(operand), + negated, + }); + } + + // `NOT` after an operand only ever opens a negated predicate. + let negated = peek_keyword(tokens, *pos, "NOT") + && ["LIKE", "ILIKE", "IN", "BETWEEN"] + .iter() + .any(|keyword| peek_keyword(tokens, *pos + 1, keyword)); + let start = if negated { *pos + 1 } else { *pos }; + + let predicate = if peek_keyword(tokens, start, "LIKE") || peek_keyword(tokens, start, "ILIKE") { + let name = if peek_keyword(tokens, start, "ILIKE") { + "ilike" + } else { + "like" + }; + *pos = start + 1; + let pattern = parse_additive(tokens, pos, depth)?; + let mut args = vec![operand, pattern]; + if peek_keyword(tokens, *pos, "ESCAPE") { + *pos += 1; + args.push(parse_additive(tokens, pos, depth)?); + } + SqlExpr::Function { + name: name.to_string(), + args, + } + } else if peek_keyword(tokens, start, "IN") { + *pos = start + 1; + let list = parse_arg_list(tokens, pos, depth)?; + // `IN` is an OR of equalities, so a negated list is built from + // inequalities joined by AND rather than wrapped below. + return Ok(in_list(operand, list, negated)); + } else if peek_keyword(tokens, start, "BETWEEN") { + *pos = start + 1; + let low = parse_additive(tokens, pos, depth)?; + expect_keyword(tokens, pos, "AND")?; + let high = parse_additive(tokens, pos, depth)?; + // `NOT BETWEEN` is the outside of the range, built directly. + return Ok(between(operand, low, high, negated)); + } else { + return Ok(operand); + }; + + Ok(if negated { + SqlExpr::Negate(Box::new(predicate)) + } else { + predicate + }) +} + +fn binary(left: SqlExpr, op: BinaryOp, right: SqlExpr) -> SqlExpr { + SqlExpr::BinaryOp { + left: Box::new(left), + op, + right: Box::new(right), + } +} + +/// `target IN (list)` as `target = a OR target = b ...`. `NOT IN` is +/// `target <> a AND target <> b ...`. An empty list is false, or true when +/// negated. +fn in_list(target: SqlExpr, list: Vec, negated: bool) -> SqlExpr { + let (compare, combine) = if negated { + (BinaryOp::NotEq, BinaryOp::And) + } else { + (BinaryOp::Eq, BinaryOp::Or) + }; + list.into_iter() + .map(|item| binary(target.clone(), compare, item)) + .reduce(|acc, next| binary(acc, combine, next)) + .unwrap_or(SqlExpr::Literal(Value::Bool(negated))) +} + +/// `target BETWEEN low AND high` as `target >= low AND target <= high`. +/// `NOT BETWEEN` is `target < low OR target > high`. +fn between(target: SqlExpr, low: SqlExpr, high: SqlExpr, negated: bool) -> SqlExpr { + if negated { + binary( + binary(target.clone(), BinaryOp::Lt, low), + BinaryOp::Or, + binary(target, BinaryOp::Gt, high), + ) + } else { + binary( + binary(target.clone(), BinaryOp::GtEq, low), + BinaryOp::And, + binary(target, BinaryOp::LtEq, high), + ) + } +} + +#[cfg(test)] +mod tests { + use nodedb_types::Value; + + use super::super::parser::parse_generated_expr; + + fn eval(text: &str, doc: serde_json::Value) -> Value { + let (expr, _) = parse_generated_expr(text).expect("parses"); + expr.eval(&Value::from(doc)).expect("evaluates") + } + + #[test] + fn like_and_not_like() { + let doc = serde_json::json!({"email": "a@b.c"}); + assert_eq!(eval("email LIKE '%@%'", doc.clone()), Value::Bool(true)); + assert_eq!(eval("email NOT LIKE '%@%'", doc), Value::Bool(false)); + let plain = serde_json::json!({"email": "nobody"}); + assert_eq!(eval("email LIKE '%@%'", plain), Value::Bool(false)); + } + + #[test] + fn ilike_ignores_case() { + let doc = serde_json::json!({"name": "Alice"}); + assert_eq!(eval("name ILIKE 'ali%'", doc.clone()), Value::Bool(true)); + assert_eq!(eval("name LIKE 'ali%'", doc), Value::Bool(false)); + } + + #[test] + fn like_with_escape() { + let doc = serde_json::json!({"code": "50%"}); + assert_eq!(eval("code LIKE '50!%' ESCAPE '!'", doc), Value::Bool(true)); + let other = serde_json::json!({"code": "500"}); + assert_eq!( + eval("code LIKE '50!%' ESCAPE '!'", other), + Value::Bool(false) + ); + } + + #[test] + fn like_on_null_is_null() { + let doc = serde_json::json!({"email": null}); + assert_eq!(eval("email LIKE '%@%'", doc), Value::Null); + } + + #[test] + fn in_and_not_in() { + let doc = serde_json::json!({"role": "admin"}); + assert_eq!( + eval("role IN ('admin', 'user')", doc.clone()), + Value::Bool(true) + ); + assert_eq!( + eval("role NOT IN ('admin', 'user')", doc.clone()), + Value::Bool(false) + ); + assert_eq!(eval("role IN ('guest')", doc), Value::Bool(false)); + } + + #[test] + fn between_and_not_between() { + let doc = serde_json::json!({"age": 30}); + assert_eq!( + eval("age BETWEEN 18 AND 65", doc.clone()), + Value::Bool(true) + ); + assert_eq!( + eval("age NOT BETWEEN 18 AND 65", doc.clone()), + Value::Bool(false) + ); + assert_eq!(eval("age BETWEEN 40 AND 65", doc), Value::Bool(false)); + } + + #[test] + fn between_binds_inside_and() { + let doc = serde_json::json!({"age": 30, "active": true}); + assert_eq!( + eval("age BETWEEN 18 AND 65 AND active = true", doc), + Value::Bool(true) + ); + } + + #[test] + fn is_null_and_is_not_null() { + let doc = serde_json::json!({"a": null, "b": 1}); + assert_eq!(eval("a IS NULL", doc.clone()), Value::Bool(true)); + assert_eq!(eval("b IS NULL", doc.clone()), Value::Bool(false)); + assert_eq!(eval("b IS NOT NULL", doc), Value::Bool(true)); + } + + #[test] + fn not_applies_to_the_whole_comparison() { + let doc = serde_json::json!({"x": 1}); + assert_eq!(eval("NOT x = 2", doc.clone()), Value::Bool(true)); + assert_eq!(eval("NOT x = 1 OR x = 1", doc), Value::Bool(true)); + } + + #[test] + fn columns_inside_predicates_are_dependencies() { + let (_, deps) = + parse_generated_expr("a LIKE b AND c IN (d, 1) AND e BETWEEN f AND g").expect("parses"); + assert_eq!(deps, vec!["a", "b", "c", "d", "e", "f", "g"]); + } +} diff --git a/nodedb-query/src/expr_parse/validate.rs b/nodedb-query/src/expr_parse/validate.rs new file mode 100644 index 000000000..8f022597c --- /dev/null +++ b/nodedb-query/src/expr_parse/validate.rs @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: Apache-2.0 + +//! Checks on a parsed expression: determinism, and the columns it reads. + +use super::error::ExprParseError; +use crate::expr::SqlExpr; + +const NON_DETERMINISTIC: &[&str] = &[ + "now", + "current_timestamp", + "random", + "nextval", + "uuid", + "uuid_v4", + "uuid_v7", + "gen_random_uuid", + "ulid", + "cuid2", + "nanoid", +]; + +pub(super) fn validate_deterministic(expr: &SqlExpr) -> Result<(), ExprParseError> { + match expr { + SqlExpr::Function { name, args } => { + if NON_DETERMINISTIC.contains(&name.as_str()) { + return Err(ExprParseError::InvalidLiteral { + detail: format!( + "non-deterministic function '{name}()' not allowed in GENERATED ALWAYS AS" + ), + }); + } + for arg in args { + validate_deterministic(arg)?; + } + Ok(()) + } + SqlExpr::BinaryOp { left, right, .. } => { + validate_deterministic(left)?; + validate_deterministic(right) + } + SqlExpr::Negate(inner) => validate_deterministic(inner), + SqlExpr::Coalesce(args) => { + for arg in args { + validate_deterministic(arg)?; + } + Ok(()) + } + SqlExpr::Case { + operand, + when_thens, + else_expr, + } => { + if let Some(op) = operand { + validate_deterministic(op)?; + } + for (cond, then) in when_thens { + validate_deterministic(cond)?; + validate_deterministic(then)?; + } + if let Some(e) = else_expr { + validate_deterministic(e)?; + } + Ok(()) + } + SqlExpr::Cast { expr, .. } => validate_deterministic(expr), + SqlExpr::NullIf(a, b) => { + validate_deterministic(a)?; + validate_deterministic(b) + } + SqlExpr::IsNull { expr, .. } => validate_deterministic(expr), + SqlExpr::Column(_) + | SqlExpr::Literal(_) + | SqlExpr::OldColumn(_) + | SqlExpr::ExcludedColumn(_) => Ok(()), + } +} + +pub(super) fn collect_columns(expr: &SqlExpr, deps: &mut Vec) { + match expr { + SqlExpr::Column(name) => deps.push(name.clone()), + SqlExpr::BinaryOp { left, right, .. } => { + collect_columns(left, deps); + collect_columns(right, deps); + } + SqlExpr::Negate(inner) => collect_columns(inner, deps), + SqlExpr::Function { args, .. } => { + for arg in args { + collect_columns(arg, deps); + } + } + SqlExpr::Coalesce(args) => { + for arg in args { + collect_columns(arg, deps); + } + } + SqlExpr::Case { + operand, + when_thens, + else_expr, + } => { + if let Some(op) = operand { + collect_columns(op, deps); + } + for (cond, then) in when_thens { + collect_columns(cond, deps); + collect_columns(then, deps); + } + if let Some(e) = else_expr { + collect_columns(e, deps); + } + } + SqlExpr::Cast { expr, .. } => collect_columns(expr, deps), + SqlExpr::NullIf(a, b) => { + collect_columns(a, deps); + collect_columns(b, deps); + } + SqlExpr::IsNull { expr, .. } => collect_columns(expr, deps), + SqlExpr::Literal(_) | SqlExpr::OldColumn(_) | SqlExpr::ExcludedColumn(_) => {} + } +} diff --git a/nodedb-query/src/msgpack_scan/filter.rs b/nodedb-query/src/msgpack_scan/filter.rs index 9ec23d83a..27d3c0380 100644 --- a/nodedb-query/src/msgpack_scan/filter.rs +++ b/nodedb-query/src/msgpack_scan/filter.rs @@ -167,7 +167,13 @@ impl ScanFilter { | FilterOp::NeColumn => {} } - let (start, end) = match idx.get(&self.field) { + // Same lookup order as `matches_binary`: the exact key, then a + // qualified key carrying the bare name. Absent, and so NULL, only + // when neither exists. + let (start, end) = match idx + .get(&self.field) + .or_else(|| find_field_by_suffix(doc, &format!(".{}", self.field))) + { Some(r) => r, None => return Ok(self.op == FilterOp::IsNull), }; diff --git a/nodedb-query/src/scan_filter/types.rs b/nodedb-query/src/scan_filter/types.rs index 60432e81a..1d94abbf7 100644 --- a/nodedb-query/src/scan_filter/types.rs +++ b/nodedb-query/src/scan_filter/types.rs @@ -148,7 +148,13 @@ impl ScanFilter { | FilterOp::NeColumn => {} } - let field_val = match doc.get(&self.field) { + // A bare name also reaches a qualified field (`amount` reads + // `orders.amount`), as the binary matcher's suffix lookup does. The + // field reads as absent, and so NULL, only when neither exists. + let field_val = match doc + .get(&self.field) + .or_else(|| field_by_suffix(doc, &self.field)) + { Some(v) => v, None => return Ok(self.op == FilterOp::IsNull), }; @@ -258,7 +264,10 @@ impl ScanFilter { nodedb_types::Value::String(s) => s.as_str(), _ => return Ok(false), }; - let other_val = match doc.get(other_col) { + let other_val = match doc + .get(other_col) + .or_else(|| field_by_suffix(doc, other_col)) + { Some(v) => v, None => return Ok(false), }; @@ -315,6 +324,29 @@ pub(crate) fn column_compare( } } +/// The field a bare name reaches in a row keyed `qualifier.column`: the +/// value under a key ending in `.name`. When several qualifiers carry the +/// name, the smallest key wins, so the choice does not depend on map order. +/// `None` for a qualified name, and when no key carries it. +fn field_by_suffix<'a>( + doc: &'a nodedb_types::Value, + name: &str, +) -> Option<&'a nodedb_types::Value> { + if name.is_empty() || name.contains('.') { + return None; + } + let nodedb_types::Value::Object(map) = doc else { + return None; + }; + map.iter() + .filter(|(key, _)| { + key.strip_suffix(name) + .is_some_and(|qualifier| qualifier.ends_with('.') && qualifier.len() > 1) + }) + .min_by(|(a, _), (b, _)| a.cmp(b)) + .map(|(_, value)| value) +} + #[cfg(test)] mod tests { use super::*; diff --git a/nodedb-sql/src/aggregate_walk.rs b/nodedb-sql/src/aggregate_walk.rs index ba0cf198e..b517425c8 100644 --- a/nodedb-sql/src/aggregate_walk.rs +++ b/nodedb-sql/src/aggregate_walk.rs @@ -33,7 +33,7 @@ use crate::parser::normalize::normalize_ident; use crate::resolver::ColumnScope; use crate::resolver::columns::TableScope; use crate::resolver::expr::convert_expr; -use crate::types::{AggregateExpr, SqlExpr}; +use crate::types::{AggregateExpr, SqlExpr, SqlValue}; /// Return `true` if any aggregate function call appears anywhere inside /// `expr` — at any nesting depth, inside any expression position. @@ -147,12 +147,36 @@ impl Visitor for AggregateExtractor<'_> { return ControlFlow::Break(()); } }; + let filter = match aggregate_filter(f, self.scope) { + Ok(filter) => filter, + Err(e) => { + self.error = Some(e); + return ControlFlow::Break(()); + } + }; + let mut arg_text = computed_arg_text(f, &args); + let (args, filter_text) = match filter { + Some((predicate, text)) => { + // The folded argument is a CASE; the key keeps naming + // the argument as written. + if arg_text.is_none() { + arg_text = Some(match args.first() { + Some(SqlExpr::Column { name, .. }) => name.clone(), + _ => "*".to_string(), + }); + } + (filtered_args(args, predicate), Some(text)) + } + None => (args, None), + }; self.out.push(AggregateExpr { function: function_name(f), args, alias: self.alias.into(), distinct, grouping_col_index: None, + filter_text, + arg_text, }); self.inside_aggregate += 1; } @@ -239,6 +263,67 @@ fn function_args_and_distinct( Ok((parsed, distinct)) } +/// Convert an aggregate call's `FILTER (WHERE ...)` clause against `scope`: +/// the predicate and its lowercase source text. +/// +/// An aggregate inside the predicate is rejected by the extractor's nested +/// aggregate check, because the visitor descends into the clause while the +/// enclosing aggregate is open. +fn aggregate_filter(f: &ast::Function, scope: &TableScope) -> Result> { + let Some(predicate) = f.filter.as_deref() else { + return Ok(None); + }; + Ok(Some(( + convert_expr(predicate, &ColumnScope::Relations(scope))?, + format!("{predicate}").to_lowercase(), + ))) +} + +/// Fold a `FILTER` predicate into the aggregate's arguments. +/// +/// Each argument becomes `CASE WHEN THEN END`, NULL for +/// a row the predicate does not admit. Every aggregate skips a NULL input, +/// so the result equals PostgreSQL's FILTER semantics. `COUNT(*)` counts +/// rows, so its wildcard becomes the constant 1. +fn filtered_args(args: Vec, predicate: SqlExpr) -> Vec { + let args = if args.is_empty() { + vec![SqlExpr::Wildcard] + } else { + args + }; + args.into_iter() + .map(|arg| { + let value = match arg { + SqlExpr::Wildcard => SqlExpr::Literal(SqlValue::Int(1)), + other => other, + }; + SqlExpr::Case { + operand: None, + when_then: vec![(predicate.clone(), value)], + else_expr: None, + } + }) + .collect() +} + +/// Lowercase source text of the first argument when it is computed. A +/// column or wildcard argument is named by the column itself. +fn computed_arg_text(f: &ast::Function, args: &[SqlExpr]) -> Option { + match args.first() { + None | Some(SqlExpr::Column { .. }) | Some(SqlExpr::Wildcard) => return None, + Some(_) => {} + } + let ast::FunctionArguments::List(list) = &f.args else { + return None; + }; + list.args.iter().find_map(|a| match a { + ast::FunctionArg::Unnamed(ast::FunctionArgExpr::Expr(e)) => { + Some(format!("{e}").to_lowercase()) + } + _ => None, + }) +} + #[cfg(test)] mod tests { use super::*; diff --git a/nodedb-sql/src/catalog.rs b/nodedb-sql/src/catalog.rs index 4638193ea..f5de86a1a 100644 --- a/nodedb-sql/src/catalog.rs +++ b/nodedb-sql/src/catalog.rs @@ -112,6 +112,11 @@ pub enum SqlCatalogError { /// collection is hard-deleted by the GC sweeper. retention_expires_at_ns: u64, }, + + /// The relation's catalog record exists but could not be read: a + /// stored definition that does not decode, or a catalog read error. + #[error("relation '{relation}' could not be read: {detail}")] + Unreadable { relation: String, detail: String }, } /// A relation that resolves to planner metadata without a stored collection. diff --git a/nodedb-sql/src/ddl_ast/collection_type/build.rs b/nodedb-sql/src/ddl_ast/collection_type/build.rs index 1bab39ff2..c70820d23 100644 --- a/nodedb-sql/src/ddl_ast/collection_type/build.rs +++ b/nodedb-sql/src/ddl_ast/collection_type/build.rs @@ -60,7 +60,14 @@ pub fn build_collection_type( } Some("timeseries") => { reject_reserved_columns(columns)?; + // The collection type's interval is the one source a flush reads. + // Undeclared, it is one hour. A declared one must parse. let partition_by = opt_val("partition_by").unwrap_or_else(|| "1h".to_string()); + nodedb_types::timeseries::PartitionInterval::parse(&partition_by).map_err(|e| { + SqlError::DataException { + detail: format!("invalid partition_by '{partition_by}': {e}"), + } + })?; let time_key = resolve_time_key(columns)?; Ok(( nodedb_types::CollectionType::timeseries(time_key, partition_by), diff --git a/nodedb-sql/src/ddl_ast/collection_type/kv.rs b/nodedb-sql/src/ddl_ast/collection_type/kv.rs index 7fb59a8a1..284926847 100644 --- a/nodedb-sql/src/ddl_ast/collection_type/kv.rs +++ b/nodedb-sql/src/ddl_ast/collection_type/kv.rs @@ -67,10 +67,11 @@ pub(crate) fn build_kv_collection_type( } // Validate: PK type must be hashable. - let pk = col_defs - .iter() - .find(|c| c.primary_key) - .expect("invariant: pk_count validated to be exactly 1 in the checks above"); + let Some(pk) = col_defs.iter().find(|c| c.primary_key) else { + return Err(SqlError::Parse { + detail: "KV collections require a PRIMARY KEY column (the hash key)".to_string(), + }); + }; if !nodedb_types::is_valid_kv_key_type(&pk.column_type) { return Err(SqlError::Parse { detail: format!( @@ -86,7 +87,7 @@ pub(crate) fn build_kv_collection_type( })?; let ttl = build_kv_ttl(options, &schema)?; - let capacity_hint = build_kv_capacity(options); + let capacity_hint = build_kv_capacity(options)?; let config = nodedb_types::KvConfig { schema, @@ -150,11 +151,62 @@ fn build_kv_ttl( })) } -/// Parse optional `capacity` from the options list. -fn build_kv_capacity(options: &[(String, String)]) -> u32 { - options +/// Parse optional `capacity` from the options list. Absent is `0`, which +/// sizes the table by the engine default. A value that is not a positive +/// integer within `u32` is refused by value, never replaced by the default. +fn build_kv_capacity(options: &[(String, String)]) -> Result { + let Some((_, raw)) = options .iter() .find(|(k, _)| k.eq_ignore_ascii_case("capacity")) - .and_then(|(_, v)| v.trim().parse::().ok()) - .unwrap_or(0) + else { + return Ok(0); + }; + let text = raw.trim().trim_matches('\''); + match text.parse::() { + Ok(capacity) if capacity > 0 => Ok(capacity), + _ => Err(SqlError::Parse { + detail: format!( + "KV capacity '{raw}' is not valid; capacity must be a positive integer up to {}", + u32::MAX + ), + }), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn options(value: &str) -> Vec<(String, String)> { + vec![("capacity".to_string(), value.to_string())] + } + + #[test] + fn an_absent_capacity_is_the_default() { + assert_eq!(build_kv_capacity(&[]).expect("absent capacity"), 0); + } + + #[test] + fn a_positive_capacity_is_kept() { + assert_eq!( + build_kv_capacity(&options("10000")).expect("capacity"), + 10_000 + ); + assert_eq!( + build_kv_capacity(&options("'64'")).expect("quoted capacity"), + 64 + ); + } + + #[test] + fn a_bad_capacity_is_refused_by_value() { + for bad in ["lots", "-5", "0", "1.5", "99999999999"] { + match build_kv_capacity(&options(bad)) { + Err(SqlError::Parse { detail }) => { + assert!(detail.contains(&format!("'{bad}'")), "{detail}"); + } + other => panic!("capacity '{bad}' must be refused, got {other:?}"), + } + } + } } diff --git a/nodedb-sql/src/ddl_ast/graph_parse/cursor.rs b/nodedb-sql/src/ddl_ast/graph_parse/cursor.rs new file mode 100644 index 000000000..40ad8c061 --- /dev/null +++ b/nodedb-sql/src/ddl_ast/graph_parse/cursor.rs @@ -0,0 +1,278 @@ +// SPDX-License-Identifier: Apache-2.0 + +//! Left-to-right clause reader for the graph DSL. +//! +//! A statement parser reads its clauses in source order. A clause keyword is +//! matched only at a clause position, and its value is read from the tokens +//! right after it. A token at a clause position that names no clause of the +//! statement is a parse error, so a misspelled keyword never falls back to +//! the clause default. A clause given twice is a parse error. + +use super::super::graph_types::{GraphDirection, GraphProperties}; +use super::tokenizer::Tok; +use crate::error::SqlError; + +pub(super) struct Cursor<'t, 'a> { + toks: &'t [Tok<'a>], + pos: usize, + statement: &'static str, +} + +impl<'t, 'a> Cursor<'t, 'a> { + /// A cursor over `toks` after its `command_words` (2 for `GRAPH TRAVERSE`). + pub(super) fn new(toks: &'t [Tok<'a>], statement: &'static str, command_words: usize) -> Self { + Self { + toks, + pos: command_words.min(toks.len()), + statement, + } + } + + /// The statement name error messages use, such as `GRAPH TRAVERSE`. + pub(super) fn statement(&self) -> &'static str { + self.statement + } + + fn peek(&self) -> Option<&'t Tok<'a>> { + let toks: &'t [Tok<'a>] = self.toks; + toks.get(self.pos) + } + + /// The next clause keyword, upper-cased. `None` at the end of the + /// statement. A string, identifier, object or list at a clause position + /// is refused. + pub(super) fn next_clause(&mut self) -> Result, SqlError> { + match self.peek() { + None => Ok(None), + Some(Tok::Word(word)) => { + self.pos += 1; + Ok(Some(word.to_ascii_uppercase())) + } + Some(other) => Err(self.unexpected(other)), + } + } + + /// The error for the keyword `next_clause` returned last, when the + /// statement has no such clause. + pub(super) fn unknown_clause(&self) -> SqlError { + match self + .pos + .checked_sub(1) + .and_then(|index| self.toks.get(index)) + { + Some(tok) => self.unexpected(tok), + None => parse_error(format!("unexpected end of {}", self.statement)), + } + } + + /// Consume the next token when it is the bare word `keyword`. + pub(super) fn eat_keyword(&mut self, keyword: &str) -> bool { + let hit = + matches!(self.peek(), Some(Tok::Word(word)) if word.eq_ignore_ascii_case(keyword)); + if hit { + self.pos += 1; + } + hit + } + + /// A node, collection or label: a quoted string, a quoted identifier, or + /// a bare word. + pub(super) fn value(&mut self, clause: &str) -> Result { + let value = match self.peek() { + Some(Tok::Quoted(text) | Tok::Ident(text)) => text.clone().into_owned(), + Some(Tok::Word(word)) => (*word).to_string(), + other => { + return Err(parse_error(format!( + "{clause} needs a value — found {}", + describe_opt(other) + ))); + } + }; + self.pos += 1; + Ok(value) + } + + /// A bare word. + pub(super) fn word(&mut self, clause: &str) -> Result<&'a str, SqlError> { + match self.peek() { + Some(Tok::Word(word)) => { + self.pos += 1; + Ok(*word) + } + other => Err(parse_error(format!( + "{clause} needs a word — found {}", + describe_opt(other) + ))), + } + } + + pub(super) fn usize(&mut self, clause: &str) -> Result { + let text = self.number_text(clause, "a non-negative integer")?; + text.parse().map_err(|_| { + parse_error(format!( + "{clause} must be a non-negative integer — found '{text}'" + )) + }) + } + + pub(super) fn f64(&mut self, clause: &str) -> Result { + let text = self.number_text(clause, "a number")?; + text.parse() + .map_err(|_| parse_error(format!("{clause} must be a number — found '{text}'"))) + } + + /// Consume the next word when it parses as a number. + pub(super) fn number_word(&mut self) -> Option { + let Some(Tok::Word(word)) = self.peek() else { + return None; + }; + let number = word.parse().ok()?; + self.pos += 1; + Some(number) + } + + fn number_text(&mut self, clause: &str, expected: &str) -> Result<&'a str, SqlError> { + match self.peek() { + Some(Tok::Word(word)) => { + self.pos += 1; + Ok(*word) + } + other => Err(parse_error(format!( + "{clause} must be {expected} — found {}", + describe_opt(other) + ))), + } + } + + /// One or more quoted strings: `'a', 'b'` or `('a', 'b')`. + pub(super) fn quoted_list(&mut self, clause: &str) -> Result, SqlError> { + let mut out = Vec::new(); + while let Some(Tok::Quoted(text)) = self.peek() { + out.push(text.clone().into_owned()); + self.pos += 1; + } + if out.is_empty() { + return Err(parse_error(format!( + "{} {clause} needs one or more quoted labels, as {clause} 'a', 'b'", + self.statement + ))); + } + Ok(out) + } + + /// A `{...}` object literal, braces included. + pub(super) fn object(&mut self, clause: &str) -> Result { + match self.peek() { + Some(Tok::Object(text)) => { + self.pos += 1; + Ok((*text).to_string()) + } + other => Err(parse_error(format!( + "{clause} needs an object literal {{...}} — found {}", + describe_opt(other) + ))), + } + } + + /// The content of a `[...]` list. + pub(super) fn list(&mut self, clause: &str) -> Result<&'a str, SqlError> { + match self.peek() { + Some(Tok::List(text)) => { + self.pos += 1; + Ok(*text) + } + other => Err(parse_error(format!( + "{clause} needs ARRAY[...] — found {}", + describe_opt(other) + ))), + } + } + + /// A `PROPERTIES` value: an object literal or a quoted JSON document. + pub(super) fn properties(&mut self) -> Result { + let properties = match self.peek() { + Some(Tok::Object(text)) => GraphProperties::Object((*text).to_string()), + Some(Tok::Quoted(text)) => GraphProperties::Quoted(text.clone().into_owned()), + other => { + return Err(parse_error(format!( + "PROPERTIES needs an object literal {{...}} or a quoted JSON document — found {}", + describe_opt(other) + ))); + } + }; + self.pos += 1; + Ok(properties) + } + + /// A `DIRECTION` value. A word outside `in`, `out`, `both` is refused by + /// name. + pub(super) fn direction(&mut self) -> Result { + let word = match self.peek() { + Some(Tok::Word(word)) => { + self.pos += 1; + *word + } + other => { + return Err(parse_error(format!( + "DIRECTION must be one of in, out, both — found {}", + describe_opt(other) + ))); + } + }; + match word.to_ascii_uppercase().as_str() { + "IN" => Ok(GraphDirection::In), + "OUT" => Ok(GraphDirection::Out), + "BOTH" => Ok(GraphDirection::Both), + _ => Err(parse_error(format!( + "DIRECTION must be one of in, out, both — found '{word}'" + ))), + } + } + + fn unexpected(&self, tok: &Tok<'_>) -> SqlError { + parse_error(format!( + "unexpected {} in {}", + describe(tok), + self.statement + )) + } +} + +/// Store a clause value. A clause the statement already has is refused. +pub(super) fn set_once( + slot: &mut Option, + value: T, + clause: &str, + statement: &str, +) -> Result<(), SqlError> { + if slot.is_some() { + return Err(parse_error(format!( + "{clause} given more than once in {statement}" + ))); + } + *slot = Some(value); + Ok(()) +} + +/// A required clause was absent. +pub(super) fn missing_clause(statement: &str, clause: &str) -> SqlError { + parse_error(format!("{statement} requires {clause}")) +} + +fn describe(tok: &Tok<'_>) -> String { + match tok { + Tok::Word(word) => format!("token '{word}'"), + Tok::Quoted(text) => format!("string '{text}'"), + Tok::Ident(text) => format!("identifier \"{text}\""), + Tok::Object(_) => "an object literal".to_owned(), + Tok::List(_) => "a [...] list".to_owned(), + } +} + +fn describe_opt(tok: Option<&Tok<'_>>) -> String { + tok.map_or_else(|| "end of statement".to_owned(), describe) +} + +fn parse_error(detail: String) -> SqlError { + SqlError::Parse { detail } +} diff --git a/nodedb-sql/src/ddl_ast/graph_parse/entry.rs b/nodedb-sql/src/ddl_ast/graph_parse/entry.rs index f49018ae3..151946d4e 100644 --- a/nodedb-sql/src/ddl_ast/graph_parse/entry.rs +++ b/nodedb-sql/src/ddl_ast/graph_parse/entry.rs @@ -28,12 +28,17 @@ pub fn try_parse(sql: &str) -> Option> { return None; } - Some(parse_graph(trimmed, &upper)) + Some(parse_graph(trimmed)) } -/// Parse a statement known to start with `GRAPH `. A trailing `EDGE WHERE` -/// predicate splits off first, so the clause text before it tokenizes alone. -fn parse_graph(trimmed: &str, upper: &str) -> Result { +/// Parse a statement known to start with `GRAPH `. Comments are blanked +/// first. A trailing `EDGE WHERE` predicate splits off next, so the clause +/// text before it tokenizes alone. +fn parse_graph(sql: &str) -> Result { + let blanked = tokenizer::blank_comments(sql); + let trimmed = blanked.trim(); + let upper = trimmed.to_ascii_uppercase(); + let upper = upper.as_str(); let (head, predicate_text) = edge_predicate::split_edge_where(trimmed)?; let predicate = match predicate_text { Some(text) => edge_predicate::parse_edge_predicate(text)?, @@ -70,7 +75,7 @@ fn parse_graph(trimmed: &str, upper: &str) -> Result } else if upper.starts_with("GRAPH ALGO ") { variants::parse_algo(&toks) } else if upper.starts_with("GRAPH RAG FUSION ") { - variants::parse_rag_fusion(&toks, trimmed) + variants::parse_rag_fusion(&toks) } else { // Starts with `GRAPH ` but names no known command. Still graph DSL, // so report it here rather than letting the SQL parser guess. @@ -532,4 +537,142 @@ mod tests { "an omitted DIRECTION must keep defaulting" ); } + + // ── Unconsumed and misplaced tokens ─────────────────────────────────── + + fn parse_error(sql: &str) -> String { + try_parse(sql) + .expect("input is graph DSL") + .expect_err("statement must be refused") + .to_string() + } + + #[test] + fn misspelled_clause_keyword_is_named() { + let error = parse_error("GRAPH TRAVERSE FROM 1 DEPTS 3 IN g"); + assert!( + error.contains("unexpected token 'DEPTS' in GRAPH TRAVERSE"), + "{error}" + ); + let error = parse_error("GRAPH NEIGHBORS OF 1 LABELS 'follows' IN g"); + assert!( + error.contains("unexpected token 'LABELS' in GRAPH NEIGHBORS"), + "{error}" + ); + } + + #[test] + fn stray_tokens_are_refused() { + for (sql, token) in [ + ("GRAPH TRAVERSE FROM 1 SIDEWAYS IN g", "SIDEWAYS"), + ("GRAPH TRAVERSE FROM 1 BANANA PHONE IN g", "BANANA"), + ("GRAPH INSERT EDGE IN 'e' FROM 'a' TO 'b' TYP 'x'", "TYP"), + ( + "GRAPH DELETE EDGE IN 'e' FROM 'a' TO 'b' TYPE 'x' EXTRA", + "EXTRA", + ), + ("GRAPH PATH IN 'g' FROM 'a' TO 'b' MAXDEPTH 3", "MAXDEPTH"), + ("GRAPH LABEL 'a' AS 'P' EXTRA", "EXTRA"), + ("GRAPH ALGO PAGERANK ON g ITERATION 5", "ITERATION"), + ( + "GRAPH RAG FUSION ON e QUERY ARRAY[0.1] VECTOR_TOPK 5", + "VECTOR_TOPK", + ), + ("GRAPH TRAVERSE IN 'g' FROM 'a' ]", "]"), + ] { + let error = parse_error(sql); + assert!( + error.contains(&format!("unexpected token '{token}'")), + "{sql}: {error}" + ); + } + } + + #[test] + fn stray_string_at_clause_position_is_refused() { + let error = parse_error("GRAPH TRAVERSE IN 'g' FROM 'a' 'b'"); + assert!(error.contains("unexpected string 'b'"), "{error}"); + } + + #[test] + fn repeated_clause_is_refused() { + let error = parse_error("GRAPH TRAVERSE IN 'g' FROM 'a' DEPTH 1 DEPTH 3"); + assert!( + error.contains("DEPTH given more than once in GRAPH TRAVERSE"), + "{error}" + ); + } + + #[test] + fn direction_value_in_does_not_shadow_in_clause() { + match parsed("GRAPH TRAVERSE FROM 'b' DIRECTION in IN 'g'") { + NodedbStatement::Graph(GraphStmt::GraphTraverse { + collection, + start, + direction, + .. + }) => { + assert_eq!(collection, "g"); + assert_eq!(start, "b"); + assert_eq!(direction, GraphDirection::In); + } + other => panic!("expected GraphTraverse, got {other:?}"), + } + } + + #[test] + fn non_numeric_clause_values_are_refused() { + let error = parse_error("GRAPH ALGO PAGERANK ON g DAMPING abc"); + assert!(error.contains("DAMPING must be a number"), "{error}"); + let error = parse_error("GRAPH TRAVERSE IN 'g' FROM 'a' DEPTH '3'"); + assert!( + error.contains("DEPTH must be a non-negative integer"), + "{error}" + ); + let error = parse_error("GRAPH RAG FUSION ON e QUERY ARRAY[0.1, x]"); + assert!(error.contains("'x'"), "{error}"); + } + + #[test] + fn comments_are_not_tokens() { + match parsed("GRAPH TRAVERSE IN 'g' /* note */ FROM 'a' -- trailing note") { + NodedbStatement::Graph(GraphStmt::GraphTraverse { + collection, start, .. + }) => { + assert_eq!(collection, "g"); + assert_eq!(start, "a"); + } + other => panic!("expected GraphTraverse, got {other:?}"), + } + } + + #[test] + fn quoted_identifier_is_a_value() { + match parsed(r#"GRAPH NEIGHBORS IN "My Graph" OF 'a'"#) { + NodedbStatement::Graph(GraphStmt::GraphNeighbors { collection, .. }) => { + assert_eq!(collection, "My Graph"); + } + other => panic!("expected GraphNeighbors, got {other:?}"), + } + } + + #[test] + fn rag_fusion_bm25_field_is_the_on_after_bm25() { + let params_of = |sql: &str| match parsed(sql) { + NodedbStatement::Graph(GraphStmt::GraphRagFusion { collection, params }) => { + assert_eq!(collection, "entities"); + params + } + other => panic!("expected GraphRagFusion, got {other:?}"), + }; + let params = params_of( + "GRAPH RAG FUSION ON entities QUERY ARRAY[0.1] BM25 'q' ON 'body' \ + RRF_K (1.0, 2.0, 3.0)", + ); + assert_eq!(params.bm25_query.as_deref(), Some("q")); + assert_eq!(params.bm25_field.as_deref(), Some("body")); + assert_eq!(params.rrf_k_triple, Some((1.0, 2.0, 3.0))); + let params = params_of("GRAPH RAG FUSION ON entities QUERY ARRAY[0.1]"); + assert_eq!(params.bm25_field, None); + } } diff --git a/nodedb-sql/src/ddl_ast/graph_parse/fusion_params.rs b/nodedb-sql/src/ddl_ast/graph_parse/fusion_params.rs index f2f237675..1bbf55fea 100644 --- a/nodedb-sql/src/ddl_ast/graph_parse/fusion_params.rs +++ b/nodedb-sql/src/ddl_ast/graph_parse/fusion_params.rs @@ -2,23 +2,20 @@ //! Shared parameter extraction for graph-vector fusion SQL surfaces. //! -//! Two syntaxes reach the same `GraphOp::RagFusion` executor today: +//! Two syntaxes reach the same `GraphOp::RagFusion` executor: //! //! - `GRAPH RAG FUSION ON QUERY ARRAY[...] ...` (DSL form) //! - `SEARCH USING FUSION(ARRAY[...] ...)` (wrapped form) //! //! They use different keyword aliases for the same parameters //! (`EXPANSION_DEPTH` vs `DEPTH`, `EDGE_LABEL` vs `LABEL`, `FINAL_TOP_K` -//! vs `TOP`). Both must extract the same typed bag so future fusion -//! variants (hybrid text+vector, multi-vector, etc.) can share this -//! code and cannot silently drop parameters the way substring-find -//! parsing did. +//! vs `TOP`). Both read into the same typed bag through one left-to-right +//! clause reader, so an unknown or misspelled parameter is refused by name. use super::super::statement::GraphDirection; -use super::helpers::{ - array_floats_after, float_pair_after, float_triple_after, quoted_after, usize_after, word_after, -}; -use super::tokenizer::{Tok, tokenize}; +use super::cursor::{Cursor, set_once}; +use super::tokenizer::{Tok, blank_comments, tokenize}; +use crate::error::SqlError; /// Keyword aliases for the shared fusion parameters. /// @@ -108,65 +105,103 @@ pub struct FusionParams { } impl FusionParams { - pub(super) fn extract(toks: &[Tok<'_>], sql: &str, kw: &FusionKeywords) -> Self { - let direction = match word_after(toks, kw.direction) - .as_deref() - .map(str::to_ascii_uppercase) - .as_deref() - { - Some("IN") => Some(GraphDirection::In), - Some("BOTH") => Some(GraphDirection::Both), - Some("OUT") => Some(GraphDirection::Out), - _ => None, - }; - - // Try to parse a three-value RRF_K triple first; fall back to the - // two-value pair. This way `RRF_K (60.0, 35.0, 50.0)` populates - // `rrf_k_triple` and leaves `rrf_k` as None, while the legacy - // `RRF_K (60.0, 35.0)` continues to populate only `rrf_k`. - let rrf_k_triple = float_triple_after(toks, kw.rrf_k); - let rrf_k = if rrf_k_triple.is_some() { - None - } else { - float_pair_after(toks, kw.rrf_k) - }; - - // BM25 text leg — only parsed when the keyword is non-empty. - let (bm25_query, bm25_field) = if !kw.bm25_query.is_empty() { - ( - quoted_after(toks, kw.bm25_query), - quoted_after(toks, kw.bm25_field), - ) - } else { - (None, None) - }; - - Self { - query_vector: array_floats_after(sql, kw.query_anchor), - vector_top_k: usize_after(toks, kw.vector_top_k), - expansion_depth: usize_after(toks, kw.expansion_depth), - edge_label: quoted_after(toks, kw.edge_label), - final_top_k: usize_after(toks, kw.final_top_k), - rrf_k, - rrf_k_triple, - vector_field: quoted_after(toks, kw.vector_field), - direction, - max_visited: usize_after(toks, kw.max_visited), - bm25_query, - bm25_field, + /// Read the fusion clauses left to right until the end of the statement. + /// + /// A keyword outside `kw` is refused by name, a clause given twice is + /// refused, and a value that does not read as its type is refused. + pub(super) fn parse(c: &mut Cursor<'_, '_>, kw: &FusionKeywords) -> Result { + let stmt = c.statement(); + let mut p = Self::default(); + while let Some(clause) = c.next_clause()? { + let k = clause.as_str(); + if k == kw.query_anchor { + if kw.query_anchor != "ARRAY" && !c.eat_keyword("ARRAY") { + return Err(SqlError::Parse { + detail: format!("{k} needs ARRAY[...] in {stmt}"), + }); + } + let vector = parse_vector(c.list(k)?, k)?; + set_once(&mut p.query_vector, vector, k, stmt)?; + } else if k == kw.vector_top_k { + set_once(&mut p.vector_top_k, c.usize(k)?, k, stmt)?; + } else if k == kw.expansion_depth { + set_once(&mut p.expansion_depth, c.usize(k)?, k, stmt)?; + } else if k == kw.edge_label { + set_once(&mut p.edge_label, c.value(k)?, k, stmt)?; + } else if k == kw.final_top_k { + set_once(&mut p.final_top_k, c.usize(k)?, k, stmt)?; + } else if k == kw.rrf_k { + if p.rrf_k.is_some() || p.rrf_k_triple.is_some() { + return Err(SqlError::Parse { + detail: format!("{k} given more than once in {stmt}"), + }); + } + match (c.number_word(), c.number_word(), c.number_word()) { + (Some(vector_k), Some(graph_k), None) => p.rrf_k = Some((vector_k, graph_k)), + (Some(vector_k), Some(text_k), Some(graph_k)) => { + p.rrf_k_triple = Some((vector_k, text_k, graph_k)); + } + _ => { + return Err(SqlError::Parse { + detail: format!("{k} needs two or three numbers, as {k} (60.0, 35.0)"), + }); + } + } + } else if k == kw.vector_field { + set_once(&mut p.vector_field, c.value(k)?, k, stmt)?; + } else if k == kw.direction { + set_once(&mut p.direction, c.direction()?, k, stmt)?; + } else if k == kw.max_visited { + set_once(&mut p.max_visited, c.usize(k)?, k, stmt)?; + } else if !kw.bm25_query.is_empty() && k == kw.bm25_query { + set_once(&mut p.bm25_query, c.value(k)?, k, stmt)?; + // The field clause belongs to the BM25 clause only when it + // follows it directly. + if c.eat_keyword(kw.bm25_field) { + set_once( + &mut p.bm25_field, + c.value(kw.bm25_field)?, + kw.bm25_field, + stmt, + )?; + } + } else { + return Err(c.unknown_clause()); + } } + Ok(p) + } +} + +/// Read the comma-separated numbers of an `ARRAY[...]` literal. +fn parse_vector(inner: &str, clause: &str) -> Result, SqlError> { + if inner.trim().is_empty() { + return Err(SqlError::Parse { + detail: format!("{clause} ARRAY[...] needs one or more numbers"), + }); } + inner + .split(',') + .map(|part| { + let text = part.trim(); + text.parse::().map_err(|_| SqlError::Parse { + detail: format!("{clause} ARRAY element must be a number — found '{text}'"), + }) + }) + .collect() } /// Parse `SEARCH USING FUSION(...)` into its collection name -/// and a typed [`FusionParams`]. Returns `None` when the SQL does not -/// match the expected shape. +/// and a typed [`FusionParams`]. /// -/// Body extraction uses the same quote- and bracket-aware tokenizer as -/// the `GRAPH RAG FUSION` path, so a keyword-shaped string literal (e.g. -/// a label value `'TOP'`) cannot shadow a real parameter keyword. -pub fn parse_search_using_fusion(sql: &str) -> Option<(String, FusionParams)> { - let toks = tokenize(sql); +/// `None` means the SQL does not start with `SEARCH USING +/// FUSION`. `Some(Err)` means it does and a clause is malformed. The body +/// goes through the same quote- and bracket-aware tokenizer and the same +/// clause reader as `GRAPH RAG FUSION`, so a keyword-shaped string literal +/// (e.g. a label value `'TOP'`) cannot shadow a real parameter keyword. +pub fn parse_search_using_fusion(sql: &str) -> Option> { + let sql = blank_comments(sql); + let toks = tokenize(&sql); let collection = match toks.as_slice() { [Tok::Word(s), Tok::Word(c), Tok::Word(u), Tok::Word(f), ..] if s.eq_ignore_ascii_case("SEARCH") @@ -177,10 +212,11 @@ pub fn parse_search_using_fusion(sql: &str) -> Option<(String, FusionParams)> { } _ => return None, }; - Some(( - collection, - FusionParams::extract(&toks, sql, &SEARCH_FUSION_KEYWORDS), - )) + let mut cursor = Cursor::new(&toks, "SEARCH USING FUSION", 4); + Some( + FusionParams::parse(&mut cursor, &SEARCH_FUSION_KEYWORDS) + .map(|params| (collection, params)), + ) } #[cfg(test)] @@ -193,7 +229,8 @@ mod tests { "SEARCH mycol USING FUSION(ARRAY[0.1, 0.2] VECTOR_TOP_K 5 DEPTH 2 \ LABEL 'related' TOP 10 RRF_K (60.0, 35.0))", ) - .unwrap(); + .expect("SEARCH USING FUSION form") + .expect("well-formed fusion clauses"); assert_eq!(col, "mycol"); assert_eq!(p.query_vector.as_deref().map(<[f32]>::len), Some(2)); assert_eq!(p.vector_top_k, Some(5)); @@ -211,7 +248,8 @@ mod tests { VECTOR_TOP_K 50 BM25 'transformer attention' ON 'body' \ DEPTH 2 LABEL 'related_to' TOP 10 RRF_K (60.0, 35.0, 50.0))", ) - .unwrap(); + .expect("SEARCH USING FUSION form") + .expect("well-formed fusion clauses"); assert_eq!(col, "entities"); assert_eq!(p.rrf_k, None); assert_eq!(p.rrf_k_triple, Some((60.0, 35.0, 50.0))); @@ -229,7 +267,8 @@ mod tests { // quoted strings whole, so `TOP 10` is the real parameter. let (_, p) = parse_search_using_fusion("SEARCH c USING FUSION(ARRAY[0.5] LABEL 'TOP_SECRET' TOP 7)") - .unwrap(); + .expect("SEARCH USING FUSION form") + .expect("well-formed fusion clauses"); assert_eq!(p.edge_label.as_deref(), Some("TOP_SECRET")); assert_eq!(p.final_top_k, Some(7)); } @@ -239,4 +278,38 @@ mod tests { assert!(parse_search_using_fusion("INSERT INTO x VALUES (1)").is_none()); assert!(parse_search_using_fusion("SEARCH x USING VECTOR(ARRAY[1.0])").is_none()); } + + fn search_error(sql: &str) -> String { + parse_search_using_fusion(sql) + .expect("SEARCH USING FUSION form") + .expect_err("malformed fusion clauses must be refused") + .to_string() + } + + #[test] + fn search_fusion_refuses_misspelled_parameter() { + let error = search_error("SEARCH c USING FUSION(ARRAY[0.5] TOPP 7)"); + assert!( + error.contains("unexpected token 'TOPP' in SEARCH USING FUSION"), + "{error}" + ); + } + + #[test] + fn search_fusion_refuses_non_numeric_vector_element() { + let error = search_error("SEARCH c USING FUSION(ARRAY[0.1, x] TOP 3)"); + assert!(error.contains("'x'"), "{error}"); + } + + #[test] + fn search_fusion_refuses_unknown_direction() { + let error = search_error("SEARCH c USING FUSION(ARRAY[0.1] DIRECTION sideways)"); + assert!(error.contains("sideways"), "{error}"); + } + + #[test] + fn search_fusion_refuses_repeated_parameter() { + let error = search_error("SEARCH c USING FUSION(ARRAY[0.1] TOP 3 TOP 4)"); + assert!(error.contains("TOP given more than once"), "{error}"); + } } diff --git a/nodedb-sql/src/ddl_ast/graph_parse/helpers.rs b/nodedb-sql/src/ddl_ast/graph_parse/helpers.rs deleted file mode 100644 index 14afef8af..000000000 --- a/nodedb-sql/src/ddl_ast/graph_parse/helpers.rs +++ /dev/null @@ -1,205 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 - -use super::{ - super::statement::{GraphDirection, GraphProperties}, - tokenizer::Tok, -}; -use crate::error::SqlError; - -pub(super) fn find_keyword(toks: &[Tok<'_>], keyword: &str) -> Option { - toks.iter() - .position(|t| matches!(t, Tok::Word(w) if w.eq_ignore_ascii_case(keyword))) -} - -pub(super) fn quoted_after(toks: &[Tok<'_>], keyword: &str) -> Option { - let pos = find_keyword(toks, keyword)?; - match toks.get(pos + 1)? { - Tok::Quoted(s) => Some(s.clone().into_owned()), - Tok::Word(w) => Some((*w).to_string()), - Tok::Object(_) => None, - } -} - -/// Collect the run of quoted tokens at the head of `toks`. -/// -/// The run ends at the first unquoted token. The tokenizer drops `,`, `(` -/// and `)`, so `'a', 'b'` and `('a', 'b')` give the same run. -fn quoted_run(toks: &[Tok<'_>]) -> Vec { - toks.iter() - .map_while(|t| match t { - Tok::Quoted(s) => Some(s.clone().into_owned()), - _ => None, - }) - .collect() -} - -pub(super) fn quoted_list_after(toks: &[Tok<'_>], keyword: &str) -> Vec { - find_keyword(toks, keyword) - .map(|pos| quoted_run(&toks[pos + 1..])) - .unwrap_or_default() -} - -/// Read an optional `LABEL '