Skip to content

Auto prefiltering for queries on dense semantic_text fields#138989

Merged
dimitris-athanasiou merged 28 commits intoelastic:mainfrom
dimitris-athanasiou:semantic_text_auto_prefiltering
Dec 12, 2025
Merged

Auto prefiltering for queries on dense semantic_text fields#138989
dimitris-athanasiou merged 28 commits intoelastic:mainfrom
dimitris-athanasiou:semantic_text_auto_prefiltering

Conversation

@dimitris-athanasiou
Copy link
Copy Markdown
Contributor

@dimitris-athanasiou dimitris-athanasiou commented Dec 3, 2025

knn queries allow specifying filters that will be applied before the knn search. This pre-filtering allows the knn to return k results. If such filters are to applied only after the knn executes, then the knn returns the k matching results but the filters can filter out some of them thus potentially returning fewer than k results.

semantic_text fields can be queried with:

DSL

  • match queries
  • semantic queries
  • knn queries

ES|QL

  • match queries
  • knn queries

For DSL, knn queries allow users to specify direct prefilters. However, match and semantic queries provide no way to do so. Same goes for ES|QL MATCH. Noting that ES|QL KNN already implements auto pre-filtering where conjunctions are pushed down to the knn query as prefilters.

This commit implements semantic_text auto pre-filtering for semantic_text queries in DSL (match and semantic queries) and ES|QL (MATCH).

We achieve this by adding an AutoPrefilteringScope object to the SearchExecutionContext. When we convert a bool query to a lucene query, we push its must, filter, and must_not clauses to the AutoPrefilteringScope. At that stage queries have already been rewritten. Semantic queries using text_embedding inference endpoints are rewritten to knn vector queries that are auto-prefiltering enabled. Then, when an auto-prefiltering enabled knn vector query is converted to its lucene equivalent, we fetch the prefilters from the SearchExecutionContext and we apply them to the knn vector query - which supports pre-filtering already.

ES|QL queries that contain MATCH automatically benefit from this implementation because they are rewritten in bool queries.

Limitations

DSL

ES|QL

  • filters that are not translatable to lucene queries will be applied as post-filters

Relates #132068

`knn` queries allow specifying `filters` that will be applied before
the knn search. This `pre-filtering` allows the `knn` to return `k`
results. If such filters are to applied only after the `knn` executes,
then the `knn` returns the `k` matching results but the filters can
filter out some of them thus potentially returning fewer than `k` results.

`semantic_text` fields can be queried with:

DSL

- `match` queries
- `semantic` queries
- `knn` queries

ES|QL

- `match` queries
- `knn` queries

For DSL, `knn` queries allow users to specify direct prefilters.
However, `match` and `semantic` queries provide no way to do so.
Same goes for ES|QL `match`. Noting that ES|QL `KNN` already implements
auto pre-filtering where conjunctions are pushed down to the `knn` query
as prefilters.

This commit implements semantic_text auto pre-filtering for `semantic_text` queries
in DSL (`match` and `semantic` queries) and ES|QL (`MATCH`).

We achieve this by adding an `AutoPrefilteringScope` object to the
`SearchExecutionContext`. When we convert a `bool` query to a lucene query,
we push its `must`, `filter`, and `must_not` clauses to the `AutoPrefilteringScope`.
At that stage queries have already been rewritten. Semantic queries using
`text_embedding` inference endpoints are rewritten to knn vector queries
that are auto-prefiltering enabled. Then, when an auto-prefiltering
enabled knn vector query is converted to its lucene equivalent, we
fetch the prefilters from the `SearchExecutionContext` and we apply
them to the knn vector query - which supports pre-filtering already.

ES|QL queries that contain `MATCH` automatically benefit from this implementation because
they are rewritten in `bool` queries.

Limitations

DSL

- nested queries are excluded from pre-filtering (elastic#138184)

ES|QL

- filters that are not translatable to lucene queries will be applied as post-filters

Relates elastic#132068
@dimitris-athanasiou dimitris-athanasiou added >bug :SearchOrg/Relevance Label for the Search (solution/org) Relevance team v9.3.0 labels Dec 3, 2025
@elasticsearchmachine elasticsearchmachine added the Team:Search - Relevance The Search organization Search Relevance team label Dec 3, 2025
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/search-relevance (Team:Search - Relevance)

@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Hi @dimitris-athanasiou, I've created a changelog YAML for you.

@Mikep86 Mikep86 requested a review from a team December 8, 2025 20:58
Copy link
Copy Markdown
Contributor

@Mikep86 Mikep86 left a comment

Choose a reason for hiding this comment

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

Partial review, nice work! I didn't get to the tests, but I reviewed all the production logic. I identified some potential edge cases that I think we could iterate on.

rescoreVectorBuilder,
vectorSimilarity
).boost(boost).queryName(queryName).addFilterQueries(filterQueries);
).boost(boost).queryName(queryName).addFilterQueries(filterQueries).setAutoPrefilteringEnabled(isAutoPrefilteringEnabled);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There's a bunch of places in the query interception logic where we need to make a copy of the knn query, except slightly tweaked. It's very easy to overlook the need to call setAutoPrefilteringEnabled when making such copies. Maybe it's time for a little static helper method that takes an origin knn query and applies boost, queryName, and autoPrefilteringEnabled values to a target knn query?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To be fair the constructor situation in KnnVectorQueryBuilder has gone wild and I agree it is very fragile. A static helper would be nice but we'd still need to remember to call it. I wonder if the right solution here is a refactoring of the constructors. How about we leave this is follow up work? I can raise an issue for tidying this up.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Agreed a more thorough refactoring is needed here, which we can do in a follow-up. IMO we should refactor KnnVectorQueryBuilder to use a builder pattern that can take an existing KnnVectorQueryBuilder to initialize.

@dimitris-athanasiou
Copy link
Copy Markdown
Contributor Author

@Mikep86 I have pushed commits to the PR where I address your feedback:

  • I have added a AutoPrefilteringUtils.pruneQuery that takes in a set of QueryBuilder classes, looks into the query tree and prunes query branches if from the query that matches one of the given classes. We only use NestedQueryBuilder for now.
  • I have removed the loop protection in KnnVectorQueryBuilder as it is no longer necessary as we ensure queries exclude themselves from becoming prefilters in their inner queries.

Copy link
Copy Markdown
Contributor

@Mikep86 Mikep86 left a comment

Choose a reason for hiding this comment

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

Fantastic work 🙌 ! All the production code looks good. I pointed out a potential edge case in the min-should-match handing, but I don't have a good solution for it (it's also a very narrow edge case). Other than that, it's just a few small adjustments to tests.

Copy link
Copy Markdown
Contributor

@Mikep86 Mikep86 left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

Comment on lines +101 to +113
// We need to adjust the minimum should match to account for the pruned clauses.
// We considered the following approaches:
// 1. strict approach: set to min(remaining_should_clauses, original_msm)
// 2. lenient approach: if msm is set and at least one should clause is pruned, prune all should clauses.
// 3. middle ground approach: set to max(0, original_msm - remaining_should_clauses)
// Let us imagine a query with 5 should clauses. 2 get pruned. msm is 3. 1 remaining clause matches.
// Approach 1 would make the entire bool query to not match as we would retain msm of 3 but only 1 clause would match.
// We do not know whether the pruned clauses would match or not. Thus, this approach seems too restrictive.
// Approach 2 would mean we prune all should clauses and the query would match,
// even if none of the remaining should clauses match.
// Approach 3 would mean we adjust the msm to 3 - 2 = 1. This would mean that the query would match if at least one
// of the remaining clauses matches.
// We opt for the lenient approach. It is as if we assume the pruned clauses matched. Seems to be the best compromise.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for the thorough description ❤️

Copy link
Copy Markdown
Member

@carlosdelest carlosdelest left a comment

Choose a reason for hiding this comment

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

LGTM, amazing work!

It would be awesome to have a test in SemanticMatchTestCase that checks that ES|QL applies prefiltering - but not needed for this PR

return Optional.empty();
}

if (query instanceof BoolQueryBuilder boolQuery) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit - Consider using pattern matching for switch

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 91c0a75. Much prettier!

}

public void testBWCVersionSerialization_GivenAutoPrefiltering() throws IOException {
for (int i = 0; i < 100; i++) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why execute this multiple times? Is this a loop for testing the test?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

During my testing I found that I needed that to surface problems faster. It runs pretty fast so I left it in.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You should use -Dtests.iters= instead 😉 . Let's remove this as you'll get plenty of executions on CI anyway.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That is true. But if you take a look at AbstractBWCSerializationTestCase you'll see we also do multiple runs there by default. What it helps with is that if someone makes a change that breaks BWC, they might run the tests once, they pass and they think it's all good. Whereas running a bunch of times significantly increases the probability to surface a failure and gives immediate feedback to the dev to fix the issue before getting in CI.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I was not aware of that, thanks!

Maybe then use NUMBER_OF_TEST_RUNS instead to keep with the pattern? 🤷

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 1a8cdc2

AbstractQueryTestCase has its own such constant, NUMBER_OF_TESTQUERIES

@dimitris-athanasiou
Copy link
Copy Markdown
Contributor Author

dimitris-athanasiou commented Dec 12, 2025

It would be awesome to have a test in SemanticMatchTestCase that checks that ES|QL applies prefiltering - but not needed for this PR

@carlosdelest I have added such a test! It's there!

@carlosdelest
Copy link
Copy Markdown
Member

It would be awesome to have a test in SemanticMatchTestCase that checks that ES|QL applies prefiltering - but not needed for this PR

@carlosdelest I have added such a test! It's there!

@dimitris-athanasiou It indeed is! Isn't that awesome? 😅 🤦

@dimitris-athanasiou dimitris-athanasiou merged commit d8b6b9c into elastic:main Dec 12, 2025
35 checks passed
dimitris-athanasiou added a commit to dimitris-athanasiou/elasticsearch that referenced this pull request Dec 18, 2025
Adds documentation for automatic pre-filtering that was introduced
in elastic#138989.
dimitris-athanasiou added a commit that referenced this pull request Dec 19, 2025
Adds documentation for automatic pre-filtering that was introduced
in #138989.

Co-authored-by: Liam Thompson <leemthompo@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug :SearchOrg/Relevance Label for the Search (solution/org) Relevance team Team:Search - Relevance The Search organization Search Relevance team v9.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants