Skip to content

Commit 8766b77

Browse files
carlosdelestomricohenn
authored andcommitted
ES\QL: Fix scoring for non full text functions (elastic#124540)
1 parent 8854471 commit 8766b77

File tree

40 files changed

+508
-248
lines changed

40 files changed

+508
-248
lines changed

docs/changelog/124540.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 124540
2+
summary: "ES|QL: Fix scoring for full text functions"
3+
area: ES|QL
4+
type: bug
5+
issues: []

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/querydsl/query/BoolQuery.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,14 @@ public BoolQuery(Source source, boolean isAnd, List<Query> queries) {
4343
}
4444

4545
@Override
46-
public QueryBuilder asBuilder() {
46+
protected QueryBuilder asBuilder() {
4747
BoolQueryBuilder boolQuery = boolQuery();
4848
for (Query query : queries) {
49+
QueryBuilder queryBuilder = query.toQueryBuilder();
4950
if (isAnd) {
50-
boolQuery.must(query.asBuilder());
51+
boolQuery.must(queryBuilder);
5152
} else {
52-
boolQuery.should(query.asBuilder());
53+
boolQuery.should(queryBuilder);
5354
}
5455
}
5556
return boolQuery;
@@ -94,4 +95,9 @@ public Query negate(Source source) {
9495
}
9596
return new BoolQuery(source, isAnd == false, negated);
9697
}
98+
99+
@Override
100+
public boolean scorable() {
101+
return true;
102+
}
97103
}

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/querydsl/query/ExistsQuery.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ public ExistsQuery(Source source, String name) {
2121
}
2222

2323
@Override
24-
public QueryBuilder asBuilder() {
24+
protected QueryBuilder asBuilder() {
2525
return existsQuery(name);
2626
}
2727

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/querydsl/query/GeoDistanceQuery.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public double distance() {
4545
}
4646

4747
@Override
48-
public QueryBuilder asBuilder() {
48+
protected QueryBuilder asBuilder() {
4949
return QueryBuilders.geoDistanceQuery(field).distance(distance, DistanceUnit.METERS).point(lat, lon);
5050
}
5151

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/querydsl/query/MatchAll.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public MatchAll(Source source) {
1717
}
1818

1919
@Override
20-
public QueryBuilder asBuilder() {
20+
protected QueryBuilder asBuilder() {
2121
return matchAllQuery();
2222
}
2323

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/querydsl/query/NotQuery.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ public Query child() {
3232
}
3333

3434
@Override
35-
public QueryBuilder asBuilder() {
36-
return boolQuery().mustNot(child.asBuilder());
35+
protected QueryBuilder asBuilder() {
36+
return boolQuery().mustNot(child.toQueryBuilder());
3737
}
3838

3939
@Override

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/querydsl/query/PrefixQuery.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public String query() {
3434
}
3535

3636
@Override
37-
public QueryBuilder asBuilder() {
37+
protected QueryBuilder asBuilder() {
3838
return prefixQuery(field, query).caseInsensitive(caseInsensitive);
3939
}
4040

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/querydsl/query/Query.java

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@
2828
* </p>
2929
*/
3030
public abstract class Query {
31+
32+
// Boosting used to remove scoring from queries that don't contribute to it
33+
public static final float NO_SCORE_BOOST = 0.0f;
34+
3135
private final Source source;
3236

3337
protected Query(Source source) {
@@ -46,9 +50,20 @@ public Source source() {
4650

4751
/**
4852
* Convert to an Elasticsearch {@link QueryBuilder} all set up to execute
49-
* the query.
53+
* the query. This ensures that queries have appropriate boosting for scoring.
54+
*/
55+
public final QueryBuilder toQueryBuilder() {
56+
QueryBuilder builder = asBuilder();
57+
if (scorable() == false) {
58+
builder = unscore(builder);
59+
}
60+
return builder;
61+
}
62+
63+
/**
64+
* Used internally to convert to retrieve a {@link QueryBuilder} by queries.
5065
*/
51-
public abstract QueryBuilder asBuilder();
66+
protected abstract QueryBuilder asBuilder();
5267

5368
/**
5469
* Used by {@link Query#toString()} to produce a pretty string.
@@ -85,4 +100,18 @@ public String toString() {
85100
public Query negate(Source source) {
86101
return new NotQuery(source, this);
87102
}
103+
104+
/**
105+
* Defines whether a query should contribute to the overall score
106+
*/
107+
public boolean scorable() {
108+
return false;
109+
}
110+
111+
/**
112+
* Removes score from a query builder, so score is not affected by the query
113+
*/
114+
public static QueryBuilder unscore(QueryBuilder builder) {
115+
return builder.boost(NO_SCORE_BOOST);
116+
}
88117
}

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/querydsl/query/QueryStringQuery.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ public QueryStringQuery(Source source, String query, Map<String, Float> fields,
8888
}
8989

9090
@Override
91-
public QueryBuilder asBuilder() {
91+
protected QueryBuilder asBuilder() {
9292
final QueryStringQueryBuilder queryBuilder = QueryBuilders.queryStringQuery(query);
9393
queryBuilder.fields(fields);
9494
options.forEach((k, v) -> {
@@ -132,4 +132,9 @@ public boolean equals(Object obj) {
132132
protected String innerToString() {
133133
return fields + ":" + query;
134134
}
135+
136+
@Override
137+
public boolean scorable() {
138+
return true;
139+
}
135140
}

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/querydsl/query/RangeQuery.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public ZoneId zoneId() {
7777
}
7878

7979
@Override
80-
public QueryBuilder asBuilder() {
80+
protected QueryBuilder asBuilder() {
8181
RangeQueryBuilder queryBuilder = rangeQuery(field).from(lower, includeLower).to(upper, includeUpper);
8282
if (Strings.hasText(format)) {
8383
queryBuilder.format(format);

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/querydsl/query/RegexQuery.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public Boolean caseInsensitive() {
4242
}
4343

4444
@Override
45-
public QueryBuilder asBuilder() {
45+
protected QueryBuilder asBuilder() {
4646
return regexpQuery(field, regex).caseInsensitive(caseInsensitive);
4747
}
4848

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/querydsl/query/TermQuery.java

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,21 +14,31 @@
1414

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

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

1923
private final String term;
2024
private final Object value;
2125
private final boolean caseInsensitive;
26+
private final boolean scorable;
2227

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

2732
public TermQuery(Source source, String term, Object value, boolean caseInsensitive) {
33+
this(source, term, value, caseInsensitive, false);
34+
}
35+
36+
public TermQuery(Source source, String term, Object value, boolean caseInsensitive, boolean scorable) {
2837
super(source);
2938
this.term = term;
3039
this.value = value;
3140
this.caseInsensitive = caseInsensitive;
41+
this.scorable = scorable;
3242
}
3343

3444
public String term() {
@@ -44,15 +54,15 @@ public Boolean caseInsensitive() {
4454
}
4555

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

5363
@Override
5464
public int hashCode() {
55-
return Objects.hash(term, value, caseInsensitive);
65+
return Objects.hash(term, value, caseInsensitive, scorable);
5666
}
5767

5868
@Override
@@ -68,11 +78,17 @@ public boolean equals(Object obj) {
6878
TermQuery other = (TermQuery) obj;
6979
return Objects.equals(term, other.term)
7080
&& Objects.equals(value, other.value)
71-
&& Objects.equals(caseInsensitive, other.caseInsensitive);
81+
&& Objects.equals(caseInsensitive, other.caseInsensitive)
82+
&& scorable == other.scorable;
7283
}
7384

7485
@Override
7586
protected String innerToString() {
7687
return term + ":" + value;
7788
}
89+
90+
@Override
91+
public boolean scorable() {
92+
return scorable;
93+
}
7894
}

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/querydsl/query/TermsQuery.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public TermsQuery(Source source, String term, Set<Object> values) {
2626
}
2727

2828
@Override
29-
public QueryBuilder asBuilder() {
29+
protected QueryBuilder asBuilder() {
3030
return termsQuery(term, values);
3131
}
3232

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/querydsl/query/WildcardQuery.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public Boolean caseInsensitive() {
4343
}
4444

4545
@Override
46-
public QueryBuilder asBuilder() {
46+
protected QueryBuilder asBuilder() {
4747
WildcardQueryBuilder wb = wildcardQuery(field, query);
4848
// ES does not allow case_insensitive to be set to "false", it should be either "true" or not specified
4949
return caseInsensitive == false ? wb : wb.caseInsensitive(caseInsensitive);

x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/querydsl/query/LeafQueryTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ private DummyLeafQuery(Source source) {
2323
}
2424

2525
@Override
26-
public QueryBuilder asBuilder() {
26+
protected QueryBuilder asBuilder() {
2727
return null;
2828
}
2929

0 commit comments

Comments
 (0)