From 22f3b53ed706476695cc990d2e5638e153a4ce0b Mon Sep 17 00:00:00 2001
From: Hicham Mallah <mallah.hicham@gmail.com>
Date: Fri, 26 Apr 2019 22:00:11 +0300
Subject: [PATCH] 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
---
.../join/query/HasChildQueryBuilder.java | 13 ++++++--
.../join/query/ChildQuerySearchIT.java | 32 +++++++++----------
.../join/query/HasChildQueryBuilderTests.java | 9 +++++-
3 files changed, 34 insertions(+), 20 deletions(-)
diff --git a/modules/parent-join/src/main/java/org/elasticsearch/join/query/HasChildQueryBuilder.java b/modules/parent-join/src/main/java/org/elasticsearch/join/query/HasChildQueryBuilder.java
index 1c44daea4e9..56bf0112044 100644
--- a/modules/parent-join/src/main/java/org/elasticsearch/join/query/HasChildQueryBuilder.java
+++ b/modules/parent-join/src/main/java/org/elasticsearch/join/query/HasChildQueryBuilder.java
@@ -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");
}
diff --git a/modules/parent-join/src/test/java/org/elasticsearch/join/query/ChildQuerySearchIT.java b/modules/parent-join/src/test/java/org/elasticsearch/join/query/ChildQuerySearchIT.java
index 8520b632056..26eece8fbd9 100644
--- a/modules/parent-join/src/test/java/org/elasticsearch/join/query/ChildQuerySearchIT.java
+++ b/modules/parent-join/src/test/java/org/elasticsearch/join/query/ChildQuerySearchIT.java
@@ -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"));
diff --git a/modules/parent-join/src/test/java/org/elasticsearch/join/query/HasChildQueryBuilderTests.java b/modules/parent-join/src/test/java/org/elasticsearch/join/query/HasChildQueryBuilderTests.java
index 2a28e232b5e..8f38b8b9ae5 100644
--- a/modules/parent-join/src/test/java/org/elasticsearch/join/query/HasChildQueryBuilderTests.java
+++ b/modules/parent-join/src/test/java/org/elasticsearch/join/query/HasChildQueryBuilderTests.java
@@ -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));