From e8302a92c700576a650d367d22613a06643cd561 Mon Sep 17 00:00:00 2001 From: Daniel Rammer Date: Thu, 17 Sep 2026 21:18:40 -0500 Subject: [PATCH 1/2] feat(mem_wal): serve single-column and nested multi-match on the fresh tier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fresh tier served exactly one multi-match shape: a top-level query naming two or more distinct columns, which `fts_plan_shape` decomposes into one arm per leaf before any validation runs. Every other shape was refused. A multi-match naming a single distinct column — `["body"]`, or `["body", "body"]` with two boosts — took the `Bound` early return and was rejected whole by `validate_lsm_fts_query`; a multi-match nested in a boolean or boost was rejected by the memtable's query mapping at any depth. The base table scores all of them as `DisjunctionScorer(children, DisjunctionScore::Max)`, so a WAL-backed table answered with a hard error where its base-only twin answered correctly. Two changes make the fresh tier score every multi-match the way the compound scorer does. `fts_plan_shape` checks for a top-level multi-match before counting columns, so it always plans one arm per leaf: one per column is the cross-column case, several on one column is the same union collapsed to the best hit per row, and the two mix freely. The single-arm case already short-circuits the union. The row-documents-only rule moves with it — collapsing per primary key drops list elements however many columns the leaves name. `FtsQueryExpr` gains a `MultiMatch` node scored as the best child per document, the same contract `combine_boolean` and `combine_boost` apply for their clause algebra, so the node evaluates identically whether its leaves all come from one index or route to one index per column. The memtable's query mapping and granularity visitor map an index-level multi-match onto it, and the LSM validator applies its per-leaf rule to the leaves instead of refusing the shape. Tests: the evaluator keeps the best child rather than a SHOULD's sum; a single-column two-leaf multi-match plans two arms and scores each row exactly as the dominating leaf alone; a MUST over a cross-column multi-match with a MUST_NOT reaches the active memtable and excludes what it names; the memtable mapping test that asserted rejection now asserts the mapped tree. Co-Authored-By: Claude Fable 5.1 --- rust/lance/src/dataset/mem_wal/index/fts.rs | 126 ++++++- .../mem_wal/memtable/scanner/builder.rs | 39 ++- .../src/dataset/mem_wal/scanner/fts_search.rs | 315 +++++++++++++++--- 3 files changed, 416 insertions(+), 64 deletions(-) diff --git a/rust/lance/src/dataset/mem_wal/index/fts.rs b/rust/lance/src/dataset/mem_wal/index/fts.rs index 5c1caa94583..06b5abf6c70 100644 --- a/rust/lance/src/dataset/mem_wal/index/fts.rs +++ b/rust/lance/src/dataset/mem_wal/index/fts.rs @@ -183,6 +183,15 @@ pub enum FtsQueryExpr { /// boost query identically. Scores may go negative. negative_boost: f32, }, + /// Disjunction scored by the best child: a document's score is the highest + /// among the children that match it (`DisjunctionScore::Max`), which is how + /// the compound scorer ranks a multi-match over committed data. The + /// children are the leaves of an index-level multi-match, each bound to its + /// own column, so a tree spanning fields still routes leaf by leaf. + MultiMatch { + /// The alternatives, scored independently. + children: Vec, + }, } /// Default maximum number of fuzzy expansions. @@ -383,9 +392,9 @@ impl FtsQueryExpr { max_expansions, boost, }, - // Boolean and Boost don't carry a top-level boost field today. + // Compound nodes don't carry a top-level boost field today. // Preserved as-is to keep behavior identical to the previous impl. - other @ (Self::Boolean { .. } | Self::Boost { .. }) => other, + other @ (Self::Boolean { .. } | Self::Boost { .. } | Self::MultiMatch { .. }) => other, } } @@ -428,7 +437,7 @@ impl FtsQueryExpr { max_expansions, boost, }, - other @ (Self::Boolean { .. } | Self::Boost { .. }) => other, + other @ (Self::Boolean { .. } | Self::Boost { .. } | Self::MultiMatch { .. }) => other, } } @@ -439,7 +448,7 @@ impl FtsQueryExpr { Self::Match { column, .. } | Self::Phrase { column, .. } | Self::Fuzzy { column, .. } => column.as_deref(), - Self::Boolean { .. } | Self::Boost { .. } => None, + Self::Boolean { .. } | Self::Boost { .. } | Self::MultiMatch { .. } => None, } } @@ -465,6 +474,9 @@ impl FtsQueryExpr { negative: negative.map(|n| Box::new(n.bind_unbound_leaves(column))), negative_boost, }, + Self::MultiMatch { children } => Self::MultiMatch { + children: bind_all(children, column), + }, leaf if leaf.column().is_some() => leaf, leaf => leaf.with_column(column), } @@ -489,6 +501,7 @@ impl FtsQueryExpr { positive.has_unbound_leaf() || negative.as_ref().is_some_and(|n| n.has_unbound_leaf()) } + Self::MultiMatch { children } => children.iter().any(Self::has_unbound_leaf), leaf => leaf.column().is_none(), } } @@ -515,6 +528,11 @@ impl FtsQueryExpr { visit(negative, out); } } + FtsQueryExpr::MultiMatch { children } => { + for child in children { + visit(child, out); + } + } leaf => { if let Some(column) = leaf.column() && !out.contains(&column) @@ -2481,8 +2499,9 @@ impl FtsMemIndex { } /// `limit` is the caller's top-k, threaded down so a top-level `Match` - /// leaf can prune with WAND. Compound branches (`Boolean`/`Boost`) need - /// their children's full result sets, so they pass `None` downward. + /// leaf can prune with WAND. Compound branches (`Boolean`/`Boost`/ + /// `MultiMatch`) need their children's full result sets, so they pass + /// `None` downward. /// `include_tail` selects read-your-writes vs immutable-only (see /// [`SearchOptions::include_tail`]) and is threaded uniformly to every leaf. fn search_query_with_state( @@ -2494,7 +2513,9 @@ impl FtsMemIndex { tail_skip: bool, ) -> Vec { match query { - FtsQueryExpr::Boolean { .. } | FtsQueryExpr::Boost { .. } => { + FtsQueryExpr::Boolean { .. } + | FtsQueryExpr::Boost { .. } + | FtsQueryExpr::MultiMatch { .. } => { // Every leaf of this subtree searches this index, so the leaf // evaluator ignores the binding and keeps the one snapshot. combine_compound(query, &|leaf| { @@ -2560,7 +2581,9 @@ impl FtsMemIndex { } // `combine_compound` routes compound nodes itself and only ever // hands a leaf here. - FtsQueryExpr::Boolean { .. } | FtsQueryExpr::Boost { .. } => Vec::new(), + FtsQueryExpr::Boolean { .. } + | FtsQueryExpr::Boost { .. } + | FtsQueryExpr::MultiMatch { .. } => Vec::new(), } } @@ -3395,8 +3418,9 @@ fn relaxed_score_threshold(anchor: f32, factor: f32) -> f32 { /// /// The clause algebra is the contract the committed compound scorer applies: /// MUST intersects and sums (`RequiredConjunctionScorer`), SHOULD sums into the -/// surviving set (`DisjunctionScore::Sum`), MUST_NOT excludes, and a boost -/// subtracts `negative_boost * negative_score`. It is the same whether the +/// surviving set (`DisjunctionScore::Sum`), MUST_NOT excludes, a boost +/// subtracts `negative_boost * negative_score`, and a multi-match keeps each +/// document's best child (`DisjunctionScore::Max`). It is the same whether the /// leaves all come from one index or from one index per column — only /// `eval_leaf` differs. fn combine_compound(expr: &FtsQueryExpr, eval_leaf: &F) -> Vec @@ -3414,10 +3438,34 @@ where negative, negative_boost, } => combine_boost(positive, negative.as_deref(), *negative_boost, eval_leaf), + FtsQueryExpr::MultiMatch { children } => combine_multi_match(children, eval_leaf), leaf => eval_leaf(leaf), } } +fn combine_multi_match(children: &[FtsQueryExpr], eval_leaf: &F) -> Vec +where + F: Fn(&FtsQueryExpr) -> Vec, +{ + // A document matching several children is in several result sets and + // comes back once, at the highest of its scores. + let mut best: HashMap = HashMap::new(); + for child in children { + for entry in combine_compound(child, eval_leaf) { + best.entry(entry.key()) + .and_modify(|score| *score = score.max(entry.score)) + .or_insert(entry.score); + } + } + best.into_iter() + .map(|(key, score)| FtsEntry { + row_position: key.row_position, + doc_index: public_doc_index(&key.doc_index), + score, + }) + .collect() +} + fn combine_boost( positive: &FtsQueryExpr, negative: Option<&FtsQueryExpr>, @@ -5807,6 +5855,64 @@ mod tests { assert!(positions.contains(&3)); } + /// A multi-match keeps each document's best child — the compound scorer's + /// `DisjunctionScore::Max` — rather than the sum a SHOULD would give. + #[test] + fn test_multi_match_scores_best_child() { + let schema = create_test_schema(); + let index = FtsMemIndex::new(1, "description".to_string()); + index + .insert(&create_boolean_test_batch(&schema), 0) + .unwrap(); + + let rust = FtsQueryExpr::match_query("rust"); + let programming = FtsQueryExpr::match_query("programming").with_boost(3.0); + // Row 0 matches both children; row 2 matches `rust` alone. + let rust_at_0 = index + .search_query(&rust) + .into_iter() + .find(|entry| entry.row_position == 0) + .unwrap() + .score; + let programming_at_0 = index + .search_query(&programming) + .into_iter() + .find(|entry| entry.row_position == 0) + .unwrap() + .score; + let rust_at_2 = index + .search_query(&rust) + .into_iter() + .find(|entry| entry.row_position == 2) + .unwrap() + .score; + + let query = FtsQueryExpr::MultiMatch { + children: vec![rust, programming], + }; + let entries = index.search_query(&query); + let mut positions: Vec<_> = entries.iter().map(|e| e.row_position).collect(); + positions.sort_unstable(); + assert_eq!( + positions, + vec![0, 1, 2, 4], + "every child's matches, each row once" + ); + let at = |row| { + entries + .iter() + .find(|e| e.row_position == row) + .unwrap() + .score + }; + assert!( + (at(0) - rust_at_0.max(programming_at_0)).abs() < 1e-6, + "best child, not the sum: {} vs {rust_at_0} / {programming_at_0}", + at(0) + ); + assert!((at(2) - rust_at_2).abs() < 1e-6); + } + #[test] fn test_boolean_must_not_only() { let schema = create_test_schema(); diff --git a/rust/lance/src/dataset/mem_wal/memtable/scanner/builder.rs b/rust/lance/src/dataset/mem_wal/memtable/scanner/builder.rs index cb4f9b8dc79..fc67a4e17ee 100644 --- a/rust/lance/src/dataset/mem_wal/memtable/scanner/builder.rs +++ b/rust/lance/src/dataset/mem_wal/memtable/scanner/builder.rs @@ -347,10 +347,11 @@ fn requested_document_granularity(query: &IndexFtsQuery) -> Result { - return Err(Error::not_supported( - "MemTable full-text search does not support multi-match queries".to_string(), - )); + IndexFtsQuery::MultiMatch(m) => { + for leaf in &m.match_queries { + visit(&IndexFtsQuery::Match(leaf.clone()), current)?; + } + return Ok(()); } }; match (*current, requested) { @@ -428,11 +429,13 @@ fn to_local_expr(query: &IndexFtsQuery) -> Result { } builder.build() } - IndexFtsQuery::MultiMatch(_) => { - return Err(Error::not_supported( - "MemTable full-text search does not support multi-match queries".to_string(), - )); - } + IndexFtsQuery::MultiMatch(m) => FtsQueryExpr::MultiMatch { + children: m + .match_queries + .iter() + .map(|leaf| to_local_expr(&IndexFtsQuery::Match(leaf.clone()))) + .collect::>()?, + }, }) } @@ -2096,8 +2099,8 @@ mod tests { "nesting flattened: {must:?}" ); - // Multi-match spans columns -> still refused; the memtable holds one - // inverted index per column. + // Multi-match maps to a best-child node whose leaves keep their own + // columns, so a tree spanning fields still routes leaf by leaf. let multi = FullTextSearchQuery::new_query(IndexFtsQuery::MultiMatch( MultiMatchQuery::try_new( "x".to_string(), @@ -2105,10 +2108,18 @@ mod tests { ) .unwrap(), )); - assert!( - local_fts_query(multi, None).is_err(), - "multi-match must be rejected" + let local = local_fts_query(multi, None).unwrap(); + let FtsQueryExpr::MultiMatch { children } = &local.expr else { + panic!("expected a MultiMatch expr, got {:?}", local.expr); + }; + assert_eq!( + children + .iter() + .map(|child| child.column()) + .collect::>(), + [Some("text"), Some("other")] ); + assert_eq!(local.columns(), ["text", "other"]); // Missing column -> error. let no_col = FullTextSearchQuery::new("hi".to_string()); diff --git a/rust/lance/src/dataset/mem_wal/scanner/fts_search.rs b/rust/lance/src/dataset/mem_wal/scanner/fts_search.rs index 8092d1b0329..bec4935112c 100644 --- a/rust/lance/src/dataset/mem_wal/scanner/fts_search.rs +++ b/rust/lance/src/dataset/mem_wal/scanner/fts_search.rs @@ -240,12 +240,12 @@ fn validate_lsm_fts_query(query: &FullTextSearchQuery) -> Result<()> { } Ok(()) } - IndexFtsQuery::MultiMatch(_) => Err(Error::not_supported( - "LSM full-text search does not support multi-match queries: the memtable \ - holds one inverted index per column, so a cross-column query has no single \ - index to search" - .to_string(), - )), + IndexFtsQuery::MultiMatch(m) => { + for leaf in &m.match_queries { + visit(&IndexFtsQuery::Match(leaf.clone()))?; + } + Ok(()) + } } } visit(&query.query) @@ -379,20 +379,46 @@ enum FtsPlanShape { /// One predicate, evaluated whole by every source against these columns. /// Usually one; several when the tree's leaves name different fields. Bound(Vec), - /// A top-level multi-match: independent per-column searches, unioned and - /// collapsed to the best field per row. + /// A top-level multi-match: one independent search per leaf, unioned and + /// collapsed to the best hit per row. PerColumn(Vec<(String, IndexFtsQuery)>), } /// Decide how `query` reaches the columns it names. /// -/// A top-level multi-match decomposes: its leaves are independent per-column -/// matches, which is exactly what the base-table path scores separately before -/// taking the best per row. Every other shape spanning columns is *one* -/// predicate over several fields — `must: [a in title, b in body]` is a -/// conjunction, not a union of per-column results — so it stays whole and each -/// source evaluates it across all of them. +/// A top-level multi-match decomposes: its leaves are independent matches — +/// usually one per column, but a column may carry several — which is exactly +/// what the base-table path scores separately before taking the best per row. +/// Every other shape spanning columns is *one* predicate over several fields — +/// `must: [a in title, b in body]` is a conjunction, not a union of per-column +/// results — so it stays whole and each source evaluates it across all of them. fn fts_plan_shape(query: &IndexFtsQuery) -> Result { + if let IndexFtsQuery::MultiMatch(multi) = query { + // Collapsing the arms to one row per primary key drops list elements, + // however many columns the leaves name. + if requested_query_document_granularity(query)? + .is_some_and(|granularity| granularity.is_list_element()) + { + return Err(Error::not_supported( + "multi-match full-text search supports row documents only, not list elements" + .to_string(), + )); + } + return multi + .match_queries + .iter() + .map(|leaf| { + let column = leaf.column.clone().ok_or_else(|| { + Error::invalid_input( + "multi-match leaf has no bound column; they are bound at construction" + .to_string(), + ) + })?; + Ok((column, IndexFtsQuery::Match(leaf.clone()))) + }) + .collect::>>() + .map(FtsPlanShape::PerColumn); + } let columns = collect_query_columns(query); if columns.len() <= 1 { return Ok(FtsPlanShape::Bound(columns)); @@ -408,23 +434,7 @@ fn fts_plan_shape(query: &IndexFtsQuery) -> Result { .to_string(), )); } - let IndexFtsQuery::MultiMatch(multi) = query else { - return Ok(FtsPlanShape::Bound(columns)); - }; - multi - .match_queries - .iter() - .map(|leaf| { - let column = leaf.column.clone().ok_or_else(|| { - Error::invalid_input( - "multi-match leaf has no bound column; they are bound at construction" - .to_string(), - ) - })?; - Ok((column, IndexFtsQuery::Match(leaf.clone()))) - }) - .collect::>>() - .map(FtsPlanShape::PerColumn) + Ok(FtsPlanShape::Bound(columns)) } /// The columns `query` names, in tree order and deduplicated. @@ -615,16 +625,16 @@ impl LsmFtsSearchPlanner { )); } - // One single-column plan per field, unioned and collapsed. Each field is - // scored independently and a row takes its best field's score, which is - // what the base-table path does for a cross-column MultiMatch - // (`DisjunctionScore::Max`). Reusing the single-column planner per field + // One single-column plan per leaf, unioned and collapsed. Each leaf is + // scored independently and a row takes its best leaf's score, which is + // what the base-table path does for a MultiMatch + // (`DisjunctionScore::Max`). Reusing the single-column planner per leaf // keeps every per-source behavior — granularity resolution, prefilter, // the cross-generation block-list — identical to a single-column search, - // at the cost of one pass over the sources per field. + // at the cost of one pass over the sources per leaf. // - // Each arm is cut at `k * fields` rather than `k`: the final top-k is - // over *rows*, and a row can occupy up to one slot per field, so a + // Each arm is cut at `k * leaves` rather than `k`: the final top-k is + // over *rows*, and a row can occupy up to one slot per leaf, so a // tighter per-arm cut could leave fewer than `k` rows after the collapse // even when more matching rows exist. let candidate_limit = limit.map(|k| k.saturating_mul(per_column.len().max(1))); @@ -1204,8 +1214,8 @@ mod tests { use crate::dataset::{Dataset, WriteParams}; use arrow_array::builder::{ListBuilder, StringBuilder}; use arrow_array::{ - Array, BooleanArray, Int32Array, ListArray, RecordBatch, RecordBatchIterator, StringArray, - UInt32Array, + Array, BooleanArray, Float32Array, Int32Array, ListArray, RecordBatch, RecordBatchIterator, + StringArray, UInt32Array, }; use arrow_schema::{DataType, Field, Schema as ArrowSchema}; use futures::TryStreamExt; @@ -2935,6 +2945,231 @@ mod tests { assert_eq!(batches.iter().map(|b| b.num_rows()).sum::(), 0); } + /// A multi-match decomposes per leaf whatever columns its leaves name — a + /// column may carry several — rather than taking the single-column bound + /// path, which evaluates the query whole. + #[test] + fn multi_match_plans_one_arm_per_leaf_even_on_one_column() { + use lance_index::scalar::inverted::query::MultiMatchQuery; + + let multi = MultiMatchQuery::try_new( + "lance".to_string(), + vec!["text".to_string(), "text".to_string()], + ) + .unwrap() + .try_with_boosts(vec![1.0, 2.0]) + .unwrap(); + let shape = fts_plan_shape(&IndexFtsQuery::MultiMatch(multi)).unwrap(); + let FtsPlanShape::PerColumn(arms) = shape else { + panic!("expected one arm per leaf"); + }; + assert_eq!(arms.len(), 2); + assert!(arms.iter().all(|(column, _)| column == "text")); + } + + /// Two leaves on one column plan as two arms whose hits collapse to the + /// best per row — the same rows and scores as the dominating leaf alone. + #[tokio::test] + async fn single_column_multi_match_scores_each_row_by_its_best_leaf() { + use lance_index::scalar::inverted::query::MultiMatchQuery; + + let schema = fts_schema(); + let batch_store = Arc::new(BatchStore::with_capacity(16)); + let mut indexes = IndexStore::new(); + indexes.enable_pk_index(&[("id".to_string(), 0)]); + indexes.add_fts("text_fts".to_string(), 1, "text".to_string()); + let active_batch = make_batch( + &schema, + &[1, 2, 3], + &["lance rocks", "lance lance", "nothing"], + ); + let (_, row_offset, batch_position) = batch_store.append(active_batch.clone()).unwrap(); + indexes + .insert_with_batch_position(&active_batch, row_offset, Some(batch_position)) + .unwrap(); + let indexes = Arc::new(indexes); + + let tmp = tempfile::tempdir().unwrap(); + let base_uri = format!("{}/base", tmp.path().to_str().unwrap()); + let collector = LsmDataSourceCollector::without_base_table(base_uri, vec![]) + .with_in_memory_memtables( + uuid::Uuid::new_v4(), + InMemoryMemTables { + active: InMemoryMemTableRef { + batch_store, + index_store: indexes, + schema: schema.clone(), + generation: 1, + }, + frozen: vec![], + }, + ); + let planner = LsmFtsSearchPlanner::new(collector, vec!["id".to_string()], schema); + let ctx = datafusion::prelude::SessionContext::new(); + let scores_for = |query: FullTextSearchQuery| { + let planner = &planner; + let ctx = &ctx; + async move { + let plan = planner + .plan_search(query, Some(10), Some(&["id".to_string()])) + .await + .expect("plans"); + let batches: Vec = plan + .execute(0, ctx.task_ctx()) + .unwrap() + .try_collect() + .await + .unwrap(); + let mut scores: Vec<(i32, f32)> = batches + .iter() + .flat_map(|batch| { + let ids = batch + .column_by_name("id") + .unwrap() + .as_any() + .downcast_ref::() + .unwrap(); + let scores = batch + .column_by_name(SCORE_COLUMN) + .unwrap() + .as_any() + .downcast_ref::() + .unwrap(); + (0..batch.num_rows()) + .map(|i| (ids.value(i), scores.value(i))) + .collect::>() + }) + .collect(); + scores.sort_by_key(|(id, _)| *id); + scores + } + }; + + let multi = MultiMatchQuery::try_new( + "lance".to_string(), + vec!["text".to_string(), "text".to_string()], + ) + .unwrap() + .try_with_boosts(vec![1.0, 2.0]) + .unwrap(); + let fused = scores_for(FullTextSearchQuery::new_query(IndexFtsQuery::MultiMatch( + multi, + ))) + .await; + let dominating = scores_for(FullTextSearchQuery::new_query(IndexFtsQuery::Match( + MatchQuery::new("lance".to_string()) + .with_column(Some("text".to_string())) + .with_boost(2.0), + ))) + .await; + + assert_eq!( + fused.iter().map(|(id, _)| *id).collect::>(), + vec![1, 2], + "each matching row once; id=3 matches nothing" + ); + assert_eq!(fused.len(), dominating.len()); + for ((id, fused_score), (_, dominating_score)) in fused.iter().zip(&dominating) { + assert!( + (fused_score - dominating_score).abs() < 1e-6, + "id={id}: best-leaf score {fused_score} != boost-2 leaf alone {dominating_score}" + ); + } + } + + /// A multi-match below the root is one clause of the enclosing tree. The + /// memtable scores it as a best-child node and routes each leaf to its own + /// column's index, so a MUST over it excludes what MUST_NOT names. + #[tokio::test] + async fn nested_multi_match_reaches_the_active_memtable() { + use lance_index::scalar::inverted::query::{BooleanQuery, MultiMatchQuery, Occur}; + + let schema = two_column_fts_schema(); + let batch_store = Arc::new(BatchStore::with_capacity(16)); + let mut indexes = IndexStore::new(); + indexes.enable_pk_index(&[("id".to_string(), 0)]); + indexes.add_fts("title_fts".to_string(), 1, "title".to_string()); + indexes.add_fts("body_fts".to_string(), 2, "body".to_string()); + let active_batch = make_two_column_batch( + &schema, + &[ + (1, "lance title", "unrelated body"), // title only + (2, "unrelated title", "lance body"), // body only + (3, "lance title", "lance body"), // both -> once + (4, "spam lance title", "lance body"), // excluded by MUST_NOT + (5, "nothing", "nothing"), // neither + ], + ); + let (_, row_offset, batch_position) = batch_store.append(active_batch.clone()).unwrap(); + indexes + .insert_with_batch_position(&active_batch, row_offset, Some(batch_position)) + .unwrap(); + let indexes = Arc::new(indexes); + + let tmp = tempfile::tempdir().unwrap(); + let base_uri = format!("{}/base", tmp.path().to_str().unwrap()); + let collector = LsmDataSourceCollector::without_base_table(base_uri, vec![]) + .with_in_memory_memtables( + uuid::Uuid::new_v4(), + InMemoryMemTables { + active: InMemoryMemTableRef { + batch_store, + index_store: indexes, + schema: schema.clone(), + generation: 1, + }, + frozen: vec![], + }, + ); + let planner = LsmFtsSearchPlanner::new(collector, vec!["id".to_string()], schema); + + let multi = IndexFtsQuery::MultiMatch( + MultiMatchQuery::try_new( + "lance".to_string(), + vec!["title".to_string(), "body".to_string()], + ) + .unwrap(), + ); + let spam = IndexFtsQuery::Match( + MatchQuery::new("spam".to_string()).with_column(Some("title".to_string())), + ); + let query = + FullTextSearchQuery::new_query(IndexFtsQuery::Boolean(BooleanQuery::new(vec![ + (Occur::Must, multi), + (Occur::MustNot, spam), + ]))); + let plan = planner + .plan_search(query, Some(10), Some(&["id".to_string()])) + .await + .expect("a boolean over a multi-match must plan"); + let ctx = datafusion::prelude::SessionContext::new(); + let batches: Vec = plan + .execute(0, ctx.task_ctx()) + .unwrap() + .try_collect() + .await + .unwrap(); + let mut ids: Vec = batches + .iter() + .flat_map(|batch| { + batch + .column_by_name("id") + .unwrap() + .as_any() + .downcast_ref::() + .unwrap() + .values() + .to_vec() + }) + .collect(); + ids.sort_unstable(); + assert_eq!( + ids, + vec![1, 2, 3], + "both columns searched through the nested multi-match, id=4 excluded, id=3 once" + ); + } + /// A multi-match reaches the active memtable across every queried column, /// and a row matching in more than one is returned once rather than per /// column. From 15356a874e6108a142afe22ba5a5bf10c763c5d2 Mon Sep 17 00:00:00 2001 From: Daniel Rammer Date: Fri, 18 Sep 2026 07:35:05 -0500 Subject: [PATCH 2/2] fix(mem_wal): keep a single-column multi-match on the bound FTS path `fts_plan_shape` sent every top-level multi-match to the per-column path, so a one-leaf `MultiMatchQuery("lance", ["text"])` hit the cross-column primary-key guard in `plan_search` and was refused on a MemWAL table without a primary key, though it has nothing to collapse. Route by the distinct columns the leaves name instead: one column stays `Bound`, where the memtable scores the new `FtsQueryExpr::MultiMatch` node and the dataset scanner keeps it on its compound scorer, the same rule `supports_compound_scorer` applies on the base table. Per-leaf arms are reserved for a multi-match that spans columns. Also drop the stale note on `validate_lsm_fts_query` claiming multi-match is refused. Co-Authored-By: Claude Fable 5.1 --- .../src/dataset/mem_wal/scanner/fts_search.rs | 112 ++++++++++++------ 1 file changed, 73 insertions(+), 39 deletions(-) diff --git a/rust/lance/src/dataset/mem_wal/scanner/fts_search.rs b/rust/lance/src/dataset/mem_wal/scanner/fts_search.rs index bec4935112c..c2e3487fdee 100644 --- a/rust/lance/src/dataset/mem_wal/scanner/fts_search.rs +++ b/rust/lance/src/dataset/mem_wal/scanner/fts_search.rs @@ -214,10 +214,9 @@ fn validate_source_document_granularities( /// Reject the query shapes the active memtable arm cannot evaluate. /// -/// Only two remain. A fuzzy Match cannot also require every term: the fuzzy -/// path expands each term independently and unions the expansions. And -/// multi-match spans columns, while the memtable holds one inverted index per -/// column, so there is no single index to search. +/// Only one remains: a fuzzy Match cannot also require every term, because the +/// fuzzy path expands each term independently and unions the expansions. A +/// multi-match is checked leaf by leaf under the same rule. fn validate_lsm_fts_query(query: &FullTextSearchQuery) -> Result<()> { fn visit(query: &IndexFtsQuery) -> Result<()> { match query { @@ -379,23 +378,29 @@ enum FtsPlanShape { /// One predicate, evaluated whole by every source against these columns. /// Usually one; several when the tree's leaves name different fields. Bound(Vec), - /// A top-level multi-match: one independent search per leaf, unioned and - /// collapsed to the best hit per row. + /// A top-level multi-match spanning columns: one independent search per + /// leaf, unioned and collapsed to the best hit per row. PerColumn(Vec<(String, IndexFtsQuery)>), } /// Decide how `query` reaches the columns it names. /// -/// A top-level multi-match decomposes: its leaves are independent matches — -/// usually one per column, but a column may carry several — which is exactly -/// what the base-table path scores separately before taking the best per row. -/// Every other shape spanning columns is *one* predicate over several fields — -/// `must: [a in title, b in body]` is a conjunction, not a union of per-column -/// results — so it stays whole and each source evaluates it across all of them. +/// A top-level multi-match spanning columns decomposes: its leaves are +/// independent matches — usually one per column, but a column may carry +/// several — which is exactly what the base-table path scores separately +/// before taking the best per row. Naming one column, it is a single-index +/// query like any other and stays bound: the memtable scores it as a +/// best-child node and the dataset scanner keeps it on its compound scorer, so +/// it needs no primary key to collapse by. Every other shape spanning columns +/// is *one* predicate over several fields — `must: [a in title, b in body]` is +/// a conjunction, not a union of per-column results — so it stays whole and +/// each source evaluates it across all of them. fn fts_plan_shape(query: &IndexFtsQuery) -> Result { + let columns = collect_query_columns(query); if let IndexFtsQuery::MultiMatch(multi) = query { - // Collapsing the arms to one row per primary key drops list elements, - // however many columns the leaves name. + // Row documents only, whatever the column count: the dataset scanner + // refuses element documents for any multi-match, and collapsing arms + // per primary key would drop elements anyway. if requested_query_document_granularity(query)? .is_some_and(|granularity| granularity.is_list_element()) { @@ -404,6 +409,9 @@ fn fts_plan_shape(query: &IndexFtsQuery) -> Result { .to_string(), )); } + if columns.len() <= 1 { + return Ok(FtsPlanShape::Bound(columns)); + } return multi .match_queries .iter() @@ -419,7 +427,6 @@ fn fts_plan_shape(query: &IndexFtsQuery) -> Result { .collect::>>() .map(FtsPlanShape::PerColumn); } - let columns = collect_query_columns(query); if columns.len() <= 1 { return Ok(FtsPlanShape::Bound(columns)); } @@ -2168,9 +2175,18 @@ mod tests { ); } + /// A single-column multi-match takes the same bound path as a plain match: + /// without a primary key there is nothing to collapse by, and nothing that + /// needs collapsing. + #[rstest::rstest] + #[case::match_query(false)] + #[case::single_column_multi_match(true)] #[tokio::test] - async fn active_filtered_search_without_pk_applies_small_limit_after_filter() { + async fn active_filtered_search_without_pk_applies_small_limit_after_filter( + #[case] multi_match: bool, + ) { use datafusion::prelude::{col, lit}; + use lance_index::scalar::inverted::query::MultiMatchQuery; let schema = fts_schema(); let batch_store = Arc::new(BatchStore::with_capacity(16)); @@ -2205,14 +2221,17 @@ mod tests { let planner = LsmFtsSearchPlanner::new(collector, vec![], schema) .with_filter(Some(col("id").gt_eq(lit(1i32)))); + let query = if multi_match { + FullTextSearchQuery::new_query(IndexFtsQuery::MultiMatch( + MultiMatchQuery::try_new("lance".to_string(), vec!["text".to_string()]).unwrap(), + )) + } else { + FullTextSearchQuery::new("lance".to_string()) + .with_column("text".to_string()) + .unwrap() + }; let plan = planner - .plan_search( - FullTextSearchQuery::new("lance".to_string()) - .with_column("text".to_string()) - .unwrap(), - Some(2), - None, - ) + .plan_search(query, Some(2), None) .await .expect("planner should produce an active-only filtered plan"); @@ -2945,30 +2964,45 @@ mod tests { assert_eq!(batches.iter().map(|b| b.num_rows()).sum::(), 0); } - /// A multi-match decomposes per leaf whatever columns its leaves name — a - /// column may carry several — rather than taking the single-column bound - /// path, which evaluates the query whole. - #[test] - fn multi_match_plans_one_arm_per_leaf_even_on_one_column() { + /// A multi-match decomposes per leaf only when its leaves span columns. + /// Naming one column — through however many leaves — it is a single-index + /// query and stays on the bound path, which needs no primary key. + #[rstest::rstest] + #[case::one_leaf(vec!["text"], false)] + #[case::two_leaves_one_column(vec!["text", "text"], false)] + #[case::one_leaf_per_column(vec!["title", "body"], true)] + #[case::mixed(vec!["title", "title", "body"], true)] + fn multi_match_decomposes_only_when_it_spans_columns( + #[case] leaves: Vec<&str>, + #[case] spans_columns: bool, + ) { use lance_index::scalar::inverted::query::MultiMatchQuery; let multi = MultiMatchQuery::try_new( "lance".to_string(), - vec!["text".to_string(), "text".to_string()], + leaves.iter().map(|leaf| leaf.to_string()).collect(), ) - .unwrap() - .try_with_boosts(vec![1.0, 2.0]) .unwrap(); - let shape = fts_plan_shape(&IndexFtsQuery::MultiMatch(multi)).unwrap(); - let FtsPlanShape::PerColumn(arms) = shape else { - panic!("expected one arm per leaf"); - }; - assert_eq!(arms.len(), 2); - assert!(arms.iter().all(|(column, _)| column == "text")); + match fts_plan_shape(&IndexFtsQuery::MultiMatch(multi)).unwrap() { + FtsPlanShape::Bound(columns) => { + assert!(!spans_columns, "expected one arm per leaf for {leaves:?}"); + assert_eq!(columns, vec![leaves[0].to_string()]); + } + FtsPlanShape::PerColumn(arms) => { + assert!(spans_columns, "expected the bound path for {leaves:?}"); + assert_eq!( + arms.iter() + .map(|(column, _)| column.as_str()) + .collect::>(), + leaves + ); + } + } } - /// Two leaves on one column plan as two arms whose hits collapse to the - /// best per row — the same rows and scores as the dominating leaf alone. + /// Two leaves on one column evaluate whole on the bound path, each row + /// scored by its best leaf — the same rows and scores as the dominating + /// leaf alone. #[tokio::test] async fn single_column_multi_match_scores_each_row_by_its_best_leaf() { use lance_index::scalar::inverted::query::MultiMatchQuery;