feat: support structured_query in the directory namespace query_table#7592
feat: support structured_query in the directory namespace query_table#7592hamersaw wants to merge 2 commits into
Conversation
The directory namespace's query_table only handled string_query; a structured_query was silently ignored, so the scan ran without any full-text filter and returned all rows. Map the namespace structured FtsQuery model into the engine FtsQuery for all variants — match, phrase, multi_match, and (recursively) boolean and boost — and run it via FullTextSearchQuery::new_query, mirroring the mapping the JNI scanner already performs. An FtsQuery with no variant set is now rejected rather than silently ignored. string_query behavior is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
A-namespace
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
wjones127
left a comment
wjones127
left a comment
There was a problem hiding this comment.
Mostly just want a little more test coverage.
| fn build_engine_match_query( | ||
| m: &lance_namespace::models::MatchQuery, | ||
| ) -> std::result::Result<MatchQuery, NamespaceError> { |
There was a problem hiding this comment.
suggestion: Should build_engine_match_query really by a TryFrom implementation instead?
There was a problem hiding this comment.
Unfortunately the same issue as the other, where TryFrom can't operate on foreign types.
| fn build_engine_fts_query( | ||
| query: &lance_namespace::models::FtsQuery, | ||
| ) -> std::result::Result<FtsQuery, NamespaceError> { |
There was a problem hiding this comment.
suggestion: should build_engine_fts_query be a TryFrom impl?
There was a problem hiding this comment.
I tried to do this, but TIL TryFrom is a foreign trait so it can't be used on types that are foreign to this crate. The source (ie. lance_namespace::models::FtsQuery) and the target (ie. lance_index::scalar::inverted::query::FtsQuery) both live elsewhere, so it won't compile with TryFrom. The options would be to define TryFrom on these types in either the lance_namespace or lance_index crates, but that feels like the wrong approach?
| .map(build_engine_match_query) | ||
| .collect::<std::result::Result<Vec<_>, _>>()?; | ||
| Ok(FtsQuery::MultiMatch(MultiMatchQuery { match_queries })) | ||
| } else if let Some(ref b) = query.boolean { |
There was a problem hiding this comment.
issue(blocking): could you add test coverage for the phrase / boolean / boost query roundtrip? Looks like you just have match query.
There was a problem hiding this comment.
Great call, added round trip tests for all of the FtsQuery variants.
Extends the structured_query mapping tests beyond the match variant to exercise every FtsQuery variant build_engine_fts_query maps. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughChangesStructured full-text search
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant query_table
participant build_engine_fts_query
participant Scanner
Client->>query_table: submit structured_query
query_table->>build_engine_fts_query: translate query
build_engine_fts_query-->>query_table: return engine FtsQuery
query_table->>Scanner: apply FtsQuery
Scanner-->>Client: return query results
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rust/lance-namespace-impls/src/dir.rs (1)
4886-4917: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winValidate
string_query/structured_querymutual exclusivity; dedupe the apply-to-scanner call.If a caller sets both
fts_query.string_queryandfts_query.structured_query, this silently applies onlystring_queryand dropsstructured_querywith no error. Theelse ifalso duplicates thescanner.full_text_search(fts).map_err(...)block from theifbranch.🛠️ Proposed fix: reject ambiguous input and apply once
if let Some(ref fts_query) = request.full_text_query { + if fts_query.string_query.is_some() && fts_query.structured_query.is_some() { + return Err(NamespaceError::InvalidInput { + message: "full_text_query must set exactly one of string_query or structured_query".to_string(), + } + .into()); + } // Handle string_query (simple string FTS) - if let Some(ref string_query) = fts_query.string_query { + let fts = if let Some(ref string_query) = fts_query.string_query { let mut fts = FullTextSearchQuery::new(string_query.query.clone()); if let Some(ref columns) = string_query.columns && !columns.is_empty() { fts = fts .with_columns(columns) .map_err(|e| NamespaceError::InvalidInput { message: format!("Invalid FTS columns: {:?}", e), })?; } - scanner - .full_text_search(fts) - .map_err(|e| NamespaceError::InvalidInput { - message: format!("Invalid full text search: {:?}", e), - })?; + Some(fts) } else if let Some(ref structured_query) = fts_query.structured_query { - // Structured FTS: map the namespace query model into the engine FtsQuery. let engine_query = build_engine_fts_query(&structured_query.query)?; - let fts = FullTextSearchQuery::new_query(engine_query); + Some(FullTextSearchQuery::new_query(engine_query)) + } else { + None + }; + if let Some(fts) = fts { scanner .full_text_search(fts) .map_err(|e| NamespaceError::InvalidInput { message: format!("Invalid full text search: {:?}", e), })?; } }As per coding guidelines, "Validate mutually exclusive builder or configuration options and report a clear error when both are set" and "Extract logic repeated in two or more places into a shared helper."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/lance-namespace-impls/src/dir.rs` around lines 4886 - 4917, Update the `fts_query` handling to reject requests where both `string_query` and `structured_query` are set, returning a clear `NamespaceError::InvalidInput`; otherwise build one `FullTextSearchQuery` from the selected variant and call `scanner.full_text_search(...).map_err(...)` once after the branching logic. Preserve the existing column validation and structured-query conversion behavior.Source: Coding guidelines
🧹 Nitpick comments (1)
rust/lance-namespace-impls/src/dir.rs (1)
5332-5552: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the FTS query mapping (+ its tests) into a dedicated submodule.
dir.rsis already ~13,900 lines.build_engine_fts_query/build_engine_match_queryand their tests form a self-contained unit with minimal coupling to the rest of the file and would fit well in their own module (e.g.dir/fts_query.rs).As per coding guidelines, "Extract substantial new logic such as bin packing or scheduling into dedicated submodules instead of inlining it into large files."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/lance-namespace-impls/src/dir.rs` around lines 5332 - 5552, Extract build_engine_fts_query, build_engine_match_query, and their associated tests into a dedicated fts_query submodule under dir; update module declarations and imports so dir.rs continues using the mapping functions without changing behavior or test coverage.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rust/lance-namespace-impls/src/dir.rs`:
- Around line 5335-5383: Update build_engine_fts_query to validate that exactly
one of match, phrase, multi_match, boolean, or boost is set before constructing
the engine query. Reject inputs with multiple variants using the existing
NamespaceError::InvalidInput path and a clear message, while preserving the
current conversion logic for valid single-variant queries and the existing error
for none set.
- Around line 5547-5552: Strengthen
test_build_engine_fts_query_requires_a_variant by matching the returned error as
NamespaceError::InvalidInput and asserting its message contains the expected
missing-variant text. Replace the is_err-only assertion while preserving the
existing empty FtsQuery setup.
- Around line 5340-5348: Validate signed namespace-model fields before
converting them in the query conversion logic around PhraseQuery and the
fuzzy-query construction: reject negative slop, fuzziness, max_expansions, and
prefix_length with descriptive errors, then perform the existing unsigned
conversions only after validation. Confirm the actual field types in
lance_namespace::models and preserve valid-value behavior while preventing
negative inputs from wrapping.
---
Outside diff comments:
In `@rust/lance-namespace-impls/src/dir.rs`:
- Around line 4886-4917: Update the `fts_query` handling to reject requests
where both `string_query` and `structured_query` are set, returning a clear
`NamespaceError::InvalidInput`; otherwise build one `FullTextSearchQuery` from
the selected variant and call `scanner.full_text_search(...).map_err(...)` once
after the branching logic. Preserve the existing column validation and
structured-query conversion behavior.
---
Nitpick comments:
In `@rust/lance-namespace-impls/src/dir.rs`:
- Around line 5332-5552: Extract build_engine_fts_query,
build_engine_match_query, and their associated tests into a dedicated fts_query
submodule under dir; update module declarations and imports so dir.rs continues
using the mapping functions without changing behavior or test coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5c856ff0-ac42-4923-8779-f6683f73e4e8
📒 Files selected for processing (1)
rust/lance-namespace-impls/src/dir.rs
| fn build_engine_fts_query( | ||
| query: &lance_namespace::models::FtsQuery, | ||
| ) -> std::result::Result<FtsQuery, NamespaceError> { | ||
| if let Some(ref m) = query.r#match { | ||
| Ok(FtsQuery::Match(build_engine_match_query(m)?)) | ||
| } else if let Some(ref p) = query.phrase { | ||
| let mut phrase = PhraseQuery::new(p.terms.clone()); | ||
| if let Some(ref column) = p.column { | ||
| phrase = phrase.with_column(Some(column.clone())); | ||
| } | ||
| if let Some(slop) = p.slop { | ||
| phrase = phrase.with_slop(slop as u32); | ||
| } | ||
| Ok(FtsQuery::Phrase(phrase)) | ||
| } else if let Some(ref mm) = query.multi_match { | ||
| let match_queries = mm | ||
| .match_queries | ||
| .iter() | ||
| .map(build_engine_match_query) | ||
| .collect::<std::result::Result<Vec<_>, _>>()?; | ||
| Ok(FtsQuery::MultiMatch(MultiMatchQuery { match_queries })) | ||
| } else if let Some(ref b) = query.boolean { | ||
| let mut clauses: Vec<(Occur, FtsQuery)> = Vec::new(); | ||
| for clause in &b.must { | ||
| clauses.push((Occur::Must, build_engine_fts_query(clause)?)); | ||
| } | ||
| for clause in &b.should { | ||
| clauses.push((Occur::Should, build_engine_fts_query(clause)?)); | ||
| } | ||
| for clause in &b.must_not { | ||
| clauses.push((Occur::MustNot, build_engine_fts_query(clause)?)); | ||
| } | ||
| Ok(FtsQuery::Boolean(BooleanQuery::new(clauses))) | ||
| } else if let Some(ref boost) = query.boost { | ||
| let positive = build_engine_fts_query(&boost.positive)?; | ||
| let negative = build_engine_fts_query(&boost.negative)?; | ||
| Ok(FtsQuery::Boost(BoostQuery::new( | ||
| positive, | ||
| negative, | ||
| boost.negative_boost, | ||
| ))) | ||
| } else { | ||
| Err(NamespaceError::InvalidInput { | ||
| message: "structured_query.query must set exactly one of match, phrase, multi_match, \ | ||
| boolean, or boost" | ||
| .to_string(), | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
build_engine_fts_query silently picks a variant instead of rejecting ambiguous input.
The if/else if chain over match/phrase/multi_match/boolean/boost only rejects the "none set" case (line 5376-5382). If a caller sets more than one variant (e.g. both match and phrase), the function silently uses whichever is checked first and drops the rest — even though the error message on line 5378-5380 states "must set exactly one of...". This contradicts the stated contract and can silently produce a query that doesn't match what the caller intended.
🛠️ Proposed fix: count set variants and reject >1
fn build_engine_fts_query(
query: &lance_namespace::models::FtsQuery,
) -> std::result::Result<FtsQuery, NamespaceError> {
+ let set_count = [
+ query.r#match.is_some(),
+ query.phrase.is_some(),
+ query.multi_match.is_some(),
+ query.boolean.is_some(),
+ query.boost.is_some(),
+ ]
+ .into_iter()
+ .filter(|set| *set)
+ .count();
+ if set_count > 1 {
+ return Err(NamespaceError::InvalidInput {
+ message: "structured_query.query must set exactly one of match, phrase, multi_match, \
+ boolean, or boost, but multiple were set"
+ .to_string(),
+ });
+ }
if let Some(ref m) = query.r#match {As per coding guidelines, "Validate mutually exclusive builder or configuration options and report a clear error when both are set."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn build_engine_fts_query( | |
| query: &lance_namespace::models::FtsQuery, | |
| ) -> std::result::Result<FtsQuery, NamespaceError> { | |
| if let Some(ref m) = query.r#match { | |
| Ok(FtsQuery::Match(build_engine_match_query(m)?)) | |
| } else if let Some(ref p) = query.phrase { | |
| let mut phrase = PhraseQuery::new(p.terms.clone()); | |
| if let Some(ref column) = p.column { | |
| phrase = phrase.with_column(Some(column.clone())); | |
| } | |
| if let Some(slop) = p.slop { | |
| phrase = phrase.with_slop(slop as u32); | |
| } | |
| Ok(FtsQuery::Phrase(phrase)) | |
| } else if let Some(ref mm) = query.multi_match { | |
| let match_queries = mm | |
| .match_queries | |
| .iter() | |
| .map(build_engine_match_query) | |
| .collect::<std::result::Result<Vec<_>, _>>()?; | |
| Ok(FtsQuery::MultiMatch(MultiMatchQuery { match_queries })) | |
| } else if let Some(ref b) = query.boolean { | |
| let mut clauses: Vec<(Occur, FtsQuery)> = Vec::new(); | |
| for clause in &b.must { | |
| clauses.push((Occur::Must, build_engine_fts_query(clause)?)); | |
| } | |
| for clause in &b.should { | |
| clauses.push((Occur::Should, build_engine_fts_query(clause)?)); | |
| } | |
| for clause in &b.must_not { | |
| clauses.push((Occur::MustNot, build_engine_fts_query(clause)?)); | |
| } | |
| Ok(FtsQuery::Boolean(BooleanQuery::new(clauses))) | |
| } else if let Some(ref boost) = query.boost { | |
| let positive = build_engine_fts_query(&boost.positive)?; | |
| let negative = build_engine_fts_query(&boost.negative)?; | |
| Ok(FtsQuery::Boost(BoostQuery::new( | |
| positive, | |
| negative, | |
| boost.negative_boost, | |
| ))) | |
| } else { | |
| Err(NamespaceError::InvalidInput { | |
| message: "structured_query.query must set exactly one of match, phrase, multi_match, \ | |
| boolean, or boost" | |
| .to_string(), | |
| }) | |
| } | |
| } | |
| fn build_engine_fts_query( | |
| query: &lance_namespace::models::FtsQuery, | |
| ) -> std::result::Result<FtsQuery, NamespaceError> { | |
| let set_count = [ | |
| query.r#match.is_some(), | |
| query.phrase.is_some(), | |
| query.multi_match.is_some(), | |
| query.boolean.is_some(), | |
| query.boost.is_some(), | |
| ] | |
| .into_iter() | |
| .filter(|set| *set) | |
| .count(); | |
| if set_count > 1 { | |
| return Err(NamespaceError::InvalidInput { | |
| message: "structured_query.query must set exactly one of match, phrase, multi_match, \ | |
| boolean, or boost, but multiple were set" | |
| .to_string(), | |
| }); | |
| } | |
| if let Some(ref m) = query.r#match { | |
| Ok(FtsQuery::Match(build_engine_match_query(m)?)) | |
| } else if let Some(ref p) = query.phrase { | |
| let mut phrase = PhraseQuery::new(p.terms.clone()); | |
| if let Some(ref column) = p.column { | |
| phrase = phrase.with_column(Some(column.clone())); | |
| } | |
| if let Some(slop) = p.slop { | |
| phrase = phrase.with_slop(slop as u32); | |
| } | |
| Ok(FtsQuery::Phrase(phrase)) | |
| } else if let Some(ref mm) = query.multi_match { | |
| let match_queries = mm | |
| .match_queries | |
| .iter() | |
| .map(build_engine_match_query) | |
| .collect::<std::result::Result<Vec<_>, _>>()?; | |
| Ok(FtsQuery::MultiMatch(MultiMatchQuery { match_queries })) | |
| } else if let Some(ref b) = query.boolean { | |
| let mut clauses: Vec<(Occur, FtsQuery)> = Vec::new(); | |
| for clause in &b.must { | |
| clauses.push((Occur::Must, build_engine_fts_query(clause)?)); | |
| } | |
| for clause in &b.should { | |
| clauses.push((Occur::Should, build_engine_fts_query(clause)?)); | |
| } | |
| for clause in &b.must_not { | |
| clauses.push((Occur::MustNot, build_engine_fts_query(clause)?)); | |
| } | |
| Ok(FtsQuery::Boolean(BooleanQuery::new(clauses))) | |
| } else if let Some(ref boost) = query.boost { | |
| let positive = build_engine_fts_query(&boost.positive)?; | |
| let negative = build_engine_fts_query(&boost.negative)?; | |
| Ok(FtsQuery::Boost(BoostQuery::new( | |
| positive, | |
| negative, | |
| boost.negative_boost, | |
| ))) | |
| } else { | |
| Err(NamespaceError::InvalidInput { | |
| message: "structured_query.query must set exactly one of match, phrase, multi_match, \ | |
| boolean, or boost" | |
| .to_string(), | |
| }) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rust/lance-namespace-impls/src/dir.rs` around lines 5335 - 5383, Update
build_engine_fts_query to validate that exactly one of match, phrase,
multi_match, boolean, or boost is set before constructing the engine query.
Reject inputs with multiple variants using the existing
NamespaceError::InvalidInput path and a clear message, while preserving the
current conversion logic for valid single-variant queries and the existing error
for none set.
Source: Coding guidelines
| } else if let Some(ref p) = query.phrase { | ||
| let mut phrase = PhraseQuery::new(p.terms.clone()); | ||
| if let Some(ref column) = p.column { | ||
| phrase = phrase.with_column(Some(column.clone())); | ||
| } | ||
| if let Some(slop) = p.slop { | ||
| phrase = phrase.with_slop(slop as u32); | ||
| } | ||
| Ok(FtsQuery::Phrase(phrase)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Unchecked signed→unsigned casts on slop/fuzziness/max_expansions/prefix_length.
p.slop as u32 (line 5346), fuzziness as u32 (line 5396), max_expansions as usize (line 5399), and prefix_length as u32 (line 5409) all cast without checking for negative input first. If these namespace-model fields are signed integers (as the presence of as casts suggests) and a caller passes e.g. -1, the value silently wraps to a huge unsigned number (u32::MAX) instead of erroring — producing nonsensical fuzzy-match/slop behavior rather than a clear validation error. Note this file already validates signed→unsigned conversions elsewhere in the same PR-adjacent code (e.g. from_version in create_table_branch, version <= 0 checks in tag/version handlers, request.k > 0 before as usize), so this is inconsistent with the rest of the file.
🛠️ Example fix pattern (repeat for each field)
if let Some(slop) = p.slop {
- phrase = phrase.with_slop(slop as u32);
+ let slop = u32::try_from(slop).map_err(|_| NamespaceError::InvalidInput {
+ message: format!("phrase.slop must be non-negative, got {}", slop),
+ })?;
+ phrase = phrase.with_slop(slop);
}As per coding guidelines, "Validate inputs and reject invalid values with descriptive errors at API boundaries; never silently clamp or adjust values." Please confirm the actual signedness of these lance_namespace::models fields.
#!/bin/bash
rg -nP 'struct (MatchQuery|PhraseQuery)\b' -A 25
rg -nP '\bfuzziness\b|\bmax_expansions\b|\bprefix_length\b|\bslop\b' -g '*.rs'Also applies to: 5385-5412
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rust/lance-namespace-impls/src/dir.rs` around lines 5340 - 5348, Validate
signed namespace-model fields before converting them in the query conversion
logic around PhraseQuery and the fuzzy-query construction: reject negative slop,
fuzziness, max_expansions, and prefix_length with descriptive errors, then
perform the existing unsigned conversions only after validation. Confirm the
actual field types in lance_namespace::models and preserve valid-value behavior
while preventing negative inputs from wrapping.
| #[test] | ||
| fn test_build_engine_fts_query_requires_a_variant() { | ||
| // An FtsQuery with no variant set is rejected rather than silently ignored. | ||
| let empty = lance_namespace::models::FtsQuery::new(); | ||
| assert!(build_engine_fts_query(&empty).is_err()); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Test only checks is_err(), not the error variant/message.
test_build_engine_fts_query_requires_a_variant asserts build_engine_fts_query(&empty).is_err() but never verifies it's NamespaceError::InvalidInput with the expected message.
✅ Proposed fix
#[test]
fn test_build_engine_fts_query_requires_a_variant() {
// An FtsQuery with no variant set is rejected rather than silently ignored.
let empty = lance_namespace::models::FtsQuery::new();
- assert!(build_engine_fts_query(&empty).is_err());
+ let err = build_engine_fts_query(&empty).unwrap_err();
+ assert!(matches!(err, NamespaceError::InvalidInput { .. }));
+ assert!(err.to_string().contains("must set exactly one of"));
}As per coding guidelines, "Assert on both the error variant and the message content in tests; do not check only is_err()."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[test] | |
| fn test_build_engine_fts_query_requires_a_variant() { | |
| // An FtsQuery with no variant set is rejected rather than silently ignored. | |
| let empty = lance_namespace::models::FtsQuery::new(); | |
| assert!(build_engine_fts_query(&empty).is_err()); | |
| } | |
| #[test] | |
| fn test_build_engine_fts_query_requires_a_variant() { | |
| // An FtsQuery with no variant set is rejected rather than silently ignored. | |
| let empty = lance_namespace::models::FtsQuery::new(); | |
| let err = build_engine_fts_query(&empty).unwrap_err(); | |
| assert!(matches!(err, NamespaceError::InvalidInput { .. })); | |
| assert!(err.to_string().contains("must set exactly one of")); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rust/lance-namespace-impls/src/dir.rs` around lines 5547 - 5552, Strengthen
test_build_engine_fts_query_requires_a_variant by matching the returned error as
NamespaceError::InvalidInput and asserting its message contains the expected
missing-variant text. Replace the is_err-only assertion while preserving the
existing empty FtsQuery setup.
Source: Coding guidelines
2 participants
Summary
The directory namespace's
query_tableonly handledstring_queryfor full-text search — astructured_querywas silently ignored, so the scan ran with no FTS filter and returned all rows. This wiresstructured_querythrough to the engine, so the localquery_tablepath honors structured FTS the same way the JNI scanner andfragment.new_scan(full_text_query)already do. There was a literal// For now, we only support string_queryTODO at the change site.What changed
rust/lance-namespace-impls/src/dir.rs:structured_querybranch inquery_tablethat runsFullTextSearchQuery::new_query(...)built from the namespace model.build_engine_fts_querymapsmodels::FtsQuery→ engineFtsQueryfor all variants (match,phrase,multi_match, and recursivelyboolean/boost), mirroring the mapping inlance-jni/src/blocking_scanner.rs.build_engine_match_querycopiesterms/column/boost/fuzziness/max_expansions/operator/prefix_length, parsing the operator string viaOperator::try_from.FtsQuerywith no variant set is now rejected with a clear error instead of silently ignored.string_querybehavior is unchanged.Tests
Unit tests for the mapping —
test_build_engine_fts_query_match(allMatchQueryoptions round-trip into the engine query) andtest_build_engine_fts_query_requires_a_variant(empty query is rejected).cargo test -p lance-namespace-impls,cargo check -p lance-namespace-impls, andcargo fmt --checkall pass.Motivation
Downstream connectors (e.g. lance-spark) route full-text search to the namespace
query_tableand build the richerstructured_queryform (fuzziness / operator / phrase / multi_match). Against a directory namespace that droppedstructured_query, those queries silently returned every row instead of filtering. This makes thedirnamespace honor the structured API — which is the canonical FTS query direction (the engine'sFullTextSearchQueryis structured internally;string_queryis the legacy convenience form, and LanceDB's remote path already prefers structured).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes