From 341733ed4e2dff39514761bd510b6bf1ce5fa751 Mon Sep 17 00:00:00 2001 From: EnRaiha <15997552+EnRaiha@users.noreply.github.com> Date: Fri, 4 Sep 2026 20:18:19 +0800 Subject: [PATCH 1/7] ci(lint): unblock Lint & Check gate (deny advisories, zerompk source, clippy) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - cargo update: h2 0.4.15 -> 0.4.19 (RUSTSEC-2026-0258 empty DATA frames), chacha20 0.10.1 -> 0.10.2 (yanked), wide 1.6.0 -> 1.7.0 (yanked), wasmtime 47.0.3 -> 47.0.4 (RUSTSEC-2026-0268/-0269) - deny.toml: ignore RUSTSEC-2026-0247/-0248/-0251 (lorо-internal's im/sized-chunks/bitmaps, archived upstream, no fixed version, review-by 2027-05-01); allow-git now matches Cargo.toml's nuskey8/zerompk (was farhan-syah — source-not-allowed for zerompk + zerompk_derive) - clippy: codebook_bytes.is_none_or (pq.rs); allow(dead_code) on wire harness start_with_failpoints - cargo deny check: advisories ok, bans ok, licenses ok, sources ok - clippy --workspace --all-targets --all-features -D warnings: 0 --- Cargo.lock | 120 ++++++++++++------------- deny.toml | 11 ++- nodedb-vector/src/quantize/pq.rs | 2 +- nodedb/tests/wire/harness/lifecycle.rs | 2 + 4 files changed, 72 insertions(+), 63 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3379b9b54..30e819bfa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1299,9 +1299,9 @@ checksum = "f079e83a288787bcd14a6aea84cee5c87a67c5a3e660c30f557a3d24761b3527" [[package]] name = "chacha20" -version = "0.10.1" +version = "0.10.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d524456ba66e72eb8b115ff89e01e497f8e6d11d78b70b1aa13c0fbd97540a81" +checksum = "65c35e4b699c7e15ccbe7ee35c005e4fc0a278d22238a2857e6ce2dadeda1b06" dependencies = [ "cfg-if", "cpufeatures 0.3.0", @@ -1519,27 +1519,27 @@ dependencies = [ [[package]] name = "cranelift-assembler-x64" -version = "0.134.3" +version = "0.134.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d552bd33b7a56dc70aeb1e1c960e51a218fa0db50f23873b500a310379450b2d" +checksum = "16c273ba1bfc3a1cb27cb4f83df27134f3cb638a61f5ed79ab22cf86eb13da0d" dependencies = [ "cranelift-assembler-x64-meta", ] [[package]] name = "cranelift-assembler-x64-meta" -version = "0.134.3" +version = "0.134.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "078e80e4c222279e3330f6aa1a256ca77ddf156d4453166a9f09defedc4594dd" +checksum = "cbf8909326a466739e83ffe11e74c0c8c6b4bfa911b612aece915daa746cff58" dependencies = [ "cranelift-srcgen", ] [[package]] name = "cranelift-bforest" -version = "0.134.3" +version = "0.134.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "820ce15d4ad3562d613c31a67b6d0434d403e7091a68d1349903842f7d31737e" +checksum = "3cb26b06b54d8b2f8cdf4d440a32fc3b3b91c71c162333a69a70a66fa265fc45" dependencies = [ "cranelift-entity", "wasmtime-internal-core", @@ -1547,9 +1547,9 @@ dependencies = [ [[package]] name = "cranelift-bitset" -version = "0.134.3" +version = "0.134.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "61bca563d4b86d285928d9e27f97f27039bb33a0fc524fa130d7d0c106bf8ab3" +checksum = "7ce4e57dbb7f73d08011808a19787c3a03c3e7854b2a479ed0b788915fb671be" dependencies = [ "serde", "serde_derive", @@ -1558,9 +1558,9 @@ dependencies = [ [[package]] name = "cranelift-codegen" -version = "0.134.3" +version = "0.134.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "709f4b7c0fb57d952658d5b5c07fbdc4149acd7b7f0de9678ae754b9c981949a" +checksum = "74b0a8968f33d0bc13bc86fe70dd3947e93acc9f358ed9848e9988448972a9be" dependencies = [ "bumpalo", "cranelift-assembler-x64", @@ -1589,9 +1589,9 @@ dependencies = [ [[package]] name = "cranelift-codegen-meta" -version = "0.134.3" +version = "0.134.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "903dc8915af62aad1d9d0f39a5968d33fa80b9aa899ed6f56e47f40ca4512e1e" +checksum = "f69e8e2fe9deb48ef7b8d0c61612f8c1e2277d0421c201e30a4c844a4ebac698" dependencies = [ "cranelift-assembler-x64-meta", "cranelift-codegen-shared", @@ -1602,24 +1602,24 @@ dependencies = [ [[package]] name = "cranelift-codegen-shared" -version = "0.134.3" +version = "0.134.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0522d74c227e49f3fd49ab055311486ae4f09083262b66705bed676952491469" +checksum = "ad4db1e87a65e9c5a832e3ede160830d4e34df938ac38b5604819f8825687e73" [[package]] name = "cranelift-control" -version = "0.134.3" +version = "0.134.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "66560ea1c5cef72e170b18e46d263dba2d3169c9d39e8cafdab2173c6362cc1a" +checksum = "9248cd37bb6ec460981f57ead368e32b1eca7a207ad50e3e9c7adc3b161f20e9" dependencies = [ "arbitrary", ] [[package]] name = "cranelift-entity" -version = "0.134.3" +version = "0.134.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b62ef5b17cc814d27e96b66a5b46da0e4ce2b8ac55a6d478048bb99d04b05526" +checksum = "26b8bc91c0a3530d132acbf118965dabe9a084029eb0fa74b64760360dec3316" dependencies = [ "cranelift-bitset", "serde", @@ -1629,9 +1629,9 @@ dependencies = [ [[package]] name = "cranelift-frontend" -version = "0.134.3" +version = "0.134.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a87e0aaa39dbf70693b348a221e45904111704ee8f9fef140498471005f9842d" +checksum = "5086f2ebc084a98b387e522680f62ef65a512444d8382151e03088716d3a1714" dependencies = [ "cranelift-codegen", "hashbrown 0.17.1", @@ -1642,15 +1642,15 @@ dependencies = [ [[package]] name = "cranelift-isle" -version = "0.134.3" +version = "0.134.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cf79003ebfa1eed5e87f3b84446ad5236f540268289960e14f387dc9b28e40b7" +checksum = "b6a658670f779afc2df083be201ebe54805f69b7b9ffa8dab04142b72ed751c9" [[package]] name = "cranelift-native" -version = "0.134.3" +version = "0.134.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "05bf4f235743c81e67ee4db617c5a4a0b65d58d3f0cfc575ee0f1a4e0cd58273" +checksum = "16a41083b1fd953debd6f32d21fcda765b258f323ce907fdd6b4715f2769c478" dependencies = [ "cranelift-codegen", "libc", @@ -1659,9 +1659,9 @@ dependencies = [ [[package]] name = "cranelift-srcgen" -version = "0.134.3" +version = "0.134.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f6977c2a71ab1e0d1e62f966b411a498aa04c4dce47d93d52f8a360a06058922" +checksum = "244d9cd0759b0a3ccdd587cafb13434f9ea43b5c9d07701d277daf5cd0f7b7c1" [[package]] name = "crash-context" @@ -2778,9 +2778,9 @@ dependencies = [ [[package]] name = "h2" -version = "0.4.15" +version = "0.4.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6cb093c84e8bd9b188d4c4a8cb6579fc016968d14c99882163cd3ff402a4f155" +checksum = "ef8e5e5a340588f4452631496976cf8636d4a7ecf600239fdc27615d2530bc16" dependencies = [ "atomic-waker", "bytes", @@ -5665,7 +5665,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b570b25f7617e43d59005d0990ccb79e950a423952cea19671b7a876da390adf" dependencies = [ "anyhow", - "itertools 0.14.0", + "itertools 0.11.0", "proc-macro2", "quote", "syn 2.0.119", @@ -5723,9 +5723,9 @@ dependencies = [ [[package]] name = "pulley-interpreter" -version = "47.0.3" +version = "47.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bc5c8c21ea032e4efbdf2d067dc45171779dbe0c8ecf20ef4a57efa7474f2b0a" +checksum = "ed2bd641cb6a3ec5bffe60405568933163e0ea619da0943e9d2a417ceaa7fe82" dependencies = [ "cranelift-bitset", "log", @@ -5735,9 +5735,9 @@ dependencies = [ [[package]] name = "pulley-macros" -version = "47.0.3" +version = "47.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9f10925455d5dde962e3604eade797ba5488644c8ee14a44191e2f2b58980574" +checksum = "d3fd4024d1e559eaf5579bc8ad84a2b5915f34b47acc13a06ffba7dac401b166" dependencies = [ "proc-macro2", "quote", @@ -6554,9 +6554,9 @@ checksum = "9774ba4a74de5f7b1c1451ed6cd5285a32eddb5cccb8cc655a4e50009e06477f" [[package]] name = "safe_arch" -version = "1.1.0" +version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3a52ec151f024d703f9fd65abb7cbe81e7cdb39f18917a3a37e3014470dc7c59" +checksum = "42c6efa15875e6ecb39ca61fb0b0c1a40b84fac5a5ffe71eef7d1000c8eb3f5f" dependencies = [ "bytemuck", ] @@ -7211,7 +7211,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" dependencies = [ "fastrand", - "getrandom 0.4.3", + "getrandom 0.3.4", "once_cell", "rustix", "windows-sys 0.61.2", @@ -8098,9 +8098,9 @@ dependencies = [ [[package]] name = "wasmtime" -version = "47.0.3" +version = "47.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c80ca6098e0d4d06886d91d7f2cc3cb6623eb583c4c0ab3c89cbfb6098c8586c" +checksum = "4cdaf5b8af5713146e3a62d940c71e3eb2ad1ebacfa6b55f0c8509a5b4b87718" dependencies = [ "addr2line", "async-trait", @@ -8136,9 +8136,9 @@ dependencies = [ [[package]] name = "wasmtime-environ" -version = "47.0.3" +version = "47.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "134f9d136d29c76f6c1b4c9b468e97a2efc38c7d49fd188e39b64870b2fea701" +checksum = "58793271eab7ec26bab99ca497131f8b37702e7dc39e2f2d18293e3e915b4964" dependencies = [ "anyhow", "cpp_demangle", @@ -8167,15 +8167,15 @@ dependencies = [ [[package]] name = "wasmtime-internal-component-util" -version = "47.0.3" +version = "47.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fc8a678149885cae00289f806fbe74c7863084cf74cace0b8dc73602279400e1" +checksum = "3d7032ff6b4d7a45e05be1cb32ec1756df5f2669f08296dbe2c61c3789dead2c" [[package]] name = "wasmtime-internal-core" -version = "47.0.3" +version = "47.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8a0092c4b9d070ac5e278b6d0db10f5e71214190f0347ca57502b4692f796321" +checksum = "f41d3b2cb9c3de7b696690af070d408b5e357011832f8b8bb7fbb315dbb620ed" dependencies = [ "hashbrown 0.17.1", "libm", @@ -8184,9 +8184,9 @@ dependencies = [ [[package]] name = "wasmtime-internal-cranelift" -version = "47.0.3" +version = "47.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6851ebc9e03cab23d9821d2b2505d380bdfe38f35ccec945247b05274d85c98b" +checksum = "1d5cda5b90247978dd2ba6ca4e309508da19e9244445671d2d0e6a34e5c3b847" dependencies = [ "cfg-if", "cranelift-codegen", @@ -8211,9 +8211,9 @@ dependencies = [ [[package]] name = "wasmtime-internal-fiber" -version = "47.0.3" +version = "47.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9b26da6d5f60d4c438da70bba3553fe810a840533a64156be287dca2081c6991" +checksum = "4d126366176553e8f304e8ac2078a4cc37134fe8f19d4d4695a09c5a2f577f10" dependencies = [ "cc", "cfg-if", @@ -8226,9 +8226,9 @@ dependencies = [ [[package]] name = "wasmtime-internal-jit-debug" -version = "47.0.3" +version = "47.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ed621ba25d7bf78b7edd7b4749abbb27c2e5cdba836c9504a424ee74b6c23149" +checksum = "c1d28ff131bc40e11d84bc3a9fa0d264506439ffbbc89f3562fbb529d1caa617" dependencies = [ "cc", "wasmtime-internal-versioned-export-macros", @@ -8236,9 +8236,9 @@ dependencies = [ [[package]] name = "wasmtime-internal-jit-icache-coherence" -version = "47.0.3" +version = "47.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5684ba160951baad06a725696f3c590e2fb0e8067c2aebee27bf7f9259058e85" +checksum = "4a52de8b6afbfcd618073f592c1450cb661574e7061f3f4a44f1f7ec0c7f7909" dependencies = [ "cfg-if", "libc", @@ -8248,9 +8248,9 @@ dependencies = [ [[package]] name = "wasmtime-internal-unwinder" -version = "47.0.3" +version = "47.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "112eead527bffa8ff0646a11fb4339a9d52ddd1da2b9a6fce4aff84c815dd94f" +checksum = "5524fbaaf3d2134dd6229164d7fc81e443cce06d1e0576524d5f4e86e747cb13" dependencies = [ "cfg-if", "cranelift-codegen", @@ -8261,9 +8261,9 @@ dependencies = [ [[package]] name = "wasmtime-internal-versioned-export-macros" -version = "47.0.3" +version = "47.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "153592e0bed824fc13c6696203fa1bd7bd20eb355316475e39a55f081ef80eca" +checksum = "e93bb9ae112a80618510f938275a5d88637b656e7d0a021fc906f52206c708ad" dependencies = [ "proc-macro2", "quote", @@ -8323,9 +8323,9 @@ dependencies = [ [[package]] name = "wide" -version = "1.6.0" +version = "1.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "be99e8317aa9f08e7d16e13033ca43faab59ab582b1d0feab7b385424c42f8b1" +checksum = "8cf05ca94c9fba0c51316899caa1d1e74a064fc310a5efa7375e614343d415be" dependencies = [ "bytemuck", "safe_arch", diff --git a/deny.toml b/deny.toml index d20aeabc8..fc0db1fb1 100644 --- a/deny.toml +++ b/deny.toml @@ -36,6 +36,13 @@ ignore = [ "RUSTSEC-2023-0089", # atomic-polyfill; review-by: 2027-01-31 "RUSTSEC-2021-0153", # encoding; review-by: 2027-01-31 "RUSTSEC-2024-0436", # paste; review-by: 2027-01-31 + # loro-internal's transitive persistence codecs (im 15.1.0, sized-chunks, + # bitmaps) are archived upstream as of 2026-05-03. No fixed version + # exists; they carry no known security impact and are pinned behind the + # loro CRDT engine. Revisit on each loro upgrade. + "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 ] [licenses] @@ -75,9 +82,9 @@ wildcards = "deny" unknown-registry = "deny" unknown-git = "deny" allow-registry = ["https://github.com/rust-lang/crates.io-index"] -# zerompk is pinned to our own fork at an exact rev while the derived codecs' +# zerompk is pinned to a fork at an exact rev while the derived codecs' # `inline(always)` fix is unreleased upstream — it folds whole codec trees into # one function and makes large plan enums cost minutes to compile. Drop this # entry together with the git source in Cargo.toml once the fix ships to # crates.io. -allow-git = ["https://github.com/farhan-syah/zerompk"] +allow-git = ["https://github.com/nuskey8/zerompk"] diff --git a/nodedb-vector/src/quantize/pq.rs b/nodedb-vector/src/quantize/pq.rs index 5eb39161e..028bb0ed0 100644 --- a/nodedb-vector/src/quantize/pq.rs +++ b/nodedb-vector/src/quantize/pq.rs @@ -331,7 +331,7 @@ impl PqCodec { book.len() == self.k && book.iter().all(|centroid| centroid.len() == self.sub_dim) }); if !valid_scalar_shape - || !codebook_bytes.is_some_and(|bytes| bytes <= MAX_PQ_CODEBOOK_BYTES) + || codebook_bytes.is_none_or(|bytes| bytes > MAX_PQ_CODEBOOK_BYTES) || !valid_codebooks { return Err(VectorError::DeserializationFailed( diff --git a/nodedb/tests/wire/harness/lifecycle.rs b/nodedb/tests/wire/harness/lifecycle.rs index ef148c944..952b91265 100644 --- a/nodedb/tests/wire/harness/lifecycle.rs +++ b/nodedb/tests/wire/harness/lifecycle.rs @@ -48,6 +48,8 @@ impl TestServer { /// `nodedb` binary under test was itself built with `--features /// failpoints` — a plain build ignores the variable and the fail point /// never fires, since `fail_point_err!` compiles to nothing without it. + // Kept for failpoint injection tests; not every harness build reaches it. + #[allow(dead_code)] pub async fn start_with_failpoints(spec: &str) -> Self { let dir = tempfile::tempdir().expect("tempdir"); let spawned = process::spawn_with_failpoints( From 3cea04dffb59a164a58c200a2d880a5a4a64e4d2 Mon Sep 17 00:00:00 2001 From: EnRaiha <15997552+EnRaiha@users.noreply.github.com> Date: Fri, 4 Sep 2026 23:55:02 +0800 Subject: [PATCH 2/7] ci(lint): marker no-determinism for test-only dummy deadline check_calvin_determinism.sh flagged sub_plan.rs:122 (Instant::now in build_dummy_task_at). The existing no-determinism comment sat in the doc block six lines above; the gate requires same-line or directly preceding. Test-only builder; deadline never reaches Calvin state. --- nodedb/src/data/executor/handlers/transaction/sub_plan.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/nodedb/src/data/executor/handlers/transaction/sub_plan.rs b/nodedb/src/data/executor/handlers/transaction/sub_plan.rs index d424e3309..3a7e8a2e7 100644 --- a/nodedb/src/data/executor/handlers/transaction/sub_plan.rs +++ b/nodedb/src/data/executor/handlers/transaction/sub_plan.rs @@ -119,6 +119,7 @@ impl CoreLoop { tid, DatabaseId::DEFAULT, crate::types::VShardId::new(0), + // no-determinism: test-only dummy deadline, not written to Calvin state std::time::Instant::now() + std::time::Duration::from_secs(60), ) } From 3cde61355721fb78a59eb984e881489b7715269c Mon Sep 17 00:00:00 2001 From: EnRaiha <15997552+EnRaiha@users.noreply.github.com> Date: Fri, 4 Sep 2026 11:21:43 +0800 Subject: [PATCH 3/7] fix(sql,vector): reject negative LIMIT, bound ANALYZE name, unify oversample - S1: LIMIT/OFFSET negative literals now error with InvalidLimitValue (SQLSTATE 2201W class) instead of silently degrading to unbounded; new fallible expr_as_nonnegative_usize in coerce.rs, limit_offset returns Result, validated downstream in select_stmt/limit apply. - S2: ANALYZE/COMPACT keyword-boundary matched; trailing parenthesised payload (ANALYZE users(id)) no longer absorbed into the collection name; both now strip ( ... ) suffixes. - V3: cost planner rerank oversample now reads rerank::codecs::bbq::DEFAULT_OVERSAMPLE (single source of truth) instead of a local divergent const (3 vs 4). Regression: 861 nodedb-sql lib tests + nodedb-vector cost tests green; repro tests s1/s2/v3 flip red->green. --- nodedb-sql/src/coerce.rs | 66 +++++++++++++++++++- nodedb-sql/src/ddl_ast/parse/maintenance.rs | 59 ++++++++++++++++- nodedb-sql/src/error.rs | 7 +++ nodedb-sql/src/planner/select/limit.rs | 2 +- nodedb-sql/src/planner/select/query_tail.rs | 25 +++++--- nodedb-sql/src/planner/select/select_stmt.rs | 2 +- nodedb-vector/src/planner/cost.rs | 22 ++++++- 7 files changed, 167 insertions(+), 16 deletions(-) diff --git a/nodedb-sql/src/coerce.rs b/nodedb-sql/src/coerce.rs index f0b6792ab..0997be528 100644 --- a/nodedb-sql/src/coerce.rs +++ b/nodedb-sql/src/coerce.rs @@ -21,6 +21,8 @@ use sqlparser::ast; +use crate::error::SqlError; + /// Resolve a `Value` into a `usize` if numeric-shaped. /// /// Accepts: @@ -63,6 +65,30 @@ pub fn expr_as_usize_literal(expr: &ast::Expr) -> Option { } } +/// Fallible LIMIT/OFFSET resolution. +/// +/// Rejects negative literals (`LIMIT -1`, `OFFSET -2`) with +/// [`SqlError::InvalidLimitValue`] instead of collapsing to `None` +/// (unbounded). Everything else behaves exactly like +/// [`expr_as_usize_literal`]. +pub fn expr_as_nonnegative_usize( + expr: &ast::Expr, + clause: &'static str, +) -> Result, SqlError> { + if matches!( + expr, + ast::Expr::UnaryOp { + op: ast::UnaryOperator::Minus, + .. + } + ) { + return Err(SqlError::InvalidLimitValue { + detail: format!("{clause} must not be negative"), + }); + } + Ok(expr_as_usize_literal(expr)) +} + /// Resolve a `Value` into an `f64` if numeric-shaped. /// /// Same UNKNOWN-coercion behavior as `as_usize_literal` but for @@ -216,4 +242,42 @@ mod tests { None ); } -} + + #[test] + fn s1_negative_limit_rejected_not_unbounded() { + let stmts = sqlparser::parser::Parser::parse_sql( + &sqlparser::dialect::GenericDialect {}, + "SELECT * FROM t LIMIT -1", + ) + .unwrap(); + let ast::Statement::Query(q) = &stmts[0] else { + panic!("expected query statement"); + }; + let Some(ast::LimitClause::LimitOffset { limit: Some(expr), .. }) = + q.limit_clause.as_ref() + else { + panic!("expected LIMIT clause"); + }; + assert!( + matches!( + expr_as_nonnegative_usize(expr, "LIMIT"), + Err(SqlError::InvalidLimitValue { .. }) + ), + "S1: negative LIMIT must error" + ); + let stmts = sqlparser::parser::Parser::parse_sql( + &sqlparser::dialect::GenericDialect {}, + "SELECT * FROM t LIMIT 10", + ) + .unwrap(); + let ast::Statement::Query(q) = &stmts[0] else { + panic!("expected query statement"); + }; + let Some(ast::LimitClause::LimitOffset { limit: Some(expr), .. }) = + q.limit_clause.as_ref() + else { + panic!("expected LIMIT clause"); + }; + assert_eq!(expr_as_nonnegative_usize(expr, "LIMIT").unwrap(), Some(10)); + } +} \ No newline at end of file diff --git a/nodedb-sql/src/ddl_ast/parse/maintenance.rs b/nodedb-sql/src/ddl_ast/parse/maintenance.rs index bde15373e..99a64ad24 100644 --- a/nodedb-sql/src/ddl_ast/parse/maintenance.rs +++ b/nodedb-sql/src/ddl_ast/parse/maintenance.rs @@ -11,14 +11,29 @@ pub(super) fn try_parse( _trimmed: &str, ) -> Option> { (|| -> Option { - if upper.starts_with("ANALYZE") { - let collection = parts.get(1).map(|s| s.to_string()); + // `ANALYZE`/`COMPACT` are keyword-boundary matched: bare `ANALYZE` + // (whole statement, no target) or `ANALYZE `. A bare prefix + // match would absorb statements like `ANALYZE users(id)` — actually + // `ANALYZE` followed by a parenthesised payload — into the collection + // name. PostgreSQL has no `ANALYZE ... (cols)` form; any trailing + // `(...)` after the collection name is stripped so the name stays + // clean. + if upper == "ANALYZE" || upper.starts_with("ANALYZE ") { + let collection = parts + .get(1) + .map(|s| s.split('(').next().unwrap_or(s).trim().to_string()); return Some(NodedbStatement::Cluster(ClusterStmt::Analyze { collection, })); } if upper.starts_with("COMPACT ") { - let collection = parts.get(1)?.to_string(); + let collection = parts + .get(1)? + .split('(') + .next() + .unwrap_or(parts.get(1)?) + .trim() + .to_string(); return Some(NodedbStatement::Cluster(ClusterStmt::Compact { collection, })); @@ -36,3 +51,41 @@ pub(super) fn try_parse( })() .map(Ok) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn s2_analyze_parenthesized_payload_absorbs_token() { + // CLAIM S2: starts_with("ANALYZE") + raw parts[1] absorbs "(id)". + let sql = "ANALYZE users(id)"; + let out = try_parse( + sql.to_uppercase().as_str(), + &sql.split_whitespace().collect::>(), + sql, + ); + let Some(Ok(NodedbStatement::Cluster(ClusterStmt::Analyze { collection }))) = out else { + panic!("expected Analyze statement, got {out:?}"); + }; + assert_eq!( + collection.as_deref(), + Some("users"), + "S2: ANALYZE users(id) must yield collection 'users', not 'users(id)'" + ); + } + + #[test] + fn s2_plain_analyze_parses_cleanly() { + let sql = "ANALYZE users"; + let out = try_parse( + sql.to_uppercase().as_str(), + &sql.split_whitespace().collect::>(), + sql, + ); + let Some(Ok(NodedbStatement::Cluster(ClusterStmt::Analyze { collection }))) = out else { + panic!("expected Analyze statement, got {out:?}"); + }; + assert_eq!(collection.as_deref(), Some("users")); + } +} diff --git a/nodedb-sql/src/error.rs b/nodedb-sql/src/error.rs index e228fbdd4..ead10b5f5 100644 --- a/nodedb-sql/src/error.rs +++ b/nodedb-sql/src/error.rs @@ -119,6 +119,13 @@ pub enum SqlError { #[error("retryable schema change on {descriptor}")] RetryableSchemaChanged { descriptor: String }, + /// A LIMIT or OFFSET clause carried a negative literal, which Postgres + /// rejects as SQLSTATE `2201W` (invalid_limit_value) rather than treating + /// as unbounded. Without the rejection the planner silently drops the + /// bound and runs the full collection scan. + #[error("invalid limit value: {detail}")] + InvalidLimitValue { detail: String }, + /// Identifier violates NodeDB's canonical identifier rules. #[error("invalid identifier '{name}': {reason}")] InvalidIdentifier { name: String, reason: &'static str }, diff --git a/nodedb-sql/src/planner/select/limit.rs b/nodedb-sql/src/planner/select/limit.rs index fbd697e2d..30541137c 100644 --- a/nodedb-sql/src/planner/select/limit.rs +++ b/nodedb-sql/src/planner/select/limit.rs @@ -27,7 +27,7 @@ pub(in crate::planner::select) fn apply_limit( mut plan: SqlPlan, tail: &QueryTail<'_>, ) -> Result { - let (limit_val, offset_val) = tail.limit_offset(); + let (limit_val, offset_val) = tail.limit_offset()?; // The LIMIT belongs to the query reading the CTE, not to the CTE body, so // it lands on the outer plan — without this a derived table like diff --git a/nodedb-sql/src/planner/select/query_tail.rs b/nodedb-sql/src/planner/select/query_tail.rs index 6c34ed993..200eda924 100644 --- a/nodedb-sql/src/planner/select/query_tail.rs +++ b/nodedb-sql/src/planner/select/query_tail.rs @@ -12,6 +12,8 @@ use sqlparser::ast; +use crate::error::SqlError; + use crate::error::Result; use crate::types::SortKey; @@ -41,24 +43,33 @@ impl QueryTail<'_> { /// `(limit, offset)` for the LIMIT clause. A missing clause is /// `(None, 0)`; a non-literal bound is `None` (unbounded). - pub(in crate::planner::select) fn limit_offset(&self) -> (Option, usize) { - match self.limit_clause { + /// + /// Negative literals are rejected with [`SqlError::InvalidLimitValue`] + /// (SQLSTATE 2201W class) instead of silently degrading to unbounded. + pub(in crate::planner::select) fn limit_offset( + &self, + ) -> std::result::Result<(Option, usize), SqlError> { + Ok(match self.limit_clause { None => (None, 0), Some(ast::LimitClause::LimitOffset { limit, offset, .. }) => { let lv = limit .as_ref() - .and_then(crate::coerce::expr_as_usize_literal); + .map(|l| crate::coerce::expr_as_nonnegative_usize(l, "LIMIT")) + .transpose()? + .flatten(); let ov = offset .as_ref() - .and_then(|o| crate::coerce::expr_as_usize_literal(&o.value)) + .map(|o| crate::coerce::expr_as_nonnegative_usize(&o.value, "OFFSET")) + .transpose()? + .flatten() .unwrap_or(0); (lv, ov) } Some(ast::LimitClause::OffsetCommaLimit { offset, limit }) => { - let lv = crate::coerce::expr_as_usize_literal(limit); - let ov = crate::coerce::expr_as_usize_literal(offset).unwrap_or(0); + let lv = crate::coerce::expr_as_nonnegative_usize(limit, "LIMIT")?; + let ov = crate::coerce::expr_as_nonnegative_usize(offset, "OFFSET")?.unwrap_or(0); (lv, ov) } - } + }) } } diff --git a/nodedb-sql/src/planner/select/select_stmt.rs b/nodedb-sql/src/planner/select/select_stmt.rs index 1a08036d8..5e0f00335 100644 --- a/nodedb-sql/src/planner/select/select_stmt.rs +++ b/nodedb-sql/src/planner/select/select_stmt.rs @@ -288,7 +288,7 @@ pub(super) fn plan_select( // answer the query from the wrong rows. Those clauses land on the join // itself downstream — the same reason `scan_projection` is empty here. let (sort_keys, limit, offset) = if subquery_joins.is_empty() { - let (limit, offset) = tail.limit_offset(); + let (limit, offset) = tail.limit_offset()?; (tail.sort_keys()?, limit, offset) } else { (Vec::new(), None, 0) diff --git a/nodedb-vector/src/planner/cost.rs b/nodedb-vector/src/planner/cost.rs index 8e58a54b4..3ed0da444 100644 --- a/nodedb-vector/src/planner/cost.rs +++ b/nodedb-vector/src/planner/cost.rs @@ -125,12 +125,15 @@ pub fn estimate_cost(inputs: &CostModelInputs) -> VectorCost { // Codec decode cost (ns per vector × ef_search vectors). let codec_decode_ns = codec_decode_ns_for(inputs.quantization); - // Rerank cost. - const DEFAULT_OVERSAMPLE: u8 = 3; + // Rerank cost. Single source of truth: the codec's oversample default + // (BBQ/rabitq training multiplier) — cost planning must not drift from + // the codec the collection actually built with. let rerank_us = if inputs.quantization == QuantizationKind::None { 0.0 } else { - DEFAULT_OVERSAMPLE as f32 * inputs.ef_search as f32 * 0.01 + crate::rerank::codecs::bbq::DEFAULT_OVERSAMPLE as f32 + * inputs.ef_search as f32 + * 0.01 }; let predicted_recall = predicted_recall_for(inputs.index_type, inputs.quantization); @@ -148,6 +151,19 @@ pub fn estimate_cost(inputs: &CostModelInputs) -> VectorCost { #[cfg(test)] mod tests { use super::*; + use crate::rerank::codecs::bbq::DEFAULT_OVERSAMPLE as BBQ_OVERSAMPLE; + + #[test] + fn v3_oversample_constant_matches_bbq() { + // CLAIM V3: cost planner local const is 3; BBQ codec default is 4. + let inputs = base_inputs(); + let ratio = estimate_cost(&inputs).rerank_us / (inputs.ef_search as f32 * 0.01); + assert!( + (ratio - BBQ_OVERSAMPLE as f32).abs() < 1e-3, + "V3: rerank oversample ratio {ratio} must equal BBQ DEFAULT_OVERSAMPLE ({BBQ_OVERSAMPLE})" + ); + } + fn base_inputs() -> CostModelInputs { CostModelInputs { From 7748e89b6226ee15d782e3b7a9dc72bc0930a2fb Mon Sep 17 00:00:00 2001 From: EnRaiha <15997552+EnRaiha@users.noreply.github.com> Date: Fri, 4 Sep 2026 12:21:27 +0800 Subject: [PATCH 4/7] =?UTF-8?q?fix(sql):=20address=20copilot=20review=20?= =?UTF-8?q?=E2=80=94=20negative=20UNKNOWN=20param,=20whitespace=20boundary?= =?UTF-8?q?,=202201W=20doc?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - coerce: reject SingleQuotedString/Number '-...' in addition to UnaryOp Minus - maintenance: use parts[0] eq_ignore_ascii_case for ANALYZE/COMPACT boundary - error: clarify InvalidLimitValue pgwire mapping (PlanError 42601 fallback, explicit 2201W needs control-plane map) --- nodedb-sql/src/coerce.rs | 12 ++++++++++++ nodedb-sql/src/ddl_ast/parse/maintenance.rs | 4 ++-- nodedb-sql/src/error.rs | 12 ++++++++---- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/nodedb-sql/src/coerce.rs b/nodedb-sql/src/coerce.rs index 0997be528..4cefc7e8c 100644 --- a/nodedb-sql/src/coerce.rs +++ b/nodedb-sql/src/coerce.rs @@ -86,6 +86,18 @@ pub fn expr_as_nonnegative_usize( detail: format!("{clause} must not be negative"), }); } + if let ast::Expr::Value(v) = expr { + let is_negative = match &v.value { + ast::Value::SingleQuotedString(s) => s.trim_start().starts_with('-'), + ast::Value::Number(n, _) => n.trim_start().starts_with('-'), + _ => false, + }; + if is_negative { + return Err(SqlError::InvalidLimitValue { + detail: format!("{clause} must not be negative"), + }); + } + } Ok(expr_as_usize_literal(expr)) } diff --git a/nodedb-sql/src/ddl_ast/parse/maintenance.rs b/nodedb-sql/src/ddl_ast/parse/maintenance.rs index 99a64ad24..da8856769 100644 --- a/nodedb-sql/src/ddl_ast/parse/maintenance.rs +++ b/nodedb-sql/src/ddl_ast/parse/maintenance.rs @@ -18,7 +18,7 @@ pub(super) fn try_parse( // name. PostgreSQL has no `ANALYZE ... (cols)` form; any trailing // `(...)` after the collection name is stripped so the name stays // clean. - if upper == "ANALYZE" || upper.starts_with("ANALYZE ") { + if parts.first().is_some_and(|p| p.eq_ignore_ascii_case("ANALYZE")) { let collection = parts .get(1) .map(|s| s.split('(').next().unwrap_or(s).trim().to_string()); @@ -26,7 +26,7 @@ pub(super) fn try_parse( collection, })); } - if upper.starts_with("COMPACT ") { + if parts.first().is_some_and(|p| p.eq_ignore_ascii_case("COMPACT")) { let collection = parts .get(1)? .split('(') diff --git a/nodedb-sql/src/error.rs b/nodedb-sql/src/error.rs index ead10b5f5..59a0195c6 100644 --- a/nodedb-sql/src/error.rs +++ b/nodedb-sql/src/error.rs @@ -119,10 +119,14 @@ pub enum SqlError { #[error("retryable schema change on {descriptor}")] RetryableSchemaChanged { descriptor: String }, - /// A LIMIT or OFFSET clause carried a negative literal, which Postgres - /// rejects as SQLSTATE `2201W` (invalid_limit_value) rather than treating - /// as unbounded. Without the rejection the planner silently drops the - /// bound and runs the full collection scan. + /// A LIMIT or OFFSET clause carried a negative literal. + /// + /// Postgres rejects this as SQLSTATE `2201W` (invalid_limit_value). + /// NodeDB surfaces this as a planner error that the pgwire layer maps + /// to `2201W` via an explicit `Error::InvalidLimitValue` conversion + /// (see `control/planner`); unmapped variants fall back to `PlanError` + /// (`42601`). The rejection prevents the planner from silently dropping + /// the bound and running a full collection scan. #[error("invalid limit value: {detail}")] InvalidLimitValue { detail: String }, From 8668f9493a05798ae025a3209492b34232324b05 Mon Sep 17 00:00:00 2001 From: EnRaiha <15997552+EnRaiha@users.noreply.github.com> Date: Fri, 4 Sep 2026 13:07:49 +0800 Subject: [PATCH 5/7] fix(sql): full negative-limit rejection incl. UNKNOWN param + wire SQLSTATE 2201W MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review follow-up (drill): - coerce: literal-only negative rejection across all forms — LIMIT -5, LIMIT '-1', LIMIT - '-1' error; LIMIT -$1 (non-literal) keeps documented unbounded semantics - maintenance: ANALYZE/COMPACT boundary via parts[0] (tab-safe) - error: InvalidLimitValue now carried to pgwire as SQLSTATE 2201W via Error::InvalidLimitValue + error_map arm + classify arm; sqlstate const INVALID_LIMIT_VALUE added --- nodedb-sql/src/coerce.rs | 91 ++++++++++++++++--- nodedb-sql/src/ddl_ast/parse/maintenance.rs | 28 ++++++ nodedb-types/src/error/sqlstate.rs | 6 ++ .../control/planner/context/query/planning.rs | 3 + .../control/server/pgwire/types/error_map.rs | 3 + nodedb/src/error/types.rs | 5 + nodedb/src/error_classify.rs | 3 + 7 files changed, 124 insertions(+), 15 deletions(-) diff --git a/nodedb-sql/src/coerce.rs b/nodedb-sql/src/coerce.rs index 4cefc7e8c..65b721815 100644 --- a/nodedb-sql/src/coerce.rs +++ b/nodedb-sql/src/coerce.rs @@ -71,28 +71,53 @@ pub fn expr_as_usize_literal(expr: &ast::Expr) -> Option { /// [`SqlError::InvalidLimitValue`] instead of collapsing to `None` /// (unbounded). Everything else behaves exactly like /// [`expr_as_usize_literal`]. +/// Fallible LIMIT/OFFSET resolution. +/// +/// Rejects statically-negative bounds with [`SqlError::InvalidLimitValue`]: +/// - `LIMIT -1` / `OFFSET -2` (`Expr::UnaryOp(Minus)` over a numeric literal) +/// - `LIMIT '-1'` (UNKNOWN-param / string literal carrying a minus sign) +/// - `LIMIT - '-1'` (unary minus over a string literal) +/// +/// Non-literal operands (`LIMIT -$1`, `LIMIT -col`) stay uncoerced: their +/// sign is unknowable at plan time, so they keep the documented pre-existing +/// behavior of any non-literal bound (`None` = no static limit). This matches +/// the docstring contract of [`expr_as_usize_literal`]: only literals are +/// resolved; everything else is the caller's semantic. pub fn expr_as_nonnegative_usize( expr: &ast::Expr, clause: &'static str, ) -> Result, SqlError> { - if matches!( - expr, - ast::Expr::UnaryOp { - op: ast::UnaryOperator::Minus, - .. + let reject_negative = |v: &ast::Value| match v { + ast::Value::SingleQuotedString(s) => s.trim_start().starts_with('-'), + ast::Value::Number(n, _) => n.trim_start().starts_with('-'), + _ => false, + }; + + if let ast::Expr::UnaryOp { + op: ast::UnaryOperator::Minus, + expr: inner, + } = expr + { + // Unary minus over ANY literal operand is statically negative + // (-5, -'5', - '-5'): the operand's own sign is irrelevant, the + // negation is. Reject all literal operands... + if matches!( + inner.as_ref(), + ast::Expr::Value(ast::ValueWithSpan { value: ast::Value::Number(..), .. }) + | ast::Expr::Value(ast::ValueWithSpan { + value: ast::Value::SingleQuotedString(..), + .. + }) + ) { + return Err(SqlError::InvalidLimitValue { + detail: format!("{clause} must not be negative"), + }); } - ) { - return Err(SqlError::InvalidLimitValue { - detail: format!("{clause} must not be negative"), - }); + // Non-literal operand ($1, col): sign unknowable at plan time → + // unchanged "non-literal bound" semantics (None → unbounded). } if let ast::Expr::Value(v) = expr { - let is_negative = match &v.value { - ast::Value::SingleQuotedString(s) => s.trim_start().starts_with('-'), - ast::Value::Number(n, _) => n.trim_start().starts_with('-'), - _ => false, - }; - if is_negative { + if reject_negative(&v.value) { return Err(SqlError::InvalidLimitValue { detail: format!("{clause} must not be negative"), }); @@ -291,5 +316,41 @@ mod tests { panic!("expected LIMIT clause"); }; assert_eq!(expr_as_nonnegative_usize(expr, "LIMIT").unwrap(), Some(10)); + + // UNKNOWN-param negative text must also be rejected, not unbounded. + assert!(matches!( + expr_as_nonnegative_usize( + &ast::Expr::Value(ast::ValueWithSpan::from(ast::Value::SingleQuotedString( + "-1".into() + ))), + "LIMIT" + ), + Err(SqlError::InvalidLimitValue { .. }) + )); + // Unary minus over a string literal. + assert!(matches!( + expr_as_nonnegative_usize( + &ast::Expr::UnaryOp { + op: ast::UnaryOperator::Minus, + expr: Box::new(ast::Expr::Value(ast::ValueWithSpan::from( + ast::Value::SingleQuotedString("1".into()) + ))), + }, + "LIMIT" + ), + Err(SqlError::InvalidLimitValue { .. }) + )); + // Non-literal operand: sign unknowable at plan time → unchanged None. + assert_eq!( + expr_as_nonnegative_usize( + &ast::Expr::UnaryOp { + op: ast::UnaryOperator::Minus, + expr: Box::new(ast::Expr::Identifier(ast::Ident::new("n"))), + }, + "LIMIT" + ) + .unwrap(), + None + ); } } \ No newline at end of file diff --git a/nodedb-sql/src/ddl_ast/parse/maintenance.rs b/nodedb-sql/src/ddl_ast/parse/maintenance.rs index da8856769..191de6d85 100644 --- a/nodedb-sql/src/ddl_ast/parse/maintenance.rs +++ b/nodedb-sql/src/ddl_ast/parse/maintenance.rs @@ -75,6 +75,34 @@ mod tests { ); } + #[test] + fn s2_analyze_tab_whitespace_is_accepted() { + let sql = "ANALYZE\tusers(id)"; + let out = try_parse( + &sql.to_uppercase().replace('\t', " "), + &sql.split_whitespace().collect::>(), + sql, + ); + let Some(Ok(NodedbStatement::Cluster(ClusterStmt::Analyze { collection }))) = out else { + panic!("expected Analyze statement, got {out:?}"); + }; + assert_eq!(collection.as_deref(), Some("users")); + } + + #[test] + fn s2_bare_analyze_has_no_collection() { + let sql = "ANALYZE"; + let out = try_parse( + sql.to_uppercase().as_str(), + &sql.split_whitespace().collect::>(), + sql, + ); + let Some(Ok(NodedbStatement::Cluster(ClusterStmt::Analyze { collection }))) = out else { + panic!("expected Analyze statement, got {out:?}"); + }; + assert_eq!(collection, None); + } + #[test] fn s2_plain_analyze_parses_cleanly() { let sql = "ANALYZE users"; diff --git a/nodedb-types/src/error/sqlstate.rs b/nodedb-types/src/error/sqlstate.rs index d5a5d6742..b3607ebf1 100644 --- a/nodedb-types/src/error/sqlstate.rs +++ b/nodedb-types/src/error/sqlstate.rs @@ -54,6 +54,12 @@ pub const NUMERIC_VALUE_OUT_OF_RANGE: &str = "22003"; /// `22012` — `division_by_zero` (`/` or `%` with a zero divisor — /// raised at runtime instead of evaluating to `NULL`) +/// `2201W` — `invalid_limit_value` — a negative LIMIT/OFFSET bound (or a +/// bound that cannot be coerced non-negative) was rejected at plan time. +/// Mirrors PostgreSQL's `2201W`; without it the planner would silently +/// degrade the bound to "unbounded" and scan the full collection. +pub const INVALID_LIMIT_VALUE: &str = "2201W"; + pub const DIVISION_BY_ZERO: &str = "22012"; /// `22023` — `invalid_parameter_value` (a `SET` value the parameter's own diff --git a/nodedb/src/control/planner/context/query/planning.rs b/nodedb/src/control/planner/context/query/planning.rs index 58ad40e91..eff48eb6e 100644 --- a/nodedb/src/control/planner/context/query/planning.rs +++ b/nodedb/src/control/planner/context/query/planning.rs @@ -45,6 +45,9 @@ fn map_plan_error(error: nodedb_sql::SqlError, tenant_id: crate::types::TenantId // A constant expression that divides by zero is the same condition the // row-scope evaluator raises, so it carries the same code. nodedb_sql::SqlError::DivisionByZero => crate::Error::DivisionByZero, + nodedb_sql::SqlError::InvalidLimitValue { detail } => { + crate::Error::InvalidLimitValue { detail } + } other => crate::Error::PlanError { detail: other.to_string(), }, diff --git a/nodedb/src/control/server/pgwire/types/error_map.rs b/nodedb/src/control/server/pgwire/types/error_map.rs index 22fd3f070..a04935469 100644 --- a/nodedb/src/control/server/pgwire/types/error_map.rs +++ b/nodedb/src/control/server/pgwire/types/error_map.rs @@ -51,6 +51,9 @@ pub fn error_to_sqlstate(err: &crate::Error) -> (&'static str, &'static str, Str format!("function {name}(...) does not exist"), ), crate::Error::DivisionByZero => ("ERROR", sqlstate::DIVISION_BY_ZERO, err.to_string()), + crate::Error::InvalidLimitValue { detail } => { + ("ERROR", sqlstate::INVALID_LIMIT_VALUE, detail.clone()) + } crate::Error::DocumentNotFound { collection, document_id, diff --git a/nodedb/src/error/types.rs b/nodedb/src/error/types.rs index 179e1ff4c..304ae7cce 100644 --- a/nodedb/src/error/types.rs +++ b/nodedb/src/error/types.rs @@ -287,6 +287,11 @@ pub enum Error { #[error("division by zero")] DivisionByZero, + /// A LIMIT/OFFSET bound was rejected at plan time (negative literal). + /// Rendered as SQLSTATE `2201W` (invalid_limit_value) at the pgwire layer. + #[error("invalid limit value: {detail}")] + InvalidLimitValue { detail: String }, + /// Descriptor lease conflict; pgwire retries within `PLAN_RETRY_BUDGET`. #[error("retryable schema change on {descriptor}")] RetryableSchemaChanged { descriptor: String }, diff --git a/nodedb/src/error_classify.rs b/nodedb/src/error_classify.rs index ed7b373d3..396fd52d7 100644 --- a/nodedb/src/error_classify.rs +++ b/nodedb/src/error_classify.rs @@ -149,6 +149,9 @@ pub(crate) fn classify(e: &Error) -> NodeDbError { Error::PlanError { detail } => NodeDbError::plan_error(detail), Error::UndefinedFunction { name } => NodeDbError::undefined_function(name.clone()), Error::DivisionByZero => NodeDbError::division_by_zero(), + // A negative/uncoercible LIMIT/OFFSET bound rejected at plan time + // (SQLSTATE 2201W). User error, not retryable. + Error::InvalidLimitValue { detail } => NodeDbError::bad_request(detail.clone()), Error::RetryableSchemaChanged { descriptor } => { NodeDbError::plan_error(format!("retryable schema change on {descriptor}")) } From 5c5f9c4aff3f37bb84b339eacd029f07b9aff3e9 Mon Sep 17 00:00:00 2001 From: EnRaiha <15997552+EnRaiha@users.noreply.github.com> Date: Fri, 4 Sep 2026 14:16:23 +0800 Subject: [PATCH 6/7] style(sql,vector): rustfmt (CI Lint & Check gate) --- nodedb-sql/src/coerce.rs | 24 ++++++++++++--------- nodedb-sql/src/ddl_ast/parse/maintenance.rs | 10 +++++++-- nodedb-vector/src/planner/cost.rs | 5 +---- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/nodedb-sql/src/coerce.rs b/nodedb-sql/src/coerce.rs index 65b721815..286834014 100644 --- a/nodedb-sql/src/coerce.rs +++ b/nodedb-sql/src/coerce.rs @@ -103,11 +103,13 @@ pub fn expr_as_nonnegative_usize( // negation is. Reject all literal operands... if matches!( inner.as_ref(), - ast::Expr::Value(ast::ValueWithSpan { value: ast::Value::Number(..), .. }) - | ast::Expr::Value(ast::ValueWithSpan { - value: ast::Value::SingleQuotedString(..), - .. - }) + ast::Expr::Value(ast::ValueWithSpan { + value: ast::Value::Number(..), + .. + }) | ast::Expr::Value(ast::ValueWithSpan { + value: ast::Value::SingleQuotedString(..), + .. + }) ) { return Err(SqlError::InvalidLimitValue { detail: format!("{clause} must not be negative"), @@ -290,8 +292,9 @@ mod tests { let ast::Statement::Query(q) = &stmts[0] else { panic!("expected query statement"); }; - let Some(ast::LimitClause::LimitOffset { limit: Some(expr), .. }) = - q.limit_clause.as_ref() + let Some(ast::LimitClause::LimitOffset { + limit: Some(expr), .. + }) = q.limit_clause.as_ref() else { panic!("expected LIMIT clause"); }; @@ -310,8 +313,9 @@ mod tests { let ast::Statement::Query(q) = &stmts[0] else { panic!("expected query statement"); }; - let Some(ast::LimitClause::LimitOffset { limit: Some(expr), .. }) = - q.limit_clause.as_ref() + let Some(ast::LimitClause::LimitOffset { + limit: Some(expr), .. + }) = q.limit_clause.as_ref() else { panic!("expected LIMIT clause"); }; @@ -353,4 +357,4 @@ mod tests { None ); } -} \ No newline at end of file +} diff --git a/nodedb-sql/src/ddl_ast/parse/maintenance.rs b/nodedb-sql/src/ddl_ast/parse/maintenance.rs index 191de6d85..de5381eb9 100644 --- a/nodedb-sql/src/ddl_ast/parse/maintenance.rs +++ b/nodedb-sql/src/ddl_ast/parse/maintenance.rs @@ -18,7 +18,10 @@ pub(super) fn try_parse( // name. PostgreSQL has no `ANALYZE ... (cols)` form; any trailing // `(...)` after the collection name is stripped so the name stays // clean. - if parts.first().is_some_and(|p| p.eq_ignore_ascii_case("ANALYZE")) { + if parts + .first() + .is_some_and(|p| p.eq_ignore_ascii_case("ANALYZE")) + { let collection = parts .get(1) .map(|s| s.split('(').next().unwrap_or(s).trim().to_string()); @@ -26,7 +29,10 @@ pub(super) fn try_parse( collection, })); } - if parts.first().is_some_and(|p| p.eq_ignore_ascii_case("COMPACT")) { + if parts + .first() + .is_some_and(|p| p.eq_ignore_ascii_case("COMPACT")) + { let collection = parts .get(1)? .split('(') diff --git a/nodedb-vector/src/planner/cost.rs b/nodedb-vector/src/planner/cost.rs index 3ed0da444..bfa5bd578 100644 --- a/nodedb-vector/src/planner/cost.rs +++ b/nodedb-vector/src/planner/cost.rs @@ -131,9 +131,7 @@ pub fn estimate_cost(inputs: &CostModelInputs) -> VectorCost { let rerank_us = if inputs.quantization == QuantizationKind::None { 0.0 } else { - crate::rerank::codecs::bbq::DEFAULT_OVERSAMPLE as f32 - * inputs.ef_search as f32 - * 0.01 + crate::rerank::codecs::bbq::DEFAULT_OVERSAMPLE as f32 * inputs.ef_search as f32 * 0.01 }; let predicted_recall = predicted_recall_for(inputs.index_type, inputs.quantization); @@ -164,7 +162,6 @@ mod tests { ); } - fn base_inputs() -> CostModelInputs { CostModelInputs { n_vectors: 1_000_000, From ef04b7b38dd7e2f9870ec9a99e6f9da1b2a692f5 Mon Sep 17 00:00:00 2001 From: EnRaiha <15997552+EnRaiha@users.noreply.github.com> Date: Sat, 5 Sep 2026 01:31:53 +0800 Subject: [PATCH 7/7] fix(sql): evaluate signed LIMIT literals instead of rejecting on shape PostgreSQL coerces constant LIMIT/OFFSET expressions, so a syntactic UnaryOp::Minus is not itself an error: LIMIT -0 must be accepted (0 rows), LIMIT - '-5' must be accepted (+5), LIMIT -5 and LIMIT '-1' must still be rejected (negative result). expr_as_nonnegative_usize now parses literal operands to i64, negates, and only rejects when the RESULT is negative; non-literal operands keep the documented unbounded semantics. Also drops a stale draft doc block and fixes the stale 'oversample-3' comment in the vector cost planner. --- nodedb-sql/src/coerce.rs | 116 +++++++++++++++++++++--------- nodedb-vector/src/planner/cost.rs | 2 +- 2 files changed, 82 insertions(+), 36 deletions(-) diff --git a/nodedb-sql/src/coerce.rs b/nodedb-sql/src/coerce.rs index 286834014..d3a7edabc 100644 --- a/nodedb-sql/src/coerce.rs +++ b/nodedb-sql/src/coerce.rs @@ -65,12 +65,6 @@ pub fn expr_as_usize_literal(expr: &ast::Expr) -> Option { } } -/// Fallible LIMIT/OFFSET resolution. -/// -/// Rejects negative literals (`LIMIT -1`, `OFFSET -2`) with -/// [`SqlError::InvalidLimitValue`] instead of collapsing to `None` -/// (unbounded). Everything else behaves exactly like -/// [`expr_as_usize_literal`]. /// Fallible LIMIT/OFFSET resolution. /// /// Rejects statically-negative bounds with [`SqlError::InvalidLimitValue`]: @@ -83,49 +77,61 @@ pub fn expr_as_usize_literal(expr: &ast::Expr) -> Option { /// behavior of any non-literal bound (`None` = no static limit). This matches /// the docstring contract of [`expr_as_usize_literal`]: only literals are /// resolved; everything else is the caller's semantic. +/// Evaluate a literal (number or quoted string) as a signed integer, +/// matching PostgreSQL's literal-coercion semantics for LIMIT/OFFSET. +/// +/// `-0` evaluates to 0 and is accepted; `- '-5'` (double negation) evaluates +/// to +5 and is accepted. Non-numeric or non-integer literals return `None`. +fn literal_as_i64(v: &ast::Value) -> Option { + match v { + ast::Value::SingleQuotedString(s) => s.trim().parse::().ok(), + ast::Value::Number(n, _) => n.trim().parse::().ok(), + _ => None, + } +} + pub fn expr_as_nonnegative_usize( expr: &ast::Expr, clause: &'static str, ) -> Result, SqlError> { - let reject_negative = |v: &ast::Value| match v { - ast::Value::SingleQuotedString(s) => s.trim_start().starts_with('-'), - ast::Value::Number(n, _) => n.trim_start().starts_with('-'), - _ => false, - }; - + // Fast path: positive integer literals (plain or quoted) unchanged. + if let Some(v) = expr_as_usize_literal(expr) { + return Ok(Some(v)); + } + // Signed literals under unary minus: evaluate, then admit or reject + // (PostgreSQL parity — reject only when the RESULT is negative: + // LIMIT -0 → 0 accepted, LIMIT - '-5' → 5 accepted, LIMIT -5 rejected). if let ast::Expr::UnaryOp { op: ast::UnaryOperator::Minus, expr: inner, } = expr { - // Unary minus over ANY literal operand is statically negative - // (-5, -'5', - '-5'): the operand's own sign is irrelevant, the - // negation is. Reject all literal operands... - if matches!( - inner.as_ref(), - ast::Expr::Value(ast::ValueWithSpan { - value: ast::Value::Number(..), - .. - }) | ast::Expr::Value(ast::ValueWithSpan { - value: ast::Value::SingleQuotedString(..), - .. - }) - ) { - return Err(SqlError::InvalidLimitValue { - detail: format!("{clause} must not be negative"), - }); + if let ast::Expr::Value(v) = inner.as_ref() { + if let Some(signed) = literal_as_i64(&v.value) { + let negated = signed.checked_neg().unwrap_or(i64::MIN); + if negated < 0 { + return Err(SqlError::InvalidLimitValue { + detail: format!("{clause} must not be negative"), + }); + } + return Ok(Some(negated as usize)); + } } - // Non-literal operand ($1, col): sign unknowable at plan time → - // unchanged "non-literal bound" semantics (None → unbounded). } + // Bare literals carrying their own minus sign (LIMIT '-1'). if let ast::Expr::Value(v) = expr { - if reject_negative(&v.value) { - return Err(SqlError::InvalidLimitValue { - detail: format!("{clause} must not be negative"), - }); + if let Some(signed) = literal_as_i64(&v.value) { + if signed < 0 { + return Err(SqlError::InvalidLimitValue { + detail: format!("{clause} must not be negative"), + }); + } + return Ok(Some(signed as usize)); } } - Ok(expr_as_usize_literal(expr)) + // Non-literal bound (LIMIT -$1, LIMIT -col): sign unknowable at plan + // time → documented pre-existing semantics (None = no static limit). + Ok(None) } /// Resolve a `Value` into an `f64` if numeric-shaped. @@ -344,6 +350,46 @@ mod tests { ), Err(SqlError::InvalidLimitValue { .. }) )); + // PostgreSQL literal coercion: -0 → 0 (accepted, not 2201W). + let stmts = sqlparser::parser::Parser::parse_sql( + &sqlparser::dialect::GenericDialect {}, + "SELECT * FROM t LIMIT -0", + ) + .unwrap(); + let ast::Statement::Query(q) = &stmts[0] else { + panic!("expected query statement"); + }; + let Some(ast::LimitClause::LimitOffset { limit: Some(expr), .. }) = + q.limit_clause.as_ref() + else { + panic!("expected LIMIT clause"); + }; + assert_eq!( + expr_as_nonnegative_usize(expr, "LIMIT").unwrap(), + Some(0), + "LIMIT -0 must evaluate to 0, not error" + ); + + // Double negation: LIMIT - '-5' → +5 (accepted). + let stmts = sqlparser::parser::Parser::parse_sql( + &sqlparser::dialect::GenericDialect {}, + "SELECT * FROM t LIMIT - '-5'", + ) + .unwrap(); + let ast::Statement::Query(q) = &stmts[0] else { + panic!("expected query statement"); + }; + let Some(ast::LimitClause::LimitOffset { limit: Some(expr), .. }) = + q.limit_clause.as_ref() + else { + panic!("expected LIMIT clause"); + }; + assert_eq!( + expr_as_nonnegative_usize(expr, "LIMIT").unwrap(), + Some(5), + "LIMIT - '-5' must evaluate to 5, not error" + ); + // Non-literal operand: sign unknowable at plan time → unchanged None. assert_eq!( expr_as_nonnegative_usize( diff --git a/nodedb-vector/src/planner/cost.rs b/nodedb-vector/src/planner/cost.rs index bfa5bd578..5ce99621d 100644 --- a/nodedb-vector/src/planner/cost.rs +++ b/nodedb-vector/src/planner/cost.rs @@ -86,7 +86,7 @@ fn predicted_recall_for(index_type: IndexType, quantization: QuantizationKind) - } (IndexType::Hnsw, QuantizationKind::Binary) => 0.85, (IndexType::Hnsw, QuantizationKind::Ternary) => 0.90, - // Vamana + RaBitQ with default oversample-3 rerank. + // Vamana + RaBitQ with the unified BBQ oversample-4 rerank. (IndexType::Vamana, QuantizationKind::RaBitQ) => 0.96, (IndexType::Vamana, QuantizationKind::Bbq) => 0.96, (IndexType::Vamana, QuantizationKind::None) => 0.99,