Skip to content

Conversation

@cbuescher
Copy link
Member

This changes the queries equals() method so that the boost factors for each term
are considered for the equality calculation. This means queries are only equal
if both their terms and associated boosts match. The ordering of the terms
doesn't matter as before, which is why we internally need to sort the terms and
boost for comparison on the first equals() call like before. Boosts that are
null are considered equal to boosts of 1.0f because topLevelQuery() will only
wrap into BoostQuery if boost is not null and different from 1f.

Closes #48184

This changes the queries equals() method so that the boost factors for each term
are considered for the equality calculation. This means queries are only equal
if both their terms and associated boosts match. The ordering of the terms
doesn't matter as before, which is why we internally need to sort the terms and
boost for comparison on the first equals() call like before. Boosts that are
`null` are considered equal to boosts of 1.0f because topLevelQuery() will only
wrap into BoostQuery if boost is not null and different from 1f.

Closes elastic#48184
@cbuescher cbuescher added >enhancement :Search/Search Search-related issues that do not fall into other categories v8.0.0 v7.6.0 labels Oct 17, 2019
@elasticmachine
Copy link
Collaborator

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

}

TermAndBoost that = (TermAndBoost) o;
return term.equals(that.term) && boost == that.boost;
Copy link
Contributor

Choose a reason for hiding this comment

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

should we compare boosts as Float.compare(boost, that.boost) ==0 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

True, wrong old habits don die easily...

Copy link
Contributor

@mayya-sharipova mayya-sharipova left a comment

Choose a reason for hiding this comment

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

thanks @cbuescher. Nice addition.

Not completely related to your PR, I wonder how ES BlendedTermQuery is related to Lucene's BlendedTermQuery?

@cbuescher
Copy link
Member Author

Not completely related to your PR, I wonder how ES BlendedTermQuery is related to Lucene's BlendedTermQuery?

I don't really know tbh. but on cursory inspection they seem to be working differently, e.g. the way they rewrite against an IndexReader seem to follow different logic (the Lucene one having different RewriteMethods while the ES ones offers to overwrite topLevelQuery(), which in the one case I see produces a DisjunctionMaxQuery).
It might be interesting to see how both versions differe at this point and if we could merge them at some point.

@cbuescher cbuescher merged commit 1039bb4 into elastic:master Oct 25, 2019
cbuescher pushed a commit that referenced this pull request Oct 25, 2019
This changes the queries equals() method so that the boost factors for each term
are considered for the equality calculation. This means queries are only equal
if both their terms and associated boosts match. The ordering of the terms
doesn't matter as before, which is why we internally need to sort the terms and
boost for comparison on the first equals() call like before. Boosts that are
`null` are considered equal to boosts of 1.0f because topLevelQuery() will only
wrap into BoostQuery if boost is not null and different from 1f.

Closes #48184
cbuescher pushed a commit that referenced this pull request Oct 25, 2019
This changes the queries equals() method so that the boost factors for each term
are considered for the equality calculation. This means queries are only equal
if both their terms and associated boosts match. The ordering of the terms
doesn't matter as before, which is why we internally need to sort the terms and
boost for comparison on the first equals() call like before. Boosts that are
`null` are considered equal to boosts of 1.0f because topLevelQuery() will only
wrap into BoostQuery if boost is not null and different from 1f.

Closes #48184
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>enhancement :Search/Search Search-related issues that do not fall into other categories v7.6.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BlendedTermQuery's equals method should not disregard boosts

4 participants