Skip to content

Conversation

romseygeek
Copy link
Contributor

We were using a valueFetcher lambda and an Analyzer as part of the equals
and hashCode implementations which could easily compare as unequal for
logically equal queries. We can safely just use the inner query for comparisons
and hashcodes as the results of the outer query are identical to running the
inner query against a shard with indexed positions.

Fixes #134432

…edTextQuery We were using a valueFetcher lambda and an Analyzer as part of the equals and hashCode implementations which could easily compare as unequal for logically equal queries. We can safely just use the inner query for comparisons and hashcodes as the results of the outer query are identical to running the inner query against a shard with indexed positions.
@romseygeek romseygeek self-assigned this Sep 10, 2025
@romseygeek romseygeek added >bug :Search/Search Search-related issues that do not fall into other categories :StorageEngine/Mapping The storage related side of mappings v9.2.0 v8.19.5 v9.1.5 labels Sep 10, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-storage-engine (Team:StorageEngine)

Query sourceConfirmedPhraseQuery3 = new SourceConfirmedTextQuery(query3, sourceFetcherProvider(), Lucene.STANDARD_ANALYZER);
assertNotEquals(sourceConfirmedPhraseQuery1, sourceConfirmedPhraseQuery3);

Query sourceConfirmedPhraseQuery4 = new SourceConfirmedTextQuery(query1, context -> null, Lucene.STANDARD_ANALYZER);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

NB - these tests were specifically checking for different analyzers and value fetcher lambdas, which we no longer want to do. The Analyzer is always the same for queries against the same mapped field, so will always be identical if the inner Query is identical.

@martijnvg martijnvg added the auto-backport Automatically create backport pull requests when merged label Sep 10, 2025
Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@benwtrent benwtrent left a comment

Choose a reason for hiding this comment

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

Yeah, this is a silly little 🐛 . Good find

@romseygeek romseygeek added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Sep 10, 2025
@elasticsearchmachine elasticsearchmachine merged commit 41fea9d into elastic:main Sep 10, 2025
34 checks passed
@romseygeek romseygeek deleted the bug/sctq-equals-hashcode branch September 10, 2025 14:50
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.19 Commit could not be cherrypicked due to conflicts
9.1

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 134451

romseygeek added a commit to romseygeek/elasticsearch that referenced this pull request Sep 10, 2025
…astic#134451) We were using a valueFetcher lambda and an Analyzer as part of the equals and hashCode implementations which could easily compare as unequal for logically equal queries. We can safely just use the inner query for comparisons and hashcodes as the results of the outer query are identical to running the inner query against a shard with indexed positions. Fixes elastic#134432
elasticsearchmachine pushed a commit that referenced this pull request Sep 10, 2025
…34451) (#134456) We were using a valueFetcher lambda and an Analyzer as part of the equals and hashCode implementations which could easily compare as unequal for logically equal queries. We can safely just use the inner query for comparisons and hashcodes as the results of the outer query are identical to running the inner query against a shard with indexed positions. Fixes #134432
elasticsearchmachine pushed a commit that referenced this pull request Sep 10, 2025
…34451) (#134459) * Use inner query for equals/hashCode() in SourceConfirmedTextQuery (#134451) We were using a valueFetcher lambda and an Analyzer as part of the equals and hashCode implementations which could easily compare as unequal for logically equal queries. We can safely just use the inner query for comparisons and hashcodes as the results of the outer query are identical to running the inner query against a shard with indexed positions. Fixes #134432 * Update docs/changelog/134459.yaml * Delete docs/changelog/134459.yaml
sarog pushed a commit to portsbuild/elasticsearch that referenced this pull request Sep 11, 2025
…astic#134451) (elastic#134459) * Use inner query for equals/hashCode() in SourceConfirmedTextQuery (elastic#134451) We were using a valueFetcher lambda and an Analyzer as part of the equals and hashCode implementations which could easily compare as unequal for logically equal queries. We can safely just use the inner query for comparisons and hashcodes as the results of the outer query are identical to running the inner query against a shard with indexed positions. Fixes elastic#134432 * Update docs/changelog/134459.yaml * Delete docs/changelog/134459.yaml
sarog pushed a commit to portsbuild/elasticsearch that referenced this pull request Sep 19, 2025
…astic#134451) (elastic#134459) * Use inner query for equals/hashCode() in SourceConfirmedTextQuery (elastic#134451) We were using a valueFetcher lambda and an Analyzer as part of the equals and hashCode implementations which could easily compare as unequal for logically equal queries. We can safely just use the inner query for comparisons and hashcodes as the results of the outer query are identical to running the inner query against a shard with indexed positions. Fixes elastic#134432 * Update docs/changelog/134459.yaml * Delete docs/changelog/134459.yaml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) backport pending >bug :Search/Search Search-related issues that do not fall into other categories :StorageEngine/Mapping The storage related side of mappings Team:Search Meta label for search team Team:StorageEngine v8.19.5 v9.1.5 v9.2.0

4 participants