Skip to content

Add the filter parts of a CriteriaQuery to the query, not as post-filter. #2906

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.

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
merged 1 commit into from
May 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,13 @@ public static Optional<Query> createQuery(Criteria criteria) {
for (Criteria chainedCriteria : criteria.getCriteriaChain()) {

if (chainedCriteria.isOr()) {
BoolQuery.Builder boolQueryBuilder = QueryBuilders.bool();
queriesForEntries(chainedCriteria).forEach(boolQueryBuilder::should);
filterQueries.add(new Query(boolQueryBuilder.build()));
Collection<? extends Query> queriesForEntries = queriesForEntries(chainedCriteria);

if (!queriesForEntries.isEmpty()) {
BoolQuery.Builder boolQueryBuilder = QueryBuilders.bool();
queriesForEntries.forEach(boolQueryBuilder::should);
filterQueries.add(new Query(boolQueryBuilder.build()));
}
} else if (chainedCriteria.isNegating()) {

Assert.notNull(criteria.getField(), "criteria must have a field");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.Objects;

import org.springframework.data.elasticsearch.annotations.FieldType;
import org.springframework.data.elasticsearch.core.query.Criteria;
Expand Down Expand Up @@ -115,11 +116,18 @@ public static Query createQuery(Criteria criteria) {
}
}

var filterQuery = CriteriaFilterProcessor.createQuery(criteria);
if (shouldQueries.isEmpty() && mustNotQueries.isEmpty() && mustQueries.isEmpty()) {
return null;

if (filterQuery.isEmpty()) {
return null;
}

// we need something to add the filter to
mustQueries.add(Query.of(qb -> qb.matchAll(m -> m)));
}

Query query = new Query.Builder().bool(boolQueryBuilder -> {
return new Query.Builder().bool(boolQueryBuilder -> {

if (!shouldQueries.isEmpty()) {
boolQueryBuilder.should(shouldQueries);
Expand All @@ -133,10 +141,10 @@ public static Query createQuery(Criteria criteria) {
boolQueryBuilder.must(mustQueries);
}

filterQuery.ifPresent(boolQueryBuilder::filter);

return boolQueryBuilder;
}).build();

return query;
}

@Nullable
Expand Down Expand Up @@ -238,7 +246,7 @@ private static Query.Builder queryFor(Criteria.CriteriaEntry entry, Field field,
queryBuilder.queryString(queryStringQuery(fieldName, '*' + searchText, true, boost));
break;
case EXPRESSION:
queryBuilder.queryString(queryStringQuery(fieldName, value.toString(), boost));
queryBuilder.queryString(queryStringQuery(fieldName, Objects.requireNonNull(value).toString(), boost));
break;
case LESS:
queryBuilder //
Expand Down Expand Up @@ -270,6 +278,7 @@ private static Query.Builder queryFor(Criteria.CriteriaEntry entry, Field field,
break;
case BETWEEN:
Object[] ranges = (Object[]) value;
Assert.notNull(value, "value for a between condition must not be null");
queryBuilder //
.range(rb -> {
rb.field(fieldName);
Expand All @@ -293,10 +302,10 @@ private static Query.Builder queryFor(Criteria.CriteriaEntry entry, Field field,
.boost(boost)); //
break;
case MATCHES:
queryBuilder.match(matchQuery(fieldName, value.toString(), Operator.Or, boost));
queryBuilder.match(matchQuery(fieldName, Objects.requireNonNull(value).toString(), Operator.Or, boost));
break;
case MATCHES_ALL:
queryBuilder.match(matchQuery(fieldName, value.toString(), Operator.And, boost));
queryBuilder.match(matchQuery(fieldName, Objects.requireNonNull(value).toString(), Operator.And, boost));

break;
case IN:
Expand Down Expand Up @@ -345,7 +354,7 @@ private static Query.Builder queryFor(Criteria.CriteriaEntry entry, Field field,
queryBuilder //
.regexp(rb -> rb //
.field(fieldName) //
.value(value.toString()) //
.value(Objects.requireNonNull(value).toString()) //
.boost(boost)); //
break;
case HAS_CHILD:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1210,7 +1210,7 @@ public <T> SearchRequest searchRequest(Query query, @Nullable String routing, @N
builder.routing(routing);
}

addFilter(query, builder);
addPostFilter(query, builder);

return builder.build();
}
Expand Down Expand Up @@ -1758,22 +1758,18 @@ co.elastic.clients.elasticsearch._types.query_dsl.Query getQuery(@Nullable Query
return getEsQuery(query, (q) -> elasticsearchConverter.updateQuery(q, clazz));
}

private void addFilter(Query query, SearchRequest.Builder builder) {
@SuppressWarnings("StatementWithEmptyBody")
private void addPostFilter(Query query, SearchRequest.Builder builder) {

if (query instanceof CriteriaQuery) {
CriteriaFilterProcessor.createQuery(((CriteriaQuery) query).getCriteria()).ifPresent(builder::postFilter);
} else // noinspection StatementWithEmptyBody
if (query instanceof StringQuery) {
// no filter for StringQuery
} else if (query instanceof NativeQuery nativeQuery) {
// we only need to handle NativeQuery here. filter from a CriteriaQuery are added into the query and not as post
// filter anymore, StringQuery do not have post filters
if (query instanceof NativeQuery nativeQuery) {

if (nativeQuery.getFilter() != null) {
builder.postFilter(nativeQuery.getFilter());
} else if (nativeQuery.getSpringDataQuery() != null) {
addFilter(nativeQuery.getSpringDataQuery(), builder);
addPostFilter(nativeQuery.getSpringDataQuery(), builder);
}
} else {
throw new IllegalArgumentException("unhandled Query implementation " + query.getClass().getName());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@

import static org.assertj.core.api.Assertions.*;

import co.elastic.clients.elasticsearch._types.GeoHashPrecision;
import co.elastic.clients.elasticsearch._types.aggregations.Aggregation;

import java.util.List;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Order;
Expand All @@ -26,6 +31,7 @@
import org.springframework.data.elasticsearch.annotations.Document;
import org.springframework.data.elasticsearch.annotations.Field;
import org.springframework.data.elasticsearch.annotations.FieldType;
import org.springframework.data.elasticsearch.client.elc.ElasticsearchAggregation;
import org.springframework.data.elasticsearch.client.elc.NativeQuery;
import org.springframework.data.elasticsearch.core.ElasticsearchOperations;
import org.springframework.data.elasticsearch.core.geo.GeoPoint;
Expand Down Expand Up @@ -98,6 +104,40 @@ void shouldBeAbleToUseCriteriaQueryWithFilterArgumentsInANativeQuery() {
assertThat(searchHits.getSearchHit(0).getId()).isEqualTo(entity1.getId());
}

@Test // #2857
@DisplayName("should apply CriteriaQuery with filter arguments in a NativeQuery to aggregations")
void shouldBeAbleToUseCriteriaQueryWithFilterArgumentsInANativeQueryToAggregations() {
var entity1 = new SampleEntity();
entity1.setId("60");
var location1 = new GeoPoint(60.0, 60.0);
entity1.setLocation(location1);
entity1.setText("60");
var entity2 = new SampleEntity();
entity2.setId("70");
entity2.setText("70");
var location70 = new GeoPoint(70.0, 70.0);
entity2.setLocation(location70);
operations.save(entity1, entity2);

var criteriaQuery = new CriteriaQuery(Criteria.where("location").within(location1, "10km"));
var nativeQuery = NativeQuery.builder()
.withQuery(criteriaQuery)
.withAggregation("geohashgrid", Aggregation.of(ab -> ab
.geohashGrid(ghg -> ghg
.field("location")
.precision(GeoHashPrecision.of(ghp -> ghp.distance("10000km"))))))
.build();

var searchHits = operations.search(nativeQuery, SampleEntity.class);

assertThat(searchHits.getTotalHits()).isEqualTo(1);
assertThat(searchHits.getSearchHit(0).getId()).isEqualTo(entity1.getId());
assertThat(searchHits.getAggregations()).isNotNull();
// noinspection unchecked
var aggregations = (List<ElasticsearchAggregation>) searchHits.getAggregations().aggregations();
assertThat(aggregations).hasSize(1);
}

@Test // #2391
@DisplayName("should be able to use StringQuery in a NativeQuery")
void shouldBeAbleToUseStringQueryInANativeQuery() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,47 +79,27 @@ void cleanup() {
@Test
public void shouldSupportAND() {

// given

// when

// then
assertThat(repository.findByNameAndText("Sugar", "Cane sugar")).hasSize(2);
assertThat(repository.findByNameAndPrice("Sugar", 1.1f)).hasSize(1);
}

@Test
public void shouldSupportOR() {

// given

// when

// then
assertThat(repository.findByNameOrPrice("Sugar", 1.9f)).hasSize(4);
assertThat(repository.findByNameOrText("Salt", "Beet sugar")).hasSize(3);
}

@Test
public void shouldSupportTrueAndFalse() {

// given

// when

// then
assertThat(repository.findByAvailableTrue()).hasSize(3);
assertThat(repository.findByAvailableFalse()).hasSize(4);
}

@Test
public void shouldSupportInAndNotInAndNot() {

// given

// when

// then
assertThat(repository.findByPriceIn(Arrays.asList(1.2f, 1.1f))).hasSize(2);
assertThat(repository.findByPriceNotIn(Arrays.asList(1.2f, 1.1f))).hasSize(5);
assertThat(repository.findByPriceNot(1.2f)).hasSize(6);
Expand All @@ -128,33 +108,18 @@ public void shouldSupportInAndNotInAndNot() {
@Test // DATAES-171
public void shouldWorkWithNotIn() {

// given

// when

// then
assertThat(repository.findByIdNotIn(Arrays.asList("2", "3"))).hasSize(5);
}

@Test
public void shouldSupportBetween() {

// given

// when

// then
assertThat(repository.findByPriceBetween(1.0f, 2.0f)).hasSize(4);
}

@Test
public void shouldSupportLessThanAndGreaterThan() {

// given

// when

// then
assertThat(repository.findByPriceLessThan(1.1f)).hasSize(1);
assertThat(repository.findByPriceLessThanEqual(1.1f)).hasSize(2);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
package org.springframework.data.elasticsearch.utils;

/**
* Class providing an index name with a prefix and an index number
* Class providing an index name with a prefix, an index number and a random 6-digit number.
*
* @author Peter-Josef Meisch
*/
Expand All @@ -35,7 +35,7 @@ public IndexNameProvider(String prefix) {
}

public void increment() {
indexName = prefix + '-' + ++idx;
indexName = prefix + '-' + ++idx + '-' + sixDigits();
}

public String indexName() {
Expand All @@ -48,4 +48,8 @@ public String indexName() {
public String getPrefix() {
return prefix;
}

private String sixDigits() {
return String.valueOf((int) (100000 + Math.random() * 900000));
}
}