Skip to content

NPE on batched query execution when the request is part of PIT with alias filters#128552

Merged
drempapis merged 19 commits intoelastic:8.19from
drempapis:fix/batch_query_pit_with_alias
Jun 3, 2025
Merged

NPE on batched query execution when the request is part of PIT with alias filters#128552
drempapis merged 19 commits intoelastic:8.19from
drempapis:fix/batch_query_pit_with_alias

Conversation

@drempapis
Copy link
Contributor

@drempapis drempapis commented May 28, 2025

As in the description of the issue (#128270)

When the conditions are met for the new feature Batched query execution (#121885) to be applied, the specific use case where executing a request within a point in time with alias filters leads to a NPE.

In SearchQueryThenFetchAsyncAction#isPartOfPIT, a null namedWriteableRegistry is passed when trying to access the search context id of the point in time builder. Since the alias filters use the registry when decoding the pit ID, this results in a NPE.

In this work, the namedWriteableRegistry is propagated to the isPartOfPIT method to facilitate context retrieval.

@drempapis drempapis added >bug Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch :Search Foundations/Search Catch all for Search Foundations labels May 28, 2025
@elasticsearchmachine
Copy link
Collaborator

Hi @drempapis, I've created a changelog YAML for you.

@drempapis drempapis changed the title add fix code May 28, 2025
@drempapis drempapis marked this pull request as ready for review May 29, 2025 15:31
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-foundations (Team:Search Foundations)

public void testIndexWithAlias() {
String indexName = "index_1";
String alias = "alias_1";
assertAcked(indicesAdmin().prepareCreate(indexName).setSettings(indexSettings(10, 0)).addAlias(new Alias(alias)));
Copy link
Contributor

Choose a reason for hiding this comment

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

Did this test fail without the fix? I was unclear on whether we need to add a filter to the alias, or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the filter is required. I inadvertently committed a version that did not include the filtering logic. Updating with some enhancements

Copy link
Contributor

@javanna javanna left a comment

Choose a reason for hiding this comment

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

I left a couple of comments on the test, the fix makes sense to me. LGTM, no need for another round once the comments are addressed.

@drempapis drempapis merged commit d77d2cf into elastic:8.19 Jun 3, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug :Search Foundations/Search Catch all for Search Foundations Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch v8.19.0

3 participants