Skip to content

KeywordField.newSetQuery() to reuse prefixed terms in IndexOrDocValuesQuery #14435

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mkhludnev merged 2 commits into from
Apr 5, 2025
Merged

KeywordField.newSetQuery() to reuse prefixed terms in IndexOrDocValuesQuery #14435

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
79d4fec
KeywordField.newSetQuery() reuses prefixed terms.
mkhludnev Apr 3, 2025
6993063
remove redundant method
mkhludnev Apr 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lucene/CHANGES.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ Optimizations

* GITHUB#14268: PointInSetQuery early exit on non-matching segments. (hanbj)

* GITHUB#14425: KeywordField.newSetQuery() reuses prefixed terms (Mikhail Khludnev)

Bug Fixes
---------------------
(No changes)
Expand Down
6 changes: 2 additions & 4 deletions lucene/core/src/java/org/apache/lucene/document/KeywordField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.apache.lucene.index.IndexOptions;
import org.apache.lucene.index.Term;
import org.apache.lucene.search.ConstantScoreQuery;
import org.apache.lucene.search.IndexOrDocValuesQuery;
import org.apache.lucene.search.MultiTermQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.SortField;
Expand Down Expand Up @@ -175,9 +174,8 @@ public static Query newExactQuery(String field, String value) {
public static Query newSetQuery(String field, Collection<BytesRef> values) {
Objects.requireNonNull(field, "field must not be null");
Objects.requireNonNull(values, "values must not be null");
Query indexQuery = new TermInSetQuery(field, values);
Query dvQuery = new TermInSetQuery(MultiTermQuery.DOC_VALUES_REWRITE, field, values);
return new IndexOrDocValuesQuery(indexQuery, dvQuery);
return TermInSetQuery.newIndexOrDocValuesQuery(
MultiTermQuery.CONSTANT_SCORE_BLENDED_REWRITE, field, values);
Comment on lines +177 to +178
Copy link
Contributor

@jainankitk jainankitk Apr 3, 2025

Choose a reason for hiding this comment

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

Probably we can use TermInSetQuery.newIndexOrDocValuesQuery(field, values) here?

Copy link
Member Author

@mkhludnev mkhludnev Apr 4, 2025

Choose a reason for hiding this comment

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

Right. Just want to make it explicit.

Copy link
Contributor

@jainankitk jainankitk Apr 4, 2025

Choose a reason for hiding this comment

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

Then we don't need that method, right?

Copy link
Contributor

@jpountz jpountz Apr 4, 2025

Choose a reason for hiding this comment

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

I would agree with this. KeywordField#newSetQuery is the convenience method, so it's nice to not have to pass a rewrite, TermInSetQuery#newIndexOrDocValuesQuery is the expert method, and experts can figure out how they want their multi-term queries to be rewritten.

Copy link
Contributor

@jainankitk jainankitk Apr 4, 2025

Choose a reason for hiding this comment

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

We should remove the unused method TermInSetQuery#newIndexOrDocValuesQuery(String field, Collection<BytesRef> terms) in that case, as anyone invoking TermInSetQuery#newIndexOrDocValuesQuery should use the expert method TermInSetQuery#newIndexOrDocValuesQuery(RewriteMethod indexRewriteMethod, String field, Collection<BytesRef> terms)?

Copy link
Member Author

@mkhludnev mkhludnev Apr 5, 2025

Choose a reason for hiding this comment

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

ok. Got it. Since we add something, let's add as least as possible.

Copy link
Contributor

@jainankitk jainankitk Apr 5, 2025

Choose a reason for hiding this comment

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

Thanks for making the change. I know its minor, but important for keeping it clean!

}

/**
Expand Down
24 changes: 23 additions & 1 deletion lucene/core/src/java/org/apache/lucene/search/TermInSetQuery.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,30 @@ public TermInSetQuery(RewriteMethod rewriteMethod, String field, Collection<Byte
termDataHashCode = termData.hashCode();
}

/**
* Creates a new {@link IndexOrDocValuesQuery} combining two {@link TermInSetQuery} with the same
* terms allowing to pack them only once that's faster. Doc Values query always uses {@link
* MultiTermQuery#DOC_VALUES_REWRITE}.
*
* @param field field name for indexed and doc values queries.
* @param indexRewriteMethod rewrite method used for indexed query.
* @param terms collection of {@link BytesRef}. Note: passing {@link SortedSet} with default
* comparator let to bypass terms sorting.
*/
public static IndexOrDocValuesQuery newIndexOrDocValuesQuery(
RewriteMethod indexRewriteMethod, String field, Collection<BytesRef> terms) {
PrefixCodedTerms packed = packTerms(field, terms);
Query indexQuery = new TermInSetQuery(indexRewriteMethod, field, packed);
Query dvQuery = new TermInSetQuery(MultiTermQuery.DOC_VALUES_REWRITE, field, packed);
return new IndexOrDocValuesQuery(indexQuery, dvQuery);
}

private TermInSetQuery(String field, PrefixCodedTerms termData) {
super(field, MultiTermQuery.CONSTANT_SCORE_BLENDED_REWRITE);
this(MultiTermQuery.CONSTANT_SCORE_BLENDED_REWRITE, field, termData);
}

private TermInSetQuery(RewriteMethod rewrite, String field, PrefixCodedTerms termData) {
super(field, rewrite);
this.field = field;
this.termData = termData;
termDataHashCode = termData.hashCode();
Expand Down