Deprecate using 0 value for min_children in has_child query (#41555)

After changing the allowed minimum value for min_children in has_child query from 0 to 1 in 
the next major version, this PR adds a deprecation warning for these cases.

Closes #41548
This commit is contained in:
Hicham Mallah 2019-04-26 22:00:11 +03:00 committed by Christoph Büscher
parent d7ab86db9c
commit 22f3b53ed7
3 changed files with 34 additions and 20 deletions

View File

@ -32,6 +32,8 @@ import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.lucene.search.Queries;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.apache.logging.log4j.LogManager;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.fielddata.IndexOrdinalsFieldData;
@ -66,13 +68,14 @@ public class HasChildQueryBuilder extends AbstractQueryBuilder<HasChildQueryBuil
/**
* The default minimum number of children that are required to match for the parent to be considered a match.
*/
public static final int DEFAULT_MIN_CHILDREN = 0;
public static final int DEFAULT_MIN_CHILDREN = 1;
/**
* The default value for ignore_unmapped.
*/
public static final boolean DEFAULT_IGNORE_UNMAPPED = false;
static final String MIN_CHILDREN_0_DEPRECATION_MESSAGE = "[min_children] 0 will be rejected " +
"starting in 8.0. Using 1 instead of 0 will return the same result set.";
private static final ParseField QUERY_FIELD = new ParseField("query");
private static final ParseField TYPE_FIELD = new ParseField("type");
private static final ParseField MAX_CHILDREN_FIELD = new ParseField("max_children");
@ -80,7 +83,8 @@ public class HasChildQueryBuilder extends AbstractQueryBuilder<HasChildQueryBuil
private static final ParseField SCORE_MODE_FIELD = new ParseField("score_mode");
private static final ParseField INNER_HITS_FIELD = new ParseField("inner_hits");
private static final ParseField IGNORE_UNMAPPED_FIELD = new ParseField("ignore_unmapped");
private static final DeprecationLogger deprecationLogger = new DeprecationLogger(
LogManager.getLogger(HasChildQueryBuilder.class));
private final QueryBuilder query;
private final String type;
private final ScoreMode scoreMode;
@ -136,6 +140,9 @@ public class HasChildQueryBuilder extends AbstractQueryBuilder<HasChildQueryBuil
if (minChildren < 0) {
throw new IllegalArgumentException("[" + NAME + "] requires non-negative 'min_children' field");
}
if (minChildren == 0) {
deprecationLogger.deprecatedAndMaybeLog("min_children", MIN_CHILDREN_0_DEPRECATION_MESSAGE);
}
if (maxChildren < 0) {
throw new IllegalArgumentException("[" + NAME + "] requires non-negative 'max_children' field");
}

View File

@ -1396,7 +1396,7 @@ public class ChildQuerySearchIT extends ParentChildTestCase {
SearchResponse response;
// Score mode = NONE
response = minMaxQuery(ScoreMode.None, 0, null);
response = minMaxQuery(ScoreMode.None, 1, null);
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
assertThat(response.getHits().getHits()[0].getId(), equalTo("2"));
@ -1434,7 +1434,7 @@ public class ChildQuerySearchIT extends ParentChildTestCase {
assertThat(response.getHits().getTotalHits().value, equalTo(0L));
response = minMaxQuery(ScoreMode.None, 0, 4);
response = minMaxQuery(ScoreMode.None, 1, 4);
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
assertThat(response.getHits().getHits()[0].getId(), equalTo("2"));
@ -1444,7 +1444,7 @@ public class ChildQuerySearchIT extends ParentChildTestCase {
assertThat(response.getHits().getHits()[2].getId(), equalTo("4"));
assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
response = minMaxQuery(ScoreMode.None, 0, 3);
response = minMaxQuery(ScoreMode.None, 1, 3);
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
assertThat(response.getHits().getHits()[0].getId(), equalTo("2"));
@ -1454,7 +1454,7 @@ public class ChildQuerySearchIT extends ParentChildTestCase {
assertThat(response.getHits().getHits()[2].getId(), equalTo("4"));
assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
response = minMaxQuery(ScoreMode.None, 0, 2);
response = minMaxQuery(ScoreMode.None, 1, 2);
assertThat(response.getHits().getTotalHits().value, equalTo(2L));
assertThat(response.getHits().getHits()[0].getId(), equalTo("2"));
@ -1472,7 +1472,7 @@ public class ChildQuerySearchIT extends ParentChildTestCase {
assertThat(e.getMessage(), equalTo("[has_child] 'max_children' is less than 'min_children'"));
// Score mode = SUM
response = minMaxQuery(ScoreMode.Total, 0, null);
response = minMaxQuery(ScoreMode.Total, 1, null);
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
assertThat(response.getHits().getHits()[0].getId(), equalTo("4"));
@ -1510,7 +1510,7 @@ public class ChildQuerySearchIT extends ParentChildTestCase {
assertThat(response.getHits().getTotalHits().value, equalTo(0L));
response = minMaxQuery(ScoreMode.Total, 0, 4);
response = minMaxQuery(ScoreMode.Total, 1, 4);
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
assertThat(response.getHits().getHits()[0].getId(), equalTo("4"));
@ -1520,7 +1520,7 @@ public class ChildQuerySearchIT extends ParentChildTestCase {
assertThat(response.getHits().getHits()[2].getId(), equalTo("2"));
assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
response = minMaxQuery(ScoreMode.Total, 0, 3);
response = minMaxQuery(ScoreMode.Total, 1, 3);
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
assertThat(response.getHits().getHits()[0].getId(), equalTo("4"));
@ -1530,7 +1530,7 @@ public class ChildQuerySearchIT extends ParentChildTestCase {
assertThat(response.getHits().getHits()[2].getId(), equalTo("2"));
assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
response = minMaxQuery(ScoreMode.Total, 0, 2);
response = minMaxQuery(ScoreMode.Total, 1, 2);
assertThat(response.getHits().getTotalHits().value, equalTo(2L));
assertThat(response.getHits().getHits()[0].getId(), equalTo("3"));
@ -1548,7 +1548,7 @@ public class ChildQuerySearchIT extends ParentChildTestCase {
assertThat(e.getMessage(), equalTo("[has_child] 'max_children' is less than 'min_children'"));
// Score mode = MAX
response = minMaxQuery(ScoreMode.Max, 0, null);
response = minMaxQuery(ScoreMode.Max, 1, null);
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
assertThat(response.getHits().getHits()[0].getId(), equalTo("4"));
@ -1586,7 +1586,7 @@ public class ChildQuerySearchIT extends ParentChildTestCase {
assertThat(response.getHits().getTotalHits().value, equalTo(0L));
response = minMaxQuery(ScoreMode.Max, 0, 4);
response = minMaxQuery(ScoreMode.Max, 1, 4);
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
assertThat(response.getHits().getHits()[0].getId(), equalTo("4"));
@ -1596,7 +1596,7 @@ public class ChildQuerySearchIT extends ParentChildTestCase {
assertThat(response.getHits().getHits()[2].getId(), equalTo("2"));
assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
response = minMaxQuery(ScoreMode.Max, 0, 3);
response = minMaxQuery(ScoreMode.Max, 1, 3);
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
assertThat(response.getHits().getHits()[0].getId(), equalTo("4"));
@ -1606,7 +1606,7 @@ public class ChildQuerySearchIT extends ParentChildTestCase {
assertThat(response.getHits().getHits()[2].getId(), equalTo("2"));
assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
response = minMaxQuery(ScoreMode.Max, 0, 2);
response = minMaxQuery(ScoreMode.Max, 1, 2);
assertThat(response.getHits().getTotalHits().value, equalTo(2L));
assertThat(response.getHits().getHits()[0].getId(), equalTo("3"));
@ -1624,7 +1624,7 @@ public class ChildQuerySearchIT extends ParentChildTestCase {
assertThat(e.getMessage(), equalTo("[has_child] 'max_children' is less than 'min_children'"));
// Score mode = AVG
response = minMaxQuery(ScoreMode.Avg, 0, null);
response = minMaxQuery(ScoreMode.Avg, 1, null);
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
assertThat(response.getHits().getHits()[0].getId(), equalTo("4"));
@ -1662,7 +1662,7 @@ public class ChildQuerySearchIT extends ParentChildTestCase {
assertThat(response.getHits().getTotalHits().value, equalTo(0L));
response = minMaxQuery(ScoreMode.Avg, 0, 4);
response = minMaxQuery(ScoreMode.Avg, 1, 4);
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
assertThat(response.getHits().getHits()[0].getId(), equalTo("4"));
@ -1672,7 +1672,7 @@ public class ChildQuerySearchIT extends ParentChildTestCase {
assertThat(response.getHits().getHits()[2].getId(), equalTo("2"));
assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
response = minMaxQuery(ScoreMode.Avg, 0, 3);
response = minMaxQuery(ScoreMode.Avg, 1, 3);
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
assertThat(response.getHits().getHits()[0].getId(), equalTo("4"));
@ -1682,7 +1682,7 @@ public class ChildQuerySearchIT extends ParentChildTestCase {
assertThat(response.getHits().getHits()[2].getId(), equalTo("2"));
assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
response = minMaxQuery(ScoreMode.Avg, 0, 2);
response = minMaxQuery(ScoreMode.Avg, 1, 2);
assertThat(response.getHits().getTotalHits().value, equalTo(2L));
assertThat(response.getHits().getHits()[0].getId(), equalTo("3"));

View File

@ -141,7 +141,7 @@ public class HasChildQueryBuilderTests extends AbstractQueryTestCase<HasChildQue
*/
@Override
protected HasChildQueryBuilder doCreateTestQueryBuilder() {
int min = randomIntBetween(0, Integer.MAX_VALUE / 2);
int min = randomIntBetween(1, Integer.MAX_VALUE / 2);
int max = randomIntBetween(min, Integer.MAX_VALUE);
QueryBuilder innerQueryBuilder = new MatchAllQueryBuilder();
@ -163,6 +163,13 @@ public class HasChildQueryBuilderTests extends AbstractQueryTestCase<HasChildQue
return hqb;
}
public void testDeprecationOfZeroMinChildren() {
QueryBuilder query = new MatchAllQueryBuilder();
HasChildQueryBuilder foo = hasChildQuery("foo", query, ScoreMode.None);
foo.minMaxChildren(0, 1);
assertWarnings(HasChildQueryBuilder.MIN_CHILDREN_0_DEPRECATION_MESSAGE);
}
@Override
protected void doAssertLuceneQuery(HasChildQueryBuilder queryBuilder, Query query, SearchContext searchContext) throws IOException {
assertThat(query, instanceOf(HasChildQueryBuilder.LateParsingQuery.class));