Skip to content

Use inner query for equals/hashCode() in SourceConfirmedTextQuery - #134451

Merged
elasticsearchmachine merged 3 commits into
elastic:mainfrom
romseygeek:bug/sctq-equals-hashcode
Sep 10, 2025
Merged

elasticsearchmachine merged 3 commits into
elastic:mainfrom
romseygeek:bug/sctq-equals-hashcode

Conversation

@romseygeek

Copy link
Copy Markdown
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.
@elasticsearchmachine

Copy link
Copy Markdown
Collaborator

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

@elasticsearchmachine

Copy link
Copy Markdown
Collaborator

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

@elasticsearchmachine

Copy link
Copy Markdown
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
Copy Markdown
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

@martijnvg martijnvg left a comment

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.

LGTM!

@benwtrent benwtrent left a comment

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.

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
Copy Markdown
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 DO NOT USE DEPRECATED - DO NOT USE :StorageEngine/Mapping The storage related side of mappings Team:Search DEPRECATED - DO NOT USE Team:StorageEngine v8.19.5 v9.1.5 v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SourceConfirmedTextQuery equals/hashCode methods refer to lambdas

4 participants