Skip to content

Commit

Permalink
fix(search): Elasticsearch bool query should (#11536)
Browse files Browse the repository at this point in the history
  • Loading branch information
@david-leifker
david-leifker authored Oct 4, 2024
1 parent ba2f1d3 commit 09e9d83
Show file tree
Hide file tree
Showing 16 changed files with 109 additions and 33 deletions.
22 changes: 20 additions & 2 deletions metadata-io/src/main/java/com/linkedin/metadata/graph/elastic/ESGraphQueryDAO.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,9 @@ private static void addFilterToQueryBuilder(
criterion.getValues())));
orQuery.should(andQuery);
}
if (!orQuery.should().isEmpty()) {
orQuery.minimumShouldMatch(1);
}
rootQuery.filter(orQuery);
}

Expand Down Expand Up @@ -177,21 +180,26 @@ private SearchResponse executeGroupByLineageSearchQuery(
// directions for lineage
// set up filters for each relationship type in the correct direction to limit buckets
BoolQueryBuilder sourceFilterQuery = QueryBuilders.boolQuery();
sourceFilterQuery.minimumShouldMatch(1);

validEdges.stream()
.filter(pair -> RelationshipDirection.OUTGOING.equals(pair.getValue().getDirection()))
.forEach(
pair ->
sourceFilterQuery.should(
getAggregationFilter(pair, RelationshipDirection.OUTGOING)));
if (!sourceFilterQuery.should().isEmpty()) {
sourceFilterQuery.minimumShouldMatch(1);
}

BoolQueryBuilder destFilterQuery = QueryBuilders.boolQuery();
destFilterQuery.minimumShouldMatch(1);
validEdges.stream()
.filter(pair -> RelationshipDirection.INCOMING.equals(pair.getValue().getDirection()))
.forEach(
pair ->
destFilterQuery.should(getAggregationFilter(pair, RelationshipDirection.INCOMING)));
if (!destFilterQuery.should().isEmpty()) {
destFilterQuery.minimumShouldMatch(1);
}

FilterAggregationBuilder sourceRelationshipTypeFilters =
AggregationBuilders.filter(FILTER_BY_SOURCE_RELATIONSHIP, sourceFilterQuery);
Expand Down Expand Up @@ -347,6 +355,9 @@ public static BoolQueryBuilder buildQuery(
relationshipType ->
relationshipQuery.should(
QueryBuilders.termQuery(RELATIONSHIP_TYPE, relationshipType)));
if (!relationshipQuery.should().isEmpty()) {
relationshipQuery.minimumShouldMatch(1);
}
finalQuery.filter(relationshipQuery);
}

Expand Down Expand Up @@ -697,6 +708,9 @@ public static QueryBuilder getLineageQuery(
urns, edgesPerEntityType.get(entityType), graphFilters));
}
});
if (!entityTypeQueries.should().isEmpty()) {
entityTypeQueries.minimumShouldMatch(1);
}

BoolQueryBuilder finalQuery = QueryBuilders.boolQuery();

Expand Down Expand Up @@ -741,6 +755,10 @@ static QueryBuilder getLineageQueryForEntityType(
query.should(getIncomingEdgeQuery(urns, incomingEdges, graphFilters));
}

if (!query.should().isEmpty()) {
query.minimumShouldMatch(1);
}

return query;
}

Expand Down
11 changes: 7 additions & 4 deletions metadata-io/src/main/java/com/linkedin/metadata/graph/elastic/GraphFilterUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,14 @@ public static QueryBuilder getUrnStatusQuery(
if (removed) {
finalQuery.filter(QueryBuilders.termQuery(statusField, removed.toString()));
} else {
finalQuery.minimumShouldMatch(1);
finalQuery.should(QueryBuilders.termQuery(statusField, removed.toString()));
finalQuery.should(QueryBuilders.boolQuery().mustNot(QueryBuilders.existsQuery(statusField)));
}

if (!finalQuery.should().isEmpty()) {
finalQuery.minimumShouldMatch(1);
}

return finalQuery;
}

Expand Down Expand Up @@ -102,7 +105,7 @@ public static QueryBuilder getEdgeTimeFilterQuery(
* 2. The createdOn and updatedOn window does not exist on the edge at all (support legacy cases)
* 3. Special lineage case: The edge is marked as a "manual" edge, meaning that the time filters should NOT be applied.
*/
BoolQueryBuilder timeFilterQuery = QueryBuilders.boolQuery();
BoolQueryBuilder timeFilterQuery = QueryBuilders.boolQuery().minimumShouldMatch(1);
timeFilterQuery.should(buildTimeWindowFilter(startTimeMillis, endTimeMillis));
timeFilterQuery.should(buildTimestampsMissingFilter());
timeFilterQuery.should(buildManualLineageFilter());
Expand Down Expand Up @@ -158,7 +161,7 @@ public static QueryBuilder getEdgeTimeFilterQuery(
*/
private static QueryBuilder buildTimeWindowFilter(
final long startTimeMillis, final long endTimeMillis) {
final BoolQueryBuilder timeWindowQuery = QueryBuilders.boolQuery();
final BoolQueryBuilder timeWindowQuery = QueryBuilders.boolQuery().minimumShouldMatch(1);

/*
* To perform comparison:
Expand Down Expand Up @@ -198,7 +201,7 @@ private static QueryBuilder buildTimestampsMissingFilter() {

private static QueryBuilder buildNotExistsFilter(String fieldName) {
// This filter returns 'true' if the field DOES NOT EXIST or it exists but is equal to 0.
final BoolQueryBuilder notExistsFilter = QueryBuilders.boolQuery();
final BoolQueryBuilder notExistsFilter = QueryBuilders.boolQuery().minimumShouldMatch(1);
notExistsFilter.should(QueryBuilders.boolQuery().mustNot(QueryBuilders.existsQuery(fieldName)));
notExistsFilter.should(QueryBuilders.boolQuery().must(QueryBuilders.termQuery(fieldName, 0L)));
return notExistsFilter;
Expand Down
5 changes: 4 additions & 1 deletion ...java/com/linkedin/metadata/search/elasticsearch/query/filter/BaseQueryFilterRewriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,12 @@ private BoolQueryBuilder handleNestedFilters(
mustNotQueryBuilders.forEach(expandedQueryBuilder::mustNot);
expandedQueryBuilder.queryName(boolQueryBuilder.queryName());
expandedQueryBuilder.adjustPureNegative(boolQueryBuilder.adjustPureNegative());
expandedQueryBuilder.minimumShouldMatch(boolQueryBuilder.minimumShouldMatch());
expandedQueryBuilder.boost(boolQueryBuilder.boost());

if (!expandedQueryBuilder.should().isEmpty()) {
expandedQueryBuilder.minimumShouldMatch(1);
}

return expandedQueryBuilder;
}

Expand Down
13 changes: 7 additions & 6 deletions .../com/linkedin/metadata/search/elasticsearch/query/request/AutocompleteRequestHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,7 @@ public SearchRequest getSearchRequest(
QueryConfiguration customQueryConfig =
customizedQueryHandler.lookupQueryConfig(input).orElse(null);

BoolQueryBuilder baseQuery = QueryBuilders.boolQuery();
baseQuery.minimumShouldMatch(1);
BoolQueryBuilder baseQuery = QueryBuilders.boolQuery().minimumShouldMatch(1);

// Initial query with input filters
BoolQueryBuilder filterQuery =
Expand Down Expand Up @@ -176,12 +175,15 @@ public BoolQueryBuilder getQuery(
BoolQueryBuilder finalQuery =
Optional.ofNullable(customAutocompleteConfig)
.flatMap(cac -> CustomizedQueryHandler.boolQueryBuilder(objectMapper, cac, query))
.orElse(QueryBuilders.boolQuery())
.minimumShouldMatch(1);
.orElse(QueryBuilders.boolQuery());

getAutocompleteQuery(customAutocompleteConfig, autocompleteFields, query)
.ifPresent(finalQuery::should);

if (!finalQuery.should().isEmpty()) {
finalQuery.minimumShouldMatch(1);
}

return finalQuery;
}

Expand All @@ -200,8 +202,7 @@ private Optional<QueryBuilder> getAutocompleteQuery(

private static BoolQueryBuilder defaultQuery(
List<String> autocompleteFields, @Nonnull String query) {
BoolQueryBuilder finalQuery = QueryBuilders.boolQuery();
finalQuery.minimumShouldMatch(1);
BoolQueryBuilder finalQuery = QueryBuilders.boolQuery().minimumShouldMatch(1);

// Search for exact matches with higher boost and ngram matches
MultiMatchQueryBuilder autocompleteQueryBuilder =
Expand Down
15 changes: 12 additions & 3 deletions ...ain/java/com/linkedin/metadata/search/elasticsearch/query/request/SearchQueryBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,7 @@ private QueryBuilder buildInternalQuery(
cqc ->
CustomizedQueryHandler.boolQueryBuilder(
opContext.getObjectMapper(), cqc, sanitizedQuery))
.orElse(QueryBuilders.boolQuery())
.minimumShouldMatch(1);
.orElse(QueryBuilders.boolQuery());

if (fulltext && !query.startsWith(STRUCTURED_QUERY_PREFIX)) {
getSimpleQuery(opContext.getEntityRegistry(), customQueryConfig, entitySpecs, sanitizedQuery)
Expand Down Expand Up @@ -135,6 +134,10 @@ private QueryBuilder buildInternalQuery(
}
}

if (!finalQuery.should().isEmpty()) {
finalQuery.minimumShouldMatch(1);
}

return finalQuery;
}

Expand Down Expand Up @@ -368,6 +371,10 @@ private Optional<QueryBuilder> getSimpleQuery(
simplePerField.should(simpleBuilder);
});

if (!simplePerField.should().isEmpty()) {
simplePerField.minimumShouldMatch(1);
}

result = Optional.of(simplePerField);
}

Expand Down Expand Up @@ -454,7 +461,9 @@ private Optional<QueryBuilder> getPrefixAndExactMatchQuery(
}
});

return finalQuery.should().size() > 0 ? Optional.of(finalQuery) : Optional.empty();
return finalQuery.should().size() > 0
? Optional.of(finalQuery.minimumShouldMatch(1))
: Optional.empty();
}

private Optional<QueryBuilder> getStructuredQuery(
Expand Down
9 changes: 4 additions & 5 deletions metadata-io/src/main/java/com/linkedin/metadata/search/utils/ESUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,6 @@ public static BoolQueryBuilder buildFilterQuery(
searchableFieldTypes,
opContext,
queryFilterRewriteChain)));
// The default is not always 1 (ensure consistent default)
finalQueryBuilder.minimumShouldMatch(1);
} else if (filter.getCriteria() != null) {
// Otherwise, build boolean query from the deprecated "criteria" field.
log.warn("Received query Filter with a deprecated field 'criteria'. Use 'or' instead.");
Expand All @@ -187,7 +185,8 @@ public static BoolQueryBuilder buildFilterQuery(
}
});
finalQueryBuilder.should(andQueryBuilder);
// The default is not always 1 (ensure consistent default)
}
if (!finalQueryBuilder.should().isEmpty()) {
finalQueryBuilder.minimumShouldMatch(1);
}
return finalQueryBuilder;
Expand Down Expand Up @@ -533,7 +532,7 @@ private static QueryBuilder getQueryBuilderFromCriterionForFieldToExpand(
final Map<String, Set<SearchableAnnotation.FieldType>> searchableFieldTypes,
@Nonnull OperationContext opContext,
@Nonnull QueryFilterRewriteChain queryFilterRewriteChain) {
final BoolQueryBuilder orQueryBuilder = new BoolQueryBuilder();
final BoolQueryBuilder orQueryBuilder = new BoolQueryBuilder().minimumShouldMatch(1);
for (String field : fields) {
orQueryBuilder.should(
getQueryBuilderFromCriterionForSingleField(
Expand Down Expand Up @@ -636,7 +635,7 @@ private static QueryBuilder buildWildcardQueryWithMultipleValues(
@Nullable String queryName,
@Nonnull AspectRetriever aspectRetriever,
String wildcardPattern) {
BoolQueryBuilder boolQuery = QueryBuilders.boolQuery();
BoolQueryBuilder boolQuery = QueryBuilders.boolQuery().minimumShouldMatch(1);

for (String value : criterion.getValues()) {
boolQuery.should(
Expand Down
1 change: 0 additions & 1 deletion metadata-io/src/test/java/com/linkedin/metadata/search/fixtures/GoldenTestBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import static com.linkedin.metadata.utils.CriterionUtils.buildCriterion;
import static io.datahubproject.test.search.SearchTestUtils.searchAcrossEntities;
import static org.testng.Assert.*;
import static org.testng.AssertJUnit.assertNotNull;

import com.google.common.collect.ImmutableList;
import com.linkedin.common.urn.Urn;
Expand Down
11 changes: 8 additions & 3 deletions ...c/test/java/com/linkedin/metadata/search/query/filter/ContainerExpansionRewriterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,16 +311,19 @@ public void testNestedBoolQueryRewrite() {
new RelatedEntities(
"IsPartOf", childUrn, parentUrn, RelationshipDirection.OUTGOING, null))));

BoolQueryBuilder testQuery = QueryBuilders.boolQuery();
BoolQueryBuilder testQuery = QueryBuilders.boolQuery().minimumShouldMatch(1);
testQuery.filter(
QueryBuilders.boolQuery()
.filter(
QueryBuilders.boolQuery().filter(QueryBuilders.termsQuery(FIELD_NAME, childUrn))));
testQuery.filter(QueryBuilders.existsQuery("someField"));
testQuery.should(
QueryBuilders.boolQuery()
.minimumShouldMatch(1)
.should(
QueryBuilders.boolQuery().should(QueryBuilders.termsQuery(FIELD_NAME, childUrn))));
QueryBuilders.boolQuery()
.minimumShouldMatch(1)
.should(QueryBuilders.termsQuery(FIELD_NAME, childUrn))));
testQuery.should(QueryBuilders.existsQuery("someField"));
testQuery.must(
QueryBuilders.boolQuery()
Expand All @@ -332,7 +335,7 @@ public void testNestedBoolQueryRewrite() {
QueryBuilders.boolQuery().mustNot(QueryBuilders.termsQuery(FIELD_NAME, childUrn))));
testQuery.mustNot(QueryBuilders.existsQuery("someField"));

BoolQueryBuilder expectedRewrite = QueryBuilders.boolQuery();
BoolQueryBuilder expectedRewrite = QueryBuilders.boolQuery().minimumShouldMatch(1);
expectedRewrite.filter(
QueryBuilders.boolQuery()
.filter(
Expand All @@ -341,8 +344,10 @@ public void testNestedBoolQueryRewrite() {
expectedRewrite.filter(QueryBuilders.existsQuery("someField"));
expectedRewrite.should(
QueryBuilders.boolQuery()
.minimumShouldMatch(1)
.should(
QueryBuilders.boolQuery()
.minimumShouldMatch(1)
.should(QueryBuilders.termsQuery(FIELD_NAME, childUrn, parentUrn))));
expectedRewrite.should(QueryBuilders.existsQuery("someField"));
expectedRewrite.must(
Expand Down
20 changes: 15 additions & 5 deletions .../src/test/java/com/linkedin/metadata/search/query/filter/DomainExpansionRewriterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,17 +312,23 @@ public void testNestedBoolQueryRewrite() {
new RelatedEntities(
"IsPartOf", childUrn, parentUrn, RelationshipDirection.INCOMING, null))));

BoolQueryBuilder testQuery = QueryBuilders.boolQuery();
BoolQueryBuilder testQuery = QueryBuilders.boolQuery().minimumShouldMatch(1);
testQuery.filter(
QueryBuilders.boolQuery()
.filter(
QueryBuilders.boolQuery().filter(QueryBuilders.termsQuery(FIELD_NAME, parentUrn))));
testQuery.filter(QueryBuilders.boolQuery().filter(QueryBuilders.existsQuery("someField")));
testQuery.should(
QueryBuilders.boolQuery()
.minimumShouldMatch(1)
.should(
QueryBuilders.boolQuery().should(QueryBuilders.termsQuery(FIELD_NAME, parentUrn))));
testQuery.should(QueryBuilders.boolQuery().should(QueryBuilders.existsQuery("someField")));
QueryBuilders.boolQuery()
.minimumShouldMatch(1)
.should(QueryBuilders.termsQuery(FIELD_NAME, parentUrn))));
testQuery.should(
QueryBuilders.boolQuery()
.minimumShouldMatch(1)
.should(QueryBuilders.existsQuery("someField")));
testQuery.must(
QueryBuilders.boolQuery()
.must(QueryBuilders.boolQuery().must(QueryBuilders.termsQuery(FIELD_NAME, parentUrn))));
Expand All @@ -334,7 +340,7 @@ public void testNestedBoolQueryRewrite() {
.mustNot(QueryBuilders.termsQuery(FIELD_NAME, parentUrn))));
testQuery.mustNot(QueryBuilders.boolQuery().mustNot(QueryBuilders.existsQuery("someField")));

BoolQueryBuilder expectedRewrite = QueryBuilders.boolQuery();
BoolQueryBuilder expectedRewrite = QueryBuilders.boolQuery().minimumShouldMatch(1);
expectedRewrite.filter(
QueryBuilders.boolQuery()
.filter(
Expand All @@ -344,11 +350,15 @@ public void testNestedBoolQueryRewrite() {
QueryBuilders.boolQuery().filter(QueryBuilders.existsQuery("someField")));
expectedRewrite.should(
QueryBuilders.boolQuery()
.minimumShouldMatch(1)
.should(
QueryBuilders.boolQuery()
.minimumShouldMatch(1)
.should(QueryBuilders.termsQuery(FIELD_NAME, childUrn, parentUrn))));
expectedRewrite.should(
QueryBuilders.boolQuery().should(QueryBuilders.existsQuery("someField")));
QueryBuilders.boolQuery()
.minimumShouldMatch(1)
.should(QueryBuilders.existsQuery("someField")));
expectedRewrite.must(
QueryBuilders.boolQuery()
.must(
Expand Down
Loading

0 comments on commit 09e9d83

Please sign in to comment.