Skip to content
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
338e82b
Score for match all is 0.0 now
carlosdelest Mar 7, 2025
af11e3b
Add scorable() method to Query class, and take it into account for cr…
carlosdelest Mar 7, 2025
0a2bb77
Add integration tests
carlosdelest Mar 7, 2025
3fdce04
Use boost() in queries according to whether they're scorable or not
carlosdelest Mar 10, 2025
ddd2efb
Update docs/changelog/124540.yaml
carlosdelest Mar 11, 2025
82b5432
[CI] Auto commit changes from spotless
Mar 11, 2025
1a2f653
Fix tests
carlosdelest Mar 11, 2025
4f7a308
Fix tests
carlosdelest Mar 11, 2025
86e7563
Fix tests
carlosdelest Mar 11, 2025
da0701a
Fix tests
carlosdelest Mar 11, 2025
30a052e
Fix tests
carlosdelest Mar 11, 2025
bd7f4f6
Spotless
carlosdelest Mar 11, 2025
65c863c
Merge remote-tracking branch 'carlosdelest/bugfix/esql-fix-scoring-fu…
carlosdelest Mar 11, 2025
48b4495
Fix changelog
carlosdelest Mar 11, 2025
7fb6d10
Add all full text functions to scoring tests
carlosdelest Mar 11, 2025
bf330f0
Minor changes
carlosdelest Mar 11, 2025
0c9f80a
Fix scores in tests, add capability for bwc
carlosdelest Mar 11, 2025
0913f82
Add docs
carlosdelest Mar 11, 2025
92436af
Fix forbidden APIs
carlosdelest Mar 11, 2025
bf8474c
Merge remote-tracking branch 'origin/main' into bugfix/esql-fix-scori…
carlosdelest Mar 11, 2025
c0b622b
Fix merges, add some testing
carlosdelest Mar 11, 2025
5f25b97
[CI] Auto commit changes from spotless
Mar 11, 2025
f932406
Fix test merges
carlosdelest Mar 11, 2025
347c651
Merge remote-tracking branch 'carlosdelest/bugfix/esql-fix-scoring-fu…
carlosdelest Mar 11, 2025
967d0fe
Fix test merges
carlosdelest Mar 11, 2025
afab198
Add Query.unscore() to remove scores from a QueryBuilder, use it from…
carlosdelest Mar 14, 2025
c7cd78c
Merge remote-tracking branch 'origin/main' into bugfix/esql-fix-scori…
carlosdelest Mar 14, 2025
2a618e1
Fix bwc test capability
carlosdelest Mar 14, 2025
66935a0
Merge remote-tracking branch 'origin/main' into bugfix/esql-fix-scori…
carlosdelest Mar 17, 2025
e641e38
Use filter again in PushFiltersToSource when no scoring is needed
carlosdelest Mar 17, 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
5 changes: 5 additions & 0 deletions docs/changelog/124540.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 124540
summary: "ES|QL: Fix scoring for full text functions"
area: ES|QL
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,14 @@ public BoolQuery(Source source, boolean isAnd, List<Query> queries) {
}

@Override
public QueryBuilder asBuilder() {
protected QueryBuilder asBuilder() {
BoolQueryBuilder boolQuery = boolQuery();
for (Query query : queries) {
QueryBuilder queryBuilder = query.toQueryBuilder();
Copy link
Member Author

Choose a reason for hiding this comment

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

asBuilder() is replaced by toQueryBuilder() - see below

if (isAnd) {
boolQuery.must(query.asBuilder());
boolQuery.must(queryBuilder);
} else {
boolQuery.should(query.asBuilder());
boolQuery.should(queryBuilder);
}
}
return boolQuery;
Expand Down Expand Up @@ -94,4 +95,9 @@ public Query negate(Source source) {
}
return new BoolQuery(source, isAnd == false, negated);
}

@Override
public boolean scorable() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public ExistsQuery(Source source, String name) {
}

@Override
public QueryBuilder asBuilder() {
protected QueryBuilder asBuilder() {
return existsQuery(name);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public double distance() {
}

@Override
public QueryBuilder asBuilder() {
protected QueryBuilder asBuilder() {
return QueryBuilders.geoDistanceQuery(field).distance(distance, DistanceUnit.METERS).point(lat, lon);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public MatchAll(Source source) {
}

@Override
public QueryBuilder asBuilder() {
protected QueryBuilder asBuilder() {
return matchAllQuery();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ public Query child() {
}

@Override
public QueryBuilder asBuilder() {
return boolQuery().mustNot(child.asBuilder());
protected QueryBuilder asBuilder() {
return boolQuery().mustNot(child.toQueryBuilder());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public String query() {
}

@Override
public QueryBuilder asBuilder() {
protected QueryBuilder asBuilder() {
return prefixQuery(field, query).caseInsensitive(caseInsensitive);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,20 @@ public Source source() {

/**
* Convert to an Elasticsearch {@link QueryBuilder} all set up to execute
* the query.
* the query. This ensures that queries have appropriate boosting for scoring.
*/
public abstract QueryBuilder asBuilder();
public final QueryBuilder toQueryBuilder() {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the method changed to include boosting information according to the query being scorable. asBuilder() is refactored to a protected method that must be implemented by subclasses.

QueryBuilder builder = asBuilder();
if (scorable() == false) {
builder.boost(0.0f);
Copy link
Member

Choose a reason for hiding this comment

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

A minor suggestion, we can define a constant variable for 0.0f, call it something like ESQL_NON_FTF_DEFAULT_BOOST, 0.0f is also used in several places in the tests like PhysicalPlanOptimizerTests they can all reference to the same constant variable.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in afab198

}
return builder;
}

/**
* Used internally to convert to retrieve a {@link QueryBuilder} by queries.
*/
protected abstract QueryBuilder asBuilder();

/**
* Used by {@link Query#toString()} to produce a pretty string.
Expand Down Expand Up @@ -85,4 +96,11 @@ public String toString() {
public Query negate(Source source) {
return new NotQuery(source, this);
}

/**
* Defines whether a query should contribute to the overall score
*/
public boolean scorable() {
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public QueryStringQuery(Source source, String query, Map<String, Float> fields,
}

@Override
public QueryBuilder asBuilder() {
protected QueryBuilder asBuilder() {
final QueryStringQueryBuilder queryBuilder = QueryBuilders.queryStringQuery(query);
queryBuilder.fields(fields);
options.forEach((k, v) -> {
Expand Down Expand Up @@ -132,4 +132,9 @@ public boolean equals(Object obj) {
protected String innerToString() {
return fields + ":" + query;
}

@Override
public boolean scorable() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public ZoneId zoneId() {
}

@Override
public QueryBuilder asBuilder() {
protected QueryBuilder asBuilder() {
RangeQueryBuilder queryBuilder = rangeQuery(field).from(lower, includeLower).to(upper, includeUpper);
if (Strings.hasText(format)) {
queryBuilder.format(format);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public Boolean caseInsensitive() {
}

@Override
public QueryBuilder asBuilder() {
protected QueryBuilder asBuilder() {
return regexpQuery(field, regex).caseInsensitive(caseInsensitive);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,31 @@

import static org.elasticsearch.index.query.QueryBuilders.termQuery;

/**
* Term query. It can be considered for scoring or not - filters that use term query as implementation will not use scoring,
* but the Term full text function will
*/
public class TermQuery extends Query {

private final String term;
private final Object value;
private final boolean caseInsensitive;
private final boolean scorable;

public TermQuery(Source source, String term, Object value) {
this(source, term, value, false);
}

public TermQuery(Source source, String term, Object value, boolean caseInsensitive) {
this(source, term, value, caseInsensitive, false);
}

public TermQuery(Source source, String term, Object value, boolean caseInsensitive, boolean scorable) {
super(source);
this.term = term;
this.value = value;
this.caseInsensitive = caseInsensitive;
this.scorable = scorable;
}

public String term() {
Expand All @@ -44,15 +54,15 @@ public Boolean caseInsensitive() {
}

@Override
public QueryBuilder asBuilder() {
protected QueryBuilder asBuilder() {
TermQueryBuilder qb = termQuery(term, value);
// ES does not allow case_insensitive to be set to "false", it should be either "true" or not specified
return caseInsensitive == false ? qb : qb.caseInsensitive(caseInsensitive);
}

@Override
public int hashCode() {
return Objects.hash(term, value, caseInsensitive);
return Objects.hash(term, value, caseInsensitive, scorable);
}

@Override
Expand All @@ -68,11 +78,17 @@ public boolean equals(Object obj) {
TermQuery other = (TermQuery) obj;
return Objects.equals(term, other.term)
&& Objects.equals(value, other.value)
&& Objects.equals(caseInsensitive, other.caseInsensitive);
&& Objects.equals(caseInsensitive, other.caseInsensitive)
&& scorable == other.scorable;
}

@Override
protected String innerToString() {
return term + ":" + value;
}

@Override
public boolean scorable() {
return scorable;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public TermsQuery(Source source, String term, Set<Object> values) {
}

@Override
public QueryBuilder asBuilder() {
protected QueryBuilder asBuilder() {
return termsQuery(term, values);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public Boolean caseInsensitive() {
}

@Override
public QueryBuilder asBuilder() {
protected QueryBuilder asBuilder() {
WildcardQueryBuilder wb = wildcardQuery(field, query);
// ES does not allow case_insensitive to be set to "false", it should be either "true" or not specified
return caseInsensitive == false ? wb : wb.caseInsensitive(caseInsensitive);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ private DummyLeafQuery(Source source) {
}

@Override
public QueryBuilder asBuilder() {
protected QueryBuilder asBuilder() {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ book_no:keyword | title:text | author:text
combinedMatchWithFunctionsScoring
required_capability: metadata_score
required_capability: match_operator_colon
required_capability: non_full_text_functions_scoring

from books metadata _score
| where title:"Tolkien" AND author:"Tolkien" AND year > 2000
Expand All @@ -153,7 +154,7 @@ from books metadata _score
| sort book_no;

book_no:keyword | title:text | author:text | year:integer | _score:double
5335 | Letters of J R R Tolkien | J.R.R. Tolkien | 2014 | 4.733664035797119
5335 | Letters of J R R Tolkien | J.R.R. Tolkien | 2014 | 3.733664035797119
;

singleQstrScoring
Expand Down Expand Up @@ -191,6 +192,7 @@ book_no:keyword | title:keyword | _score:double
combinedMatchWithScoringEvalNoSort
required_capability: metadata_score
required_capability: match_operator_colon
required_capability: non_full_text_functions_scoring

from books metadata _score
| where title:"Tolkien" AND author:"Tolkien" AND year > 2000
Expand All @@ -200,7 +202,7 @@ from books metadata _score

ignoreOrder:true
book_no:keyword | title:text | author:text | year:integer | c_score:double
5335 | Letters of J R R Tolkien | J.R.R. Tolkien | 2014 | 5.0
5335 | Letters of J R R Tolkien | J.R.R. Tolkien | 2014 | 4.0
;

singleQstrScoringRename
Expand Down Expand Up @@ -241,6 +243,7 @@ book_no:keyword | author:text | _score:do
combinedMatchWithFunctionsScoringNoSort
required_capability: metadata_score
required_capability: match_operator_colon
required_capability: non_full_text_functions_scoring

from books metadata _score
| where title:"Tolkien" AND author:"Tolkien" AND year > 2000
Expand All @@ -249,12 +252,13 @@ from books metadata _score

ignoreOrder:true
book_no:keyword | title:text | author:text | year:integer | _score:double
5335 | Letters of J R R Tolkien | J.R.R. Tolkien | 2014 | 4.733664035797119
5335 | Letters of J R R Tolkien | J.R.R. Tolkien | 2014 | 3.733664035797119
;

combinedMatchWithScoringEval
required_capability: metadata_score
required_capability: match_operator_colon
required_capability: non_full_text_functions_scoring

from books metadata _score
| where title:"Tolkien" AND author:"Tolkien" AND year > 2000
Expand All @@ -264,7 +268,7 @@ from books metadata _score
| sort book_no;

book_no:keyword | title:text | author:text | year:integer | c_score:double
5335 | Letters of J R R Tolkien | J.R.R. Tolkien | 2014 | 5.0
5335 | Letters of J R R Tolkien | J.R.R. Tolkien | 2014 | 4.0
;

singleQstrScoringEval
Expand Down Expand Up @@ -361,6 +365,7 @@ _id:keyword
scoresNonPushableFunctions

required_capability: metadata_score
required_capability: non_full_text_functions_scoring

from books metadata _score
| where length(title) > 100
Expand All @@ -369,13 +374,14 @@ from books metadata _score
;

book_no:keyword | _score:double
2924 | 1.0
8678 | 1.0
2924 | 0.0
8678 | 0.0
;

scoresPushableFunctions

required_capability: metadata_score
required_capability: non_full_text_functions_scoring

from books metadata _score
| where year >= 2017
Expand All @@ -384,11 +390,11 @@ from books metadata _score
;

book_no:keyword | _score:double
6818 | 1.0
7400 | 1.0
8480 | 1.0
8534 | 1.0
8615 | 1.0
6818 | 0.0
7400 | 0.0
8480 | 0.0
8534 | 0.0
8615 | 0.0
;

conjunctionScoresPushableNonPushableFunctions
Expand All @@ -413,6 +419,7 @@ conjunctionScoresPushableFunctions

required_capability: metadata_score
required_capability: match_function
required_capability: non_full_text_functions_scoring

from books metadata _score
| where match(title, "Lord") and ratings > 4.6
Expand All @@ -421,8 +428,8 @@ from books metadata _score
;

book_no:keyword | _score:double
7140 | 2.746896743774414
4023 | 2.5062403678894043
7140 | 1.746896743774414
4023 | 1.5062403678894043
;

disjunctionScoresPushableNonPushableFunctions
Expand All @@ -438,12 +445,12 @@ from books metadata _score
;

book_no:keyword | _score:double
2675 | 3.5619282722473145
2714 | 2.9245924949645996
7140 | 2.746896743774414
4023 | 2.5062403678894043
2924 | 1.0
8678 | 1.0
2675 | 2.5619282722473145
2714 | 1.9245924949645996
7140 | 1.746896743774414
4023 | 1.5062403678894043
2924 | 0.0
8678 | 0.0
;


Expand Down
Loading