Skip to content

[phrase matching] Text index fixes#6730

Merged
generall merged 4 commits intoadd-phrase-match-variantfrom
phrase-fixes
Jun 23, 2025
Merged

[phrase matching] Text index fixes#6730
generall merged 4 commits intoadd-phrase-match-variantfrom
phrase-fixes

Conversation

@coszio
Copy link
Contributor

@coszio coszio commented Jun 19, 2025

Builds on top of #6670.

Two fixes for text index:

  • Allow immutable text index with rocks-db storage
  • Phrases with repeated tokens were being incorrectly handled when checking specific ids.

@coszio coszio requested review from generall and timvisee June 19, 2025 23:29
@coszio coszio added this to the Phrase matching milestone Jun 19, 2025
}

#[cfg(test)]
mod tests {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This (with a few other diffs) sneaked back in after being moved in #6638, it is now in test_congruence.rs file

Comment on lines -366 to +373
IndexSelector::RocksDb(IndexSelectorRocksDb {
db,
is_appendable: _,
}) => FieldIndexBuilder::FullTextIndex(FullTextIndex::builder_rocksdb(
Arc::clone(db),
config,
&field.to_string(),
)),
IndexSelector::RocksDb(IndexSelectorRocksDb { db, is_appendable }) => {
FieldIndexBuilder::FullTextIndex(FullTextIndex::builder_rocksdb(
Arc::clone(db),
config,
&field.to_string(),
*is_appendable,
))
}
Copy link
Contributor Author

@coszio coszio Jun 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where it was selecting an immutable index, but silently creating a mutable one

token_to_posting: impl Fn(&TokenId) -> Option<PostingListView<'a, Positions>>,
) -> bool {
let Some(mut posting_iterators): Option<Vec<_>> = phrase
.to_token_set()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not deduplicating here was the main bug

@generall generall merged commit 5366848 into add-phrase-match-variant Jun 23, 2025
17 checks passed
@generall generall deleted the phrase-fixes branch June 23, 2025 13:08
coszio added a commit that referenced this pull request Jun 24, 2025
* allow rocksdb-based immutable text index

* fix repeated-token phrases

* fmt

* Update OpenAPI spec

---------

Co-authored-by: timvisee <tim@visee.me>
coszio added a commit that referenced this pull request Jun 24, 2025
* add `"match": { "phrase": ... }` condition

* gen grpc and openapi

* [phrase matching] expose `phrase_matching` flag in rest and grpc (#6620)

* expose setting in rest and grpc

* phrase matching openapi test

* regen openapi

* [phrase matching] Text index fixes (#6730)

* allow rocksdb-based immutable text index

* fix repeated-token phrases

* fmt

* Update OpenAPI spec

---------

Co-authored-by: timvisee <tim@visee.me>

* add repeated word case in openapi test

* [phrase match | strict mode] Allow phrase condition when enabled in index (#6749)

* allow phrase filter when index is present

* prettier error message

* clippppppy

---------

Co-authored-by: timvisee <tim@visee.me>
generall pushed a commit that referenced this pull request Jul 17, 2025
* add `"match": { "phrase": ... }` condition

* gen grpc and openapi

* [phrase matching] expose `phrase_matching` flag in rest and grpc (#6620)

* expose setting in rest and grpc

* phrase matching openapi test

* regen openapi

* [phrase matching] Text index fixes (#6730)

* allow rocksdb-based immutable text index

* fix repeated-token phrases

* fmt

* Update OpenAPI spec

---------

Co-authored-by: timvisee <tim@visee.me>

* add repeated word case in openapi test

* [phrase match | strict mode] Allow phrase condition when enabled in index (#6749)

* allow phrase filter when index is present

* prettier error message

* clippppppy

---------

Co-authored-by: timvisee <tim@visee.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants