honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

SQL: Preserve scoring in bool queries (#30730) 

Make all bool constructs use match/should (that is a query context) as
that is controlled and changed to a filter context by ES automatically
based on the sort order (_doc, field vs _sort) and trackScores.

Fix #29685
This commit is contained in:
Costin Leau 2018-05-21 21:50:06 +03:00 committed by GitHub
parent be8c0f27be
commit dcf0f9f8dd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 33 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java
View File

@ -5,8 +5,6 @@
*/
package org.elasticsearch.xpack.sql.execution.search;
import org.elasticsearch.index.query.BoolQueryBuilder;
import org.elasticsearch.index.query.ConstantScoreQueryBuilder;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.search.aggregations.AggregationBuilder;
import org.elasticsearch.search.aggregations.bucket.composite.CompositeAggregationBuilder;
@ -28,6 +26,7 @@ import org.elasticsearch.xpack.sql.querydsl.container.Sort;
import java.util.List;
import static java.util.Collections.singletonList;
import static org.elasticsearch.index.query.QueryBuilders.boolQuery;
import static org.elasticsearch.search.sort.SortBuilders.fieldSort;
import static org.elasticsearch.search.sort.SortBuilders.scoreSort;
import static org.elasticsearch.search.sort.SortBuilders.scriptSort;
@ -37,20 +36,23 @@ public abstract class SourceGenerator {
private static final List<String> NO_STORED_FIELD = singletonList(StoredFieldsContext._NONE_);
public static SearchSourceBuilder sourceBuilder(QueryContainer container, QueryBuilder filter, Integer size) {
final SearchSourceBuilder source = new SearchSourceBuilder();
QueryBuilder finalQuery = null;
// add the source
if (container.query() == null) {
if (container.query() != null) {
if (filter != null) {
source.query(new ConstantScoreQueryBuilder(filter));
finalQuery = boolQuery().must(container.query().asBuilder()).filter(filter);
} else {
finalQuery = container.query().asBuilder();
}
} else {
if (filter != null) {
source.query(new BoolQueryBuilder().must(container.query().asBuilder()).filter(filter));
} else {
source.query(container.query().asBuilder());
finalQuery = boolQuery().filter(filter);
}
}
final SearchSourceBuilder source = new SearchSourceBuilder();
source.query(finalQuery);
SqlSourceBuilder sortBuilder = new SqlSourceBuilder();
// Iterate through all the columns requested, collecting the fields that
// need to be retrieved from the result documents

9
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/BoolQuery.java
View File

@ -5,13 +5,13 @@
*/
package org.elasticsearch.xpack.sql.querydsl.query;
import java.util.Objects;
import org.elasticsearch.index.query.BoolQueryBuilder;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.search.sort.NestedSortBuilder;
import org.elasticsearch.xpack.sql.tree.Location;
import java.util.Objects;
import static org.elasticsearch.index.query.QueryBuilders.boolQuery;
/**
@ -63,9 +63,8 @@ public class BoolQuery extends Query {
public QueryBuilder asBuilder() {
BoolQueryBuilder boolQuery = boolQuery();
if (isAnd) {
// TODO are we throwing out score by using filter?
boolQuery.filter(left.asBuilder());
boolQuery.filter(right.asBuilder());
boolQuery.must(left.asBuilder());
boolQuery.must(right.asBuilder());
} else {
boolQuery.should(left.asBuilder());
boolQuery.should(right.asBuilder());

17
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/SourceGeneratorTests.java
View File

@ -5,9 +5,6 @@
*/
package org.elasticsearch.xpack.sql.execution.search;
import org.elasticsearch.index.query.BoolQueryBuilder;
import org.elasticsearch.index.query.ConstantScoreQueryBuilder;
import org.elasticsearch.index.query.MatchQueryBuilder;
import org.elasticsearch.index.query.Operator;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.search.aggregations.AggregatorFactories.Builder;
@ -28,6 +25,8 @@ import org.elasticsearch.xpack.sql.tree.Location;
import org.elasticsearch.xpack.sql.type.KeywordEsField;
import static java.util.Collections.singletonList;
import static org.elasticsearch.index.query.QueryBuilders.boolQuery;
import static org.elasticsearch.index.query.QueryBuilders.matchQuery;
import static org.elasticsearch.search.sort.SortBuilders.fieldSort;
import static org.elasticsearch.search.sort.SortBuilders.scoreSort;
@ -42,22 +41,22 @@ public class SourceGeneratorTests extends ESTestCase {
public void testQueryNoFilter() {
QueryContainer container = new QueryContainer().with(new MatchQuery(Location.EMPTY, "foo", "bar"));
SearchSourceBuilder sourceBuilder = SourceGenerator.sourceBuilder(container, null, randomIntBetween(1, 10));
assertEquals(new MatchQueryBuilder("foo", "bar").operator(Operator.OR), sourceBuilder.query());
assertEquals(matchQuery("foo", "bar").operator(Operator.OR), sourceBuilder.query());
}
public void testNoQueryFilter() {
QueryContainer container = new QueryContainer();
QueryBuilder filter = new MatchQueryBuilder("bar", "baz");
QueryBuilder filter = matchQuery("bar", "baz");
SearchSourceBuilder sourceBuilder = SourceGenerator.sourceBuilder(container, filter, randomIntBetween(1, 10));
assertEquals(new ConstantScoreQueryBuilder(new MatchQueryBuilder("bar", "baz")), sourceBuilder.query());
assertEquals(boolQuery().filter(matchQuery("bar", "baz")), sourceBuilder.query());
}
public void testQueryFilter() {
QueryContainer container = new QueryContainer().with(new MatchQuery(Location.EMPTY, "foo", "bar"));
QueryBuilder filter = new MatchQueryBuilder("bar", "baz");
QueryBuilder filter = matchQuery("bar", "baz");
SearchSourceBuilder sourceBuilder = SourceGenerator.sourceBuilder(container, filter, randomIntBetween(1, 10));
assertEquals(new BoolQueryBuilder().must(new MatchQueryBuilder("foo", "bar").operator(Operator.OR))
.filter(new MatchQueryBuilder("bar", "baz")), sourceBuilder.query());
assertEquals(boolQuery().must(matchQuery("foo", "bar").operator(Operator.OR)).filter(matchQuery("bar", "baz")),
sourceBuilder.query());
}
public void testLimit() {

20
x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/rest/RestSqlTestCase.java
View File

@ -16,7 +16,6 @@ import org.elasticsearch.client.Request;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.common.CheckedSupplier;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.io.Streams;
import org.elasticsearch.common.xcontent.XContentHelper;
@ -33,12 +32,11 @@ import java.nio.charset.StandardCharsets;
import java.sql.JDBCType;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.TreeMap;
import static java.util.Collections.emptyList;
import static java.util.Collections.emptyMap;
import static java.util.Collections.singletonList;
import static java.util.Collections.singletonMap;
import static java.util.Collections.unmodifiableMap;
@ -396,19 +394,23 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe
assertNotNull(query);
@SuppressWarnings("unchecked")
Map<String, Object> constantScore = (Map<String, Object>) query.get("constant_score");
assertNotNull(constantScore);
Map<String, Object> bool = (Map<String, Object>) query.get("bool");
assertNotNull(bool);
@SuppressWarnings("unchecked")
Map<String, Object> filter = (Map<String, Object>) constantScore.get("filter");
List<Object> filter = (List<Object>) bool.get("filter");
assertNotNull(filter);
@SuppressWarnings("unchecked")
Map<String, Object> match = (Map<String, Object>) filter.get("match");
assertNotNull(match);
Map<String, Object> map = (Map<String, Object>) filter.get(0);
assertNotNull(map);
@SuppressWarnings("unchecked")
Map<String, Object> matchQuery = (Map<String, Object>) match.get("test");
Map<String, Object> matchQ = (Map<String, Object>) map.get("match");
@SuppressWarnings("unchecked")
Map<String, Object> matchQuery = (Map<String, Object>) matchQ.get("test");
assertNotNull(matchQuery);
assertEquals("foo", matchQuery.get("query"));
}