Make 0 as invalid value for `min_children` in `has_child` query (#41347)

* squashing multiple commits

* fixing #32949 updated DEFAULT_MIN_CHILDREN to be 1, this way in case min_children is not provided it will be set to 1 by default.

* Fix ChildQuerySearchIT

modules/parent-join/src/main/java/org/elasticsearch/join/query/HasChildQueryBuilder.java

@@ -66,7 +66,7 @@ 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.
@@ -133,11 +133,11 @@ public class HasChildQueryBuilder extends AbstractQueryBuilder<HasChildQueryBuil
      * the maximum number of children that are required to match for the parent to be considered a match.
      */
     public HasChildQueryBuilder minMaxChildren(int minChildren, int maxChildren) {
-        if (minChildren < 0) {
-            throw new IllegalArgumentException("[" + NAME + "] requires non-negative 'min_children' field");
+        if (minChildren <= 0) {
+            throw new IllegalArgumentException("[" + NAME + "] requires positive 'min_children' field");
         }
-        if (maxChildren < 0) {
-            throw new IllegalArgumentException("[" + NAME + "] requires non-negative 'max_children' field");
+        if (maxChildren <= 0) {
+            throw new IllegalArgumentException("[" + NAME + "] requires positive 'max_children' field");
         }
         if (maxChildren < minChildren) {
             throw new IllegalArgumentException("[" + NAME + "] 'max_children' is less than 'min_children'");

modules/parent-join/src/test/java/org/elasticsearch/join/query/ChildQuerySearchIT.java

@@ -1396,16 +1396,6 @@ public class ChildQuerySearchIT extends ParentChildTestCase {
         SearchResponse response;
 
         // Score mode = NONE
-        response = minMaxQuery(ScoreMode.None, 0, null);
-
-        assertThat(response.getHits().getTotalHits().value, equalTo(3L));
-        assertThat(response.getHits().getHits()[0].getId(), equalTo("2"));
-        assertThat(response.getHits().getHits()[0].getScore(), equalTo(1f));
-        assertThat(response.getHits().getHits()[1].getId(), equalTo("3"));
-        assertThat(response.getHits().getHits()[1].getScore(), equalTo(1f));
-        assertThat(response.getHits().getHits()[2].getId(), equalTo("4"));
-        assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
-
         response = minMaxQuery(ScoreMode.None, 1, null);
 
         assertThat(response.getHits().getTotalHits().value, equalTo(3L));
@@ -1434,7 +1424,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 +1434,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 +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, 2);
+        response = minMaxQuery(ScoreMode.None, 1, 2);
 
         assertThat(response.getHits().getTotalHits().value, equalTo(2L));
         assertThat(response.getHits().getHits()[0].getId(), equalTo("2"));
@@ -1472,16 +1462,6 @@ 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);
-
-        assertThat(response.getHits().getTotalHits().value, equalTo(3L));
-        assertThat(response.getHits().getHits()[0].getId(), equalTo("4"));
-        assertThat(response.getHits().getHits()[0].getScore(), equalTo(6f));
-        assertThat(response.getHits().getHits()[1].getId(), equalTo("3"));
-        assertThat(response.getHits().getHits()[1].getScore(), equalTo(3f));
-        assertThat(response.getHits().getHits()[2].getId(), equalTo("2"));
-        assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
-
         response = minMaxQuery(ScoreMode.Total, 1, null);
 
         assertThat(response.getHits().getTotalHits().value, equalTo(3L));
@@ -1510,7 +1490,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 +1500,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 +1510,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,16 +1528,6 @@ 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);
-
-        assertThat(response.getHits().getTotalHits().value, equalTo(3L));
-        assertThat(response.getHits().getHits()[0].getId(), equalTo("4"));
-        assertThat(response.getHits().getHits()[0].getScore(), equalTo(3f));
-        assertThat(response.getHits().getHits()[1].getId(), equalTo("3"));
-        assertThat(response.getHits().getHits()[1].getScore(), equalTo(2f));
-        assertThat(response.getHits().getHits()[2].getId(), equalTo("2"));
-        assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
-
         response = minMaxQuery(ScoreMode.Max, 1, null);
 
         assertThat(response.getHits().getTotalHits().value, equalTo(3L));
@@ -1586,7 +1556,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 +1566,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 +1576,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,16 +1594,6 @@ 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);
-
-        assertThat(response.getHits().getTotalHits().value, equalTo(3L));
-        assertThat(response.getHits().getHits()[0].getId(), equalTo("4"));
-        assertThat(response.getHits().getHits()[0].getScore(), equalTo(2f));
-        assertThat(response.getHits().getHits()[1].getId(), equalTo("3"));
-        assertThat(response.getHits().getHits()[1].getScore(), equalTo(1.5f));
-        assertThat(response.getHits().getHits()[2].getId(), equalTo("2"));
-        assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
-
         response = minMaxQuery(ScoreMode.Avg, 1, null);
 
         assertThat(response.getHits().getTotalHits().value, equalTo(3L));
@@ -1662,7 +1622,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 +1632,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 +1642,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"));

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();
@@ -215,10 +215,10 @@ public class HasChildQueryBuilderTests extends AbstractQueryTestCase<HasChildQue
         int positiveValue = randomIntBetween(0, Integer.MAX_VALUE);
         HasChildQueryBuilder foo = hasChildQuery("foo", query, ScoreMode.None); // all good
         e = expectThrows(IllegalArgumentException.class, () -> foo.minMaxChildren(randomIntBetween(Integer.MIN_VALUE, -1), positiveValue));
-        assertEquals("[has_child] requires non-negative 'min_children' field", e.getMessage());
+        assertEquals("[has_child] requires positive 'min_children' field", e.getMessage());
 
         e = expectThrows(IllegalArgumentException.class, () -> foo.minMaxChildren(positiveValue, randomIntBetween(Integer.MIN_VALUE, -1)));
-        assertEquals("[has_child] requires non-negative 'max_children' field", e.getMessage());
+        assertEquals("[has_child] requires positive 'max_children' field", e.getMessage());
 
         e = expectThrows(IllegalArgumentException.class, () -> foo.minMaxChildren(positiveValue, positiveValue - 10));
         assertEquals("[has_child] 'max_children' is less than 'min_children'", e.getMessage());