From e9f969f6001a5a0edfc1216c62230e944c7ff16f Mon Sep 17 00:00:00 2001 From: EnRaiha <15997552+EnRaiha@users.noreply.github.com> Date: Fri, 18 Sep 2026 06:49:24 +0800 Subject: [PATCH 1/2] fix(search): read derived search cells from the plan The derived-relation inference re-derived search routing from the AST and diverged from the planner in both directions: a WHERE sparse_score(...) or a one-argument ORDER BY vector_distance(...) declared a distance cell the planner never fills (silent NULL where main refused 42703), while SqlPlan::MultiVectorSearch went undeclared (42703 stays). SqlPlan::carries_search_cells() matches the three variants whose rows lower to the vector-hit shape; infer_subquery_relation reads the cells from the subquery's own plan, which the planner call sites already build. TableScope's derived arm plans non-lateral factors for the same input; a correlated LATERAL factor cannot be planned at scope time and routes no cells. The four AST heuristic functions and the third registry singleton are gone. --- nodedb-sql/src/lib.rs | 2 +- nodedb-sql/src/planner/cte/join_link.rs | 6 +- nodedb-sql/src/planner/cte/recursive_scan.rs | 6 +- nodedb-sql/src/planner/dml/insert.rs | 24 ++-- .../planner/dml_helpers/insert_select_bind.rs | 4 +- nodedb-sql/src/planner/join/plan.rs | 1 + nodedb-sql/src/planner/lateral/plan.rs | 13 ++- .../src/planner/select/comma_lateral.rs | 2 + nodedb-sql/src/planner/select/derived_from.rs | 10 +- nodedb-sql/src/planner/select/entry.rs | 9 +- nodedb-sql/src/planner/select/select_stmt.rs | 6 +- nodedb-sql/src/planner/subquery/exists.rs | 2 +- nodedb-sql/src/resolver/columns.rs | 40 +++++-- nodedb-sql/src/resolver/derived.rs | 104 ++++++++++++++++-- nodedb-sql/src/types/plan/mod.rs | 1 + nodedb-sql/src/types/plan/search_cells.rs | 93 ++++++++++++++++ 16 files changed, 285 insertions(+), 38 deletions(-) create mode 100644 nodedb-sql/src/types/plan/search_cells.rs diff --git a/nodedb-sql/src/lib.rs b/nodedb-sql/src/lib.rs index 7952b0505..ab0dfdbf5 100644 --- a/nodedb-sql/src/lib.rs +++ b/nodedb-sql/src/lib.rs @@ -146,7 +146,7 @@ fn plan_statements( let mut dml_plans = if is_upsert { planner::dml::plan_upsert(ins, catalog)? } else { - planner::dml::plan_insert(ins, catalog)? + planner::dml::plan_insert(ins, catalog, &functions, temporal)? }; plans.append(&mut dml_plans); } diff --git a/nodedb-sql/src/planner/cte/join_link.rs b/nodedb-sql/src/planner/cte/join_link.rs index b95d00136..dc6662df9 100644 --- a/nodedb-sql/src/planner/cte/join_link.rs +++ b/nodedb-sql/src/planner/cte/join_link.rs @@ -7,9 +7,11 @@ use sqlparser::ast::{self, SetExpr}; use crate::error::{Result, SqlError}; +use crate::functions::registry::FunctionRegistry; use crate::parser::normalize::{normalize_ident, table_name_from_factor}; use crate::planner::select::CteCatalog; use crate::resolver::columns::TableScope; +use crate::temporal::TemporalScope; use crate::types::*; /// Extract recursive info from the AST when normal planning fails @@ -24,6 +26,8 @@ pub(super) fn extract_recursive_info( expr: &SetExpr, cte_name: &str, catalog: &dyn SqlCatalog, + functions: &FunctionRegistry, + temporal: TemporalScope, ) -> Result { let select = match expr { SetExpr::Select(s) => s, @@ -69,7 +73,7 @@ pub(super) fn extract_recursive_info( // The working table is absent from the ordinary catalog, so the CTE name // resolves as an open relation for the duration of this arm. let arm_catalog = CteCatalog::open(catalog, cte_name); - let scope = TableScope::resolve_from(&arm_catalog, &select.from)?; + let scope = TableScope::resolve_from(&arm_catalog, functions, temporal, &select.from)?; // Extract the join link from the ON condition. let join_link = if let (Some(real_alias), Some(cte_al), Some(on_expr)) = diff --git a/nodedb-sql/src/planner/cte/recursive_scan.rs b/nodedb-sql/src/planner/cte/recursive_scan.rs index 7d4da4ebf..76e70ac1f 100644 --- a/nodedb-sql/src/planner/cte/recursive_scan.rs +++ b/nodedb-sql/src/planner/cte/recursive_scan.rs @@ -104,6 +104,8 @@ pub fn plan_recursive_cte( distinct, }, catalog, + functions, + temporal, ) } @@ -172,6 +174,8 @@ fn plan_recursive_scan_from_parts( base: &SqlPlan, parts: &RecursiveParts<'_>, catalog: &dyn SqlCatalog, + functions: &FunctionRegistry, + temporal: crate::TemporalScope, ) -> Result { let RecursiveParts { left, @@ -198,7 +202,7 @@ fn plan_recursive_scan_from_parts( // shape directly instead of attempting ordinary planning and swallowing // whichever error happens to occur first. let (recursive_filters, join_link) = - super::join_link::extract_recursive_info(right, cte_name, catalog)?; + super::join_link::extract_recursive_info(right, cte_name, catalog, functions, temporal)?; // The anchor plan carries the CTE's resolved output columns; propagate // them so the recursive scan self-describes its output schema. diff --git a/nodedb-sql/src/planner/dml/insert.rs b/nodedb-sql/src/planner/dml/insert.rs index bbbe1ce1b..b7bfb4293 100644 --- a/nodedb-sql/src/planner/dml/insert.rs +++ b/nodedb-sql/src/planner/dml/insert.rs @@ -18,7 +18,12 @@ use crate::error::Result; use crate::types::*; /// Plan an INSERT statement. -pub fn plan_insert(ins: &ast::Insert, catalog: &dyn SqlCatalog) -> Result> { +pub fn plan_insert( + ins: &ast::Insert, + catalog: &dyn SqlCatalog, + functions: &crate::functions::registry::FunctionRegistry, + temporal: crate::TemporalScope, +) -> Result> { let (table_name, info) = resolve_target(ins, "INSERT", catalog)?; let target_scope = target_scope(&table_name, &info)?; @@ -39,13 +44,9 @@ pub fn plan_insert(ins: &ast::Insert, catalog: &dyn SqlCatalog) -> Result { /// so a correlated reference to an outer relation resolves. pub outer_scope: &'a TableScope, pub catalog: &'a dyn SqlCatalog, + pub functions: &'a FunctionRegistry, pub temporal: TemporalScope, } @@ -56,6 +58,7 @@ pub fn plan_lateral_join(args: LateralJoinArgs<'_>) -> Result { outer_projection, outer_scope, catalog, + functions, temporal, } = args; let select = match subquery.body.as_ref() { @@ -103,6 +106,8 @@ pub fn plan_lateral_join(args: LateralJoinArgs<'_>) -> Result { outer_projection, outer_scope, catalog, + functions, + temporal, }) } else if has_equi && analysis.non_equi.is_empty() { // Equi-correlated, no LIMIT: rewrite as a regular hash join. @@ -255,6 +260,8 @@ struct LateralTopKPlanArgs<'a> { outer_projection: Vec, outer_scope: &'a TableScope, catalog: &'a dyn SqlCatalog, + functions: &'a FunctionRegistry, + temporal: TemporalScope, } /// Plan the `LateralTopK` variant: equi-correlated + ORDER BY + LIMIT k. @@ -271,6 +278,8 @@ fn plan_lateral_top_k(args: LateralTopKPlanArgs<'_>) -> Result { outer_projection, outer_scope, catalog, + functions, + temporal, } = args; // Build a bare inner Scan without correlation filters (those are injected // at runtime per outer row). @@ -283,8 +292,8 @@ fn plan_lateral_top_k(args: LateralTopKPlanArgs<'_>) -> Result { // The Top-K plan does not retain the inner alias, but it must still reject // malformed aliases before expressions referencing them are lowered. let _inner_alias = extract_inner_alias(select)?; - let inner_scope = - TableScope::resolve_from(catalog, &select.from)?.nested_in(outer_scope.clone()); + let inner_scope = TableScope::resolve_from(catalog, functions, temporal, &select.from)? + .nested_in(outer_scope.clone()); let inner_filters = inner_non_correlated_filters(select, outer_alias.as_deref().unwrap_or(""), &inner_scope)?; diff --git a/nodedb-sql/src/planner/select/comma_lateral.rs b/nodedb-sql/src/planner/select/comma_lateral.rs index ec7c7faf2..d37e73597 100644 --- a/nodedb-sql/src/planner/select/comma_lateral.rs +++ b/nodedb-sql/src/planner/select/comma_lateral.rs @@ -24,6 +24,7 @@ pub(super) fn try_plan_comma_lateral( select: &Select, scope: &TableScope, catalog: &dyn SqlCatalog, + functions: &crate::functions::registry::FunctionRegistry, temporal: TemporalScope, ) -> Result> { if select.from.len() != 2 || !is_lateral_derived(&select.from[1].relation) { @@ -74,6 +75,7 @@ pub(super) fn try_plan_comma_lateral( outer_projection: projection, outer_scope: scope, catalog, + functions, temporal, }) .map(Some) diff --git a/nodedb-sql/src/planner/select/derived_from.rs b/nodedb-sql/src/planner/select/derived_from.rs index a75398041..48d5e91d4 100644 --- a/nodedb-sql/src/planner/select/derived_from.rs +++ b/nodedb-sql/src/planner/select/derived_from.rs @@ -58,8 +58,14 @@ pub(in crate::planner::select) fn try_plan_derived_from( // Replan the outer SELECT against a catalog that resolves the alias to // the columns the subquery projects. The outer can reference `alias.col` // qualified or unqualified. - let relation = - crate::resolver::derived::infer_subquery_relation(catalog, &alias_name, subquery)?; + let relation = crate::resolver::derived::infer_subquery_relation( + catalog, + &alias_name, + subquery, + Some(&inner_plan), + functions, + temporal, + )?; let derived_catalog = CteCatalog { inner: catalog, relations: vec![( diff --git a/nodedb-sql/src/planner/select/entry.rs b/nodedb-sql/src/planner/select/entry.rs index 48a41f8f6..ce78f733b 100644 --- a/nodedb-sql/src/planner/select/entry.rs +++ b/nodedb-sql/src/planner/select/entry.rs @@ -96,7 +96,14 @@ pub fn plan_query( .map(|column| check_ast_identifier(&column.name)) .collect::>()?; let cte_plan = plan_query(&cte.query, catalog, functions, temporal)?; - let info = infer_subquery_relation(catalog, &name, &cte.query)?; + let info = infer_subquery_relation( + catalog, + &name, + &cte.query, + Some(&cte_plan), + functions, + temporal, + )?; definitions.push((name.clone(), cte_plan)); relations.push((name, rename_output_columns(info, &declared))); } diff --git a/nodedb-sql/src/planner/select/select_stmt.rs b/nodedb-sql/src/planner/select/select_stmt.rs index 3ab107f9e..92b287cc1 100644 --- a/nodedb-sql/src/planner/select/select_stmt.rs +++ b/nodedb-sql/src/planner/select/select_stmt.rs @@ -43,7 +43,7 @@ pub(super) fn plan_select( // attrs, so ORDER BY and the tail clauses resolve its columns. return Ok(PlannedSelect { plan, - scope: TableScope::resolve_from(catalog, &select.from)?, + scope: TableScope::resolve_from(catalog, functions, temporal, &select.from)?, }); } @@ -61,7 +61,7 @@ pub(super) fn plan_select( } // 1. Resolve FROM tables. - let scope = TableScope::resolve_from(catalog, &select.from)?; + let scope = TableScope::resolve_from(catalog, functions, temporal, &select.from)?; // 2. Handle constant queries (no FROM clause): SELECT 1, SELECT 'hello', etc. if select.from.is_empty() { @@ -111,7 +111,7 @@ pub(super) fn plan_select( } // 3b. Comma-LATERAL syntax: `FROM t, LATERAL (SELECT ...) x`. - if let Some(plan) = try_plan_comma_lateral(select, &scope, catalog, temporal)? { + if let Some(plan) = try_plan_comma_lateral(select, &scope, catalog, functions, temporal)? { return Ok(PlannedSelect { plan, scope }); } diff --git a/nodedb-sql/src/planner/subquery/exists.rs b/nodedb-sql/src/planner/subquery/exists.rs index 971a4c205..234456cff 100644 --- a/nodedb-sql/src/planner/subquery/exists.rs +++ b/nodedb-sql/src/planner/subquery/exists.rs @@ -50,7 +50,7 @@ pub(super) fn plan_exists_subquery( }); }; - let local = TableScope::resolve_from(catalog, &select.from)?; + let local = TableScope::resolve_from(catalog, functions, temporal, &select.from)?; let nested = local.clone().nested_in(outer.clone()); // EXISTS discards the projected values. The conversion still runs so a diff --git a/nodedb-sql/src/resolver/columns.rs b/nodedb-sql/src/resolver/columns.rs index cc5d7f18b..7e7de3481 100644 --- a/nodedb-sql/src/resolver/columns.rs +++ b/nodedb-sql/src/resolver/columns.rs @@ -7,7 +7,9 @@ use std::collections::HashMap; use nodedb_types::DatabaseId; use crate::error::{Result, SqlError}; +use crate::functions::registry::FunctionRegistry; use crate::parser::normalize::table_name_from_factor; +use crate::temporal::TemporalScope; use crate::types::{CollectionInfo, ColumnInfo, SqlCatalog}; /// Synthetic temporal columns an audit read injects into every version row. @@ -243,15 +245,21 @@ impl TableScope { } /// Resolve tables from a FROM clause. + /// + /// A derived-subquery factor is planned here so its relation carries the + /// search cells its plan produces; the planner context (`functions`, + /// `temporal`) is required for that and for no other arm. pub fn resolve_from( catalog: &dyn SqlCatalog, + functions: &FunctionRegistry, + temporal: TemporalScope, from: &[sqlparser::ast::TableWithJoins], ) -> Result { let mut scope = Self::new(); for table_with_joins in from { - scope.resolve_table_factor(catalog, &table_with_joins.relation)?; + scope.resolve_table_factor(catalog, functions, temporal, &table_with_joins.relation)?; for join in &table_with_joins.joins { - scope.resolve_table_factor(catalog, &join.relation)?; + scope.resolve_table_factor(catalog, functions, temporal, &join.relation)?; } } Ok(scope) @@ -260,6 +268,8 @@ impl TableScope { fn resolve_table_factor( &mut self, catalog: &dyn SqlCatalog, + functions: &FunctionRegistry, + temporal: TemporalScope, factor: &sqlparser::ast::TableFactor, ) -> Result<()> { // ARRAY_*(...) table-valued function: synthesize a ResolvedTable @@ -270,10 +280,13 @@ impl TableScope { return Ok(()); } // Derived subquery, LATERAL or not: register the alias as the relation - // its projection list exposes, so a column reference on the alias - // resolves without a catalog lookup. The inner plan is built - // separately. + // its projection list exposes — a column reference on the alias + // resolves without a catalog lookup. A non-LATERAL subquery is planned + // so its relation carries the search cells the plan produces; a + // correlated LATERAL one cannot be planned at scope time, and the + // lateral planner routes its shapes without those cells. if let sqlparser::ast::TableFactor::Derived { + lateral, subquery, alias: Some(alias), .. @@ -285,8 +298,21 @@ impl TableScope { .iter() .map(|column| crate::reserved::check_ast_identifier(&column.name)) .collect::>()?; - let info = - crate::resolver::derived::infer_subquery_relation(catalog, &alias_str, subquery)?; + let plan = if *lateral { + None + } else { + Some(crate::planner::select::plan_query( + subquery, catalog, functions, temporal, + )?) + }; + let info = crate::resolver::derived::infer_subquery_relation( + catalog, + &alias_str, + subquery, + plan.as_ref(), + functions, + temporal, + )?; self.add(ResolvedTable { name: alias_str.clone(), alias: Some(alias_str), diff --git a/nodedb-sql/src/resolver/derived.rs b/nodedb-sql/src/resolver/derived.rs index 104c5e1b3..e19ebe256 100644 --- a/nodedb-sql/src/resolver/derived.rs +++ b/nodedb-sql/src/resolver/derived.rs @@ -5,21 +5,48 @@ use sqlparser::ast::{self, Expr, SelectItem, SelectItemQualifiedWildcardKind, SetExpr}; use crate::error::{Result, SqlError}; +use crate::functions::registry::FunctionRegistry; use crate::parser::normalize::{normalize_ident, normalize_object_name_checked}; use crate::resolver::columns::TableScope; -use crate::types::{CollectionInfo, ColumnInfo, EngineType, SqlCatalog, SqlDataType}; +use crate::temporal::TemporalScope; +use crate::types::{CollectionInfo, ColumnInfo, EngineType, SqlCatalog, SqlDataType, SqlPlan}; /// The relation a subquery alias exposes. /// /// A synthesized relation carries `EngineType::DocumentSchemaless`: it is a /// MessagePack row stream, and the CTE lowering depends on that. Openness /// rides on `CollectionInfo::open_schema`, not on the engine. +/// +/// `plan` is the subquery's own plan. The search cells (`distance`, +/// `_surrogate`) are read from it, never re-derived from the AST: the planner +/// is the only layer that decides whether an operator form routes to a search. +/// A correlated LATERAL subquery cannot be planned at scope time and carries +/// no plan: it routes no search cells either (the lateral planner handles its +/// shapes directly). pub fn infer_subquery_relation( catalog: &dyn SqlCatalog, alias: &str, query: &ast::Query, + plan: Option<&SqlPlan>, + functions: &FunctionRegistry, + temporal: TemporalScope, ) -> Result { - let (columns, open_schema) = infer_projection(catalog, query)?; + let (mut columns, open_schema) = infer_projection(catalog, query, functions, temporal)?; + // A search-shaped plan answers with the source columns plus two synthetic + // cells: `distance` (Float64) and `_surrogate`, the internal row id + // resolved to the user primary key. Neither is a declared column of a + // closed-schema source, so derived-relation inference must declare them: + // without it `s.distance` and `s._surrogate` resolve against no relation + // and are refused with 42703, while the same projection over an + // open-schema source runs. + if plan.is_some_and(SqlPlan::carries_search_cells) { + if !columns.iter().any(|c| c.name == "distance") { + columns.push(synthetic_column("distance")); + } + if !columns.iter().any(|c| c.name == "_surrogate") { + columns.push(synthetic_column("_surrogate")); + } + } Ok(CollectionInfo { name: alias.to_string(), engine: EngineType::DocumentSchemaless, @@ -85,16 +112,23 @@ pub fn rename_output_columns(mut info: CollectionInfo, names: &[String]) -> Coll fn infer_projection( catalog: &dyn SqlCatalog, query: &ast::Query, + functions: &FunctionRegistry, + temporal: TemporalScope, ) -> Result<(Vec, bool)> { - infer_body(catalog, &query.body) + infer_body(catalog, &query.body, functions, temporal) } -fn infer_body(catalog: &dyn SqlCatalog, body: &SetExpr) -> Result<(Vec, bool)> { +fn infer_body( + catalog: &dyn SqlCatalog, + body: &SetExpr, + functions: &FunctionRegistry, + temporal: TemporalScope, +) -> Result<(Vec, bool)> { match body { - SetExpr::Select(select) => infer_select_projection(catalog, select), - SetExpr::Query(query) => infer_projection(catalog, query), + SetExpr::Select(select) => infer_select_projection(catalog, select, functions, temporal), + SetExpr::Query(query) => infer_projection(catalog, query, functions, temporal), // A set operation takes its output names from the left arm. - SetExpr::SetOperation { left, .. } => infer_body(catalog, left), + SetExpr::SetOperation { left, .. } => infer_body(catalog, left, functions, temporal), // A row constructor, a `TABLE` command, and a DML body carry no // projection list to read names from. SetExpr::Values(_) @@ -109,8 +143,10 @@ fn infer_body(catalog: &dyn SqlCatalog, body: &SetExpr) -> Result<(Vec Result<(Vec, bool)> { - let scope = TableScope::resolve_from(catalog, &select.from)?; + let scope = TableScope::resolve_from(catalog, functions, temporal, &select.from)?; let mut columns = Vec::new(); let mut open = false; @@ -266,7 +302,24 @@ mod tests { } fn infer(sql: &str) -> CollectionInfo { - infer_subquery_relation(&TestCatalog, "t", &parse_query(sql)).expect("inference failed") + let query = parse_query(sql); + let functions = crate::functions::registry::FunctionRegistry::new(); + let plan = crate::planner::select::plan_query( + &query, + &TestCatalog, + &functions, + TemporalScope::default(), + ) + .expect("plan failed"); + infer_subquery_relation( + &TestCatalog, + "t", + &query, + Some(&plan), + &functions, + TemporalScope::default(), + ) + .expect("inference failed") } #[test] @@ -320,4 +373,37 @@ mod tests { assert_eq!(names, vec!["p", "q"]); assert_eq!(info.columns[0].data_type, SqlDataType::Int64); } + + #[test] + fn vector_search_projection_declares_the_synthetic_distance_column() { + // `SEARCH c USING VECTOR(...)` preprocesses to `ORDER BY + // vector_distance(...)`; the response layer appends a `distance` + // cell, so the derived relation must name it even when the source + // schema is closed. + let info = infer("SELECT * FROM src ORDER BY vector_distance(b, ARRAY[0.1, 0.2]) LIMIT 2"); + let names: Vec<&str> = info.columns.iter().map(|c| c.name.as_str()).collect(); + assert!(names.contains(&"distance"), "columns: {names:?}"); + } + + #[test] + fn plain_ordered_projection_has_no_distance_column() { + let info = infer("SELECT * FROM src ORDER BY b LIMIT 2"); + let names: Vec<&str> = info.columns.iter().map(|c| c.name.as_str()).collect(); + assert!(!names.contains(&"distance"), "columns: {names:?}"); + } + + /// Every function that routes an ORDER BY to a vector search produces a + /// `distance` cell, so every one of them must declare it. + #[test] + fn every_vector_search_function_declares_the_distance_column() { + for call in [ + "vector_distance(b, ARRAY[0.1])", + "vector_cosine_distance(b, ARRAY[0.1])", + "vector_neg_inner_product(b, ARRAY[0.1])", + ] { + let info = infer(&format!("SELECT * FROM src ORDER BY {call} LIMIT 2")); + let names: Vec<&str> = info.columns.iter().map(|c| c.name.as_str()).collect(); + assert!(names.contains(&"distance"), "{call}: columns {names:?}"); + } + } } diff --git a/nodedb-sql/src/types/plan/mod.rs b/nodedb-sql/src/types/plan/mod.rs index 371c9a13e..a420fb1fe 100644 --- a/nodedb-sql/src/types/plan/mod.rs +++ b/nodedb-sql/src/types/plan/mod.rs @@ -5,6 +5,7 @@ mod cacheability; mod merge_types; mod row_types; +mod search_cells; mod variant_name; mod variants; mod vector_opts; diff --git a/nodedb-sql/src/types/plan/search_cells.rs b/nodedb-sql/src/types/plan/search_cells.rs new file mode 100644 index 000000000..51233cd5e --- /dev/null +++ b/nodedb-sql/src/types/plan/search_cells.rs @@ -0,0 +1,93 @@ +// SPDX-License-Identifier: Apache-2.0 + +//! Search-cell classification for plan variants. + +use super::variants::SqlPlan; + +impl SqlPlan { + /// Whether this plan's rows carry the search cells `distance` and + /// `_surrogate`. + /// + /// The three variants here lower to the vector-hit row shape + /// (`classify_hit_shape` → `HitShape::Vector`, over `VectorOp::Search | + /// MultiSearch | SparseSearch | MultiVectorScoreSearch`): the engine emits + /// `{id: , distance, …}` rows, and the post-process arm resolves + /// the surrogate to the user primary key. + /// + /// Hybrid fusion rows (`HybridSearch`, `HybridSearchTriple`) classify as + /// `HitShape::Hybrid` and carry a `doc_id` plus a score alias instead, so + /// they are not search-cell plans. + pub fn carries_search_cells(&self) -> bool { + matches!( + self, + SqlPlan::VectorSearch { .. } + | SqlPlan::SparseSearch { .. } + | SqlPlan::MultiVectorSearch { .. } + ) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::temporal::TemporalScope; + use crate::types::*; + + fn scan() -> SqlPlan { + SqlPlan::Scan { + collection: "c".into(), + alias: None, + engine: EngineType::DocumentSchemaless, + filters: Vec::new(), + projection: Vec::new(), + sort_keys: Vec::new(), + limit: None, + offset: 0, + distinct: false, + window_functions: Vec::new(), + temporal: TemporalScope::default(), + } + } + + #[test] + fn only_the_three_vector_hit_variants_carry_the_cells() { + assert!( + SqlPlan::VectorSearch { + collection: "c".into(), + field: "e".into(), + query_vector: vec![0.1], + top_k: 1, + ef_search: 2, + metric: DistanceMetric::Cosine, + filters: Vec::new(), + array_prefilter: None, + ann_options: VectorAnnOptions::default(), + skip_payload_fetch: false, + payload_filters: Vec::new(), + projection: Vec::new(), + } + .carries_search_cells() + ); + assert!( + SqlPlan::SparseSearch { + collection: "c".into(), + field: "t".into(), + query_entries: vec![(3, 1.0)], + top_k: 1, + projection: Vec::new(), + } + .carries_search_cells() + ); + assert!( + SqlPlan::MultiVectorSearch { + collection: "c".into(), + query_vector: vec![0.1], + top_k: 1, + ef_search: 2, + projection: Vec::new(), + } + .carries_search_cells() + ); + assert!(!scan().carries_search_cells()); + } +} From e0a51d035f3ff781e6eb368c346564bd73b5382a Mon Sep 17 00:00:00 2001 From: EnRaiha <15997552+EnRaiha@users.noreply.github.com> Date: Fri, 18 Sep 2026 06:49:24 +0800 Subject: [PATCH 2/2] test(search): pin the declared cells and the planner divergences The tolerant assertions pinned one outcome each: a declared distance parses as f64 with no empty escape, a body routing no search refuses 42703 instead of reading a NULL cell. Guards cover the four divergences the review listed: sparse WHERE, one-argument ORDER BY vector_distance, a body off a derived relation, and WHERE multi_vector_search (declared; the lowering refuses 42601). --- .../cases/sql_search_subquery_composition.rs | 328 ++++++++++++++++++ 1 file changed, 328 insertions(+) diff --git a/nodedb/tests/wire/cases/sql_search_subquery_composition.rs b/nodedb/tests/wire/cases/sql_search_subquery_composition.rs index 3e76a1d04..14cf6dc6d 100644 --- a/nodedb/tests/wire/cases/sql_search_subquery_composition.rs +++ b/nodedb/tests/wire/cases/sql_search_subquery_composition.rs @@ -264,3 +264,331 @@ async fn outer_order_by_distance_then_limit_takes_farthest() { "LIMIT after an outer ORDER BY must cut the reordered rows, got: {rows:?}" ); } + +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn outer_order_by_vector_distance_declares_distance() { + let server = TestServer::start().await; + create_vector_collection(&server, "vec_implicit").await; + + // A hand-written `ORDER BY vector_distance(...)` derived table (no SEARCH + // keyword) takes the same sort-trigger rewrite as the SEARCH form, so the + // synthetic `distance` column must resolve AND carry a value: declaring + // the column without producing a cell would make `s.distance` a phantom. + let rows = server + .query_text( + "SELECT s.distance FROM \ + (SELECT * FROM vec_implicit \ + ORDER BY vector_distance(embedding, ARRAY[0.1, 0.2, 0.3, 0.4]) LIMIT 2) s", + ) + .await + .unwrap(); + assert_eq!(rows.len(), 2, "two rows expected, got: {rows:?}"); + let first: f64 = rows[0] + .parse() + .unwrap_or_else(|_| panic!("distance must be numeric, got: {rows:?}")); + let second: f64 = rows[1] + .parse() + .unwrap_or_else(|_| panic!("distance must be numeric, got: {rows:?}")); + assert!( + first <= second, + "distance must be ordered nearest-first, got: {rows:?}" + ); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn outer_order_by_vector_distance_without_index_stays_consistent() { + let server = TestServer::start().await; + server.exec("CREATE COLLECTION vec_no_index").await.unwrap(); + for (id, v) in [ + ("r0", [0.10f32, 0.20, 0.30, 0.40]), + ("r1", [0.11, 0.21, 0.31, 0.41]), + ] { + server + .exec(&format!( + "INSERT INTO vec_no_index (id, embedding) VALUES ('{id}', ARRAY[{},{},{},{}])", + v[0], v[1], v[2], v[3] + )) + .await + .unwrap(); + } + + // The sort-trigger rewrite does not consult the index, so the plan is the + // same `VectorSearch` a collection with an index gets — but the search + // itself serves no hits without one. The pinned outcome is an empty + // result: a declared `distance` cell is never NULL, and a row only ever + // appears with a value. + let rows = server + .query_text( + "SELECT s.distance FROM \ + (SELECT * FROM vec_no_index \ + ORDER BY vector_distance(embedding, ARRAY[0.1, 0.2, 0.3, 0.4]) LIMIT 2) s", + ) + .await + .unwrap_or_else(|e| panic!("the rewrite serves distances without an index: {e}")); + assert!( + rows.is_empty(), + "no index serves no hits; a returned row would carry a numeric distance: {rows:?}" + ); +} + +/// The filed defect: a closed-schema source declares no `distance` column, so +/// the synthetic cell the response layer appends resolves only when +/// derived-relation inference adds the name. On an open source the same +/// projection always resolved, which is why this case pins the closed schema. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn search_over_a_closed_schema_resolves_a_synthetic_distance_column() { + let server = TestServer::start().await; + server + .exec( + "CREATE COLLECTION sp_strict (id TEXT PRIMARY KEY, embedding VECTOR(3)) \ + WITH (engine = 'document_strict')", + ) + .await + .unwrap(); + server + .exec("CREATE VECTOR INDEX idx_sp_strict ON sp_strict (embedding) METRIC COSINE DIM 3") + .await + .unwrap(); + // The string form: an `ARRAY[...]` literal on a strict schema is a separate + // defect and not what this case measures. + server + .exec("INSERT INTO sp_strict (id, embedding) VALUES ('s1', '[0.1, 0.2, 0.3]')") + .await + .unwrap(); + + let rows = server + .query_text( + "SELECT s.distance \ + FROM (SEARCH sp_strict USING VECTOR(embedding, ARRAY[0.1, 0.2, 0.3], 2)) s", + ) + .await + .unwrap(); + assert_eq!(rows.len(), 1, "one row expected, got: {rows:?}"); + rows[0] + .parse::() + .unwrap_or_else(|_| panic!("distance must be numeric, got: {rows:?}")); +} + +/// A closed-schema collection with a vector index and one row: the shape every +/// distance-column case below needs. +async fn create_closed_vector(server: &TestServer, name: &str) { + server + .exec(&format!( + "CREATE COLLECTION {name} (id TEXT PRIMARY KEY, embedding VECTOR(3)) \ + WITH (engine = 'document_strict')" + )) + .await + .unwrap(); + server + .exec(&format!( + "CREATE VECTOR INDEX idx_{name}_emb ON {name} (embedding) METRIC COSINE DIM 3" + )) + .await + .unwrap(); + server + .exec(&format!( + "INSERT INTO {name} (id, embedding) VALUES ('s1', '[0.1, 0.2, 0.3]')" + )) + .await + .unwrap(); +} + +/// Every function that routes an ORDER BY to a vector search appends a +/// `distance` cell, so each one must resolve over a closed schema — not only +/// `vector_distance`. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn every_vector_search_form_resolves_the_distance_column() { + let server = TestServer::start().await; + create_closed_vector(&server, "sp_forms").await; + + for order_by in [ + "vector_cosine_distance(embedding, ARRAY[0.1, 0.2, 0.3])", + "vector_neg_inner_product(embedding, ARRAY[0.1, 0.2, 0.3])", + "embedding <=> ARRAY[0.1, 0.2, 0.3]", + ] { + let sql = format!( + "SELECT s.distance FROM \ + (SELECT * FROM sp_forms ORDER BY {order_by} LIMIT 2) s" + ); + let rows = server.query_text(&sql).await.unwrap_or_else(|e| { + panic!("ORDER BY {order_by} must resolve s.distance: {e}"); + }); + assert_eq!(rows.len(), 1, "ORDER BY {order_by}: one row, got {rows:?}"); + rows[0].parse::().unwrap_or_else(|_| { + panic!("ORDER BY {order_by}: distance must be numeric, got {rows:?}") + }); + } +} + +/// A search also answers with `_surrogate`. A WHERE operator form routes to a +/// search (the preprocessor rewrites it to a bare call that +/// `try_extract_where_search` dispatches), so its cells are declared; a +/// comparison wrapped around the call does not route, so the projection must +/// refuse `42703` instead of reading a NULL cell. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn search_projections_resolve_surrogate_and_a_where_trigger() { + let server = TestServer::start().await; + create_closed_vector(&server, "sp_cells").await; + + let rows = server + .query_text( + "SELECT s._surrogate FROM \ + (SELECT * FROM sp_cells ORDER BY vector_distance(embedding, ARRAY[0.1, 0.2, 0.3]) LIMIT 2) s", + ) + .await + .unwrap_or_else(|e| panic!("s._surrogate must resolve over a closed schema: {e}")); + assert_eq!(rows.len(), 1, "one row, got {rows:?}"); + rows[0] + .parse::() + .unwrap_or_else(|_| panic!("_surrogate must be an integer id, got {rows:?}")); + + let rows = server + .query_text( + "SELECT s.distance FROM \ + (SELECT * FROM sp_cells \ + WHERE embedding <=> ARRAY[0.1, 0.2, 0.3] LIMIT 2) s", + ) + .await + .unwrap_or_else(|e| panic!("a routing WHERE form must declare s.distance: {e}")); + assert_eq!(rows.len(), 1, "one row expected, got {rows:?}"); + rows[0] + .parse::() + .unwrap_or_else(|_| panic!("distance must be numeric, got {rows:?}")); + + let error = server + .query_text( + "SELECT s.distance FROM \ + (SELECT * FROM sp_cells \ + WHERE vector_cosine_distance(embedding, ARRAY[0.1, 0.2, 0.3]) < 2.0) s", + ) + .await + .expect_err("a comparison around the call routes no search"); + assert!( + error.contains("42703"), + "the undeclared cell must refuse, got: {error}" + ); +} + +/// A sparse trigger routes an ORDER BY to `SqlPlan::SparseSearch`, so +/// `s.distance` resolves with a value per returned hit. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn a_sparse_order_by_resolves_the_distance_column() { + let server = TestServer::start().await; + server + .exec("CREATE TABLE sp_sparse (id TEXT PRIMARY KEY, terms SPARSEVECTOR)") + .await + .unwrap(); + server + .exec("INSERT INTO sp_sparse (id, terms) VALUES ('s1', '{3: 1.0, 7: 0.5}')") + .await + .unwrap(); + + let sql = "SELECT s.distance FROM \ + (SELECT * FROM sp_sparse ORDER BY sparse_score(terms, '{3: 1.0}') LIMIT 2) s"; + let rows = server + .query_text(sql) + .await + .unwrap_or_else(|e| panic!("a sparse ORDER BY declares s.distance: {e}")); + assert_eq!(rows.len(), 1, "one row expected, got {rows:?}"); + rows[0] + .parse::() + .unwrap_or_else(|_| panic!("distance must be numeric, got {rows:?}")); +} + +/// A WHERE `sparse_score(...)` is a scalar fallback in the planner +/// (`where_search`'s dispatch sends every non-WHERE trigger to `Ok(None)`), +/// so the plan carries no search cells. The projection must refuse `42703` +/// instead of declaring a cell whose rows read NULL. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn a_sparse_where_trigger_refuses_the_distance_cell() { + let server = TestServer::start().await; + server + .exec("CREATE TABLE sp_sparse_where (id TEXT PRIMARY KEY, terms SPARSEVECTOR)") + .await + .unwrap(); + server + .exec("INSERT INTO sp_sparse_where (id, terms) VALUES ('s1', '{3: 1.0, 7: 0.5}')") + .await + .unwrap(); + + let error = server + .query_text( + "SELECT s.distance FROM \ + (SELECT * FROM sp_sparse_where WHERE sparse_score(terms, '{3: 1.0}') > 0.1) s", + ) + .await + .expect_err("the scalar fallback carries no distance cell"); + assert!( + error.contains("42703"), + "expected 42703 for the undeclared cell, got: {error}" + ); +} + +/// A one-argument `vector_distance` does not route to a search +/// (`order_by/triggers.rs` returns `Ok(None)` below two arguments), so no +/// cell is declared and `s.distance` must refuse `42703`. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn a_single_argument_order_by_refuses_the_distance_cell() { + let server = TestServer::start().await; + create_closed_vector(&server, "sp_one_arg").await; + + let error = server + .query_text( + "SELECT s.distance FROM \ + (SELECT * FROM sp_one_arg ORDER BY vector_distance(embedding) LIMIT 2) s", + ) + .await + .expect_err("a one-argument call routes no search"); + assert!( + error.contains("42703") || error.contains("42883"), + "expected 42703 (undeclared cell) or 42883 (arity), got: {error}" + ); +} + +/// A body whose FROM is a derived relation gives `try_extract_sort_search` no +/// `Scan` or `Join` to read (`Ok(None)`), so its order-by trigger routes no +/// search and the outer projection must refuse `42703`. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn a_body_off_a_derived_relation_refuses_the_distance_cell() { + let server = TestServer::start().await; + create_closed_vector(&server, "sp_nested").await; + + let error = server + .query_text( + "SELECT s.distance FROM \ + (SELECT * FROM (SELECT * FROM sp_nested LIMIT 2) inner_s \ + ORDER BY vector_distance(embedding, ARRAY[0.1, 0.2, 0.3]) LIMIT 2) s", + ) + .await + .expect_err("a non-Scan body routes no search"); + assert!( + error.contains("42703") || error.contains("42883"), + "expected 42703 (undeclared cell) or 42883 (arity), got: {error}" + ); +} + +/// `WHERE multi_vector_search(field, query)` plans `SqlPlan::MultiVectorSearch`, +/// whose rows carry the search cells: `s.distance` is declared, so planning +/// passes and any refusal belongs to the lowering step, never `42703`. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn a_multi_vector_where_trigger_declares_the_distance_cell() { + let server = TestServer::start().await; + create_closed_vector(&server, "sp_multi").await; + + let error = server + .query_text( + "SELECT s.distance FROM \ + (SELECT * FROM sp_multi \ + WHERE multi_vector_search(embedding, ARRAY[0.1, 0.2, 0.3])) s", + ) + .await + .expect_err("the variant is not lowered yet; the cell declaration still happens"); + assert!( + !error.contains("42703"), + "the cell follows the plan and must resolve; got: {error}" + ); + assert!( + error.contains("42601") && error.contains("MultiVectorSearch"), + "expected the lowering refusal (42601, variant), got: {error}" + ); +}