Skip to content

Commit bc95794

Browse files
codebirdChristoph Büscher
codebird
authored and
Christoph Büscher
committed
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
1 parent f89420e commit bc95794

File tree

3 files changed

+20
-60
lines changed

3 files changed

+20
-60
lines changed

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public class HasChildQueryBuilder extends AbstractQueryBuilder<HasChildQueryBuil
6666
/**
6767
* The default minimum number of children that are required to match for the parent to be considered a match.
6868
*/
69-
public static final int DEFAULT_MIN_CHILDREN = 0;
69+
public static final int DEFAULT_MIN_CHILDREN = 1;
7070

7171
/**
7272
* The default value for ignore_unmapped.
@@ -133,11 +133,11 @@ protected void doWriteTo(StreamOutput out) throws IOException {
133133
* the maximum number of children that are required to match for the parent to be considered a match.
134134
*/
135135
public HasChildQueryBuilder minMaxChildren(int minChildren, int maxChildren) {
136-
if (minChildren < 0) {
137-
throw new IllegalArgumentException("[" + NAME + "] requires non-negative 'min_children' field");
136+
if (minChildren <= 0) {
137+
throw new IllegalArgumentException("[" + NAME + "] requires positive 'min_children' field");
138138
}
139-
if (maxChildren < 0) {
140-
throw new IllegalArgumentException("[" + NAME + "] requires non-negative 'max_children' field");
139+
if (maxChildren <= 0) {
140+
throw new IllegalArgumentException("[" + NAME + "] requires positive 'max_children' field");
141141
}
142142
if (maxChildren < minChildren) {
143143
throw new IllegalArgumentException("[" + NAME + "] 'max_children' is less than 'min_children'");

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

Lines changed: 12 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1396,16 +1396,6 @@ public void testMinMaxChildren() throws Exception {
13961396
SearchResponse response;
13971397

13981398
// Score mode = NONE
1399-
response = minMaxQuery(ScoreMode.None, 0, null);
1400-
1401-
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
1402-
assertThat(response.getHits().getHits()[0].getId(), equalTo("2"));
1403-
assertThat(response.getHits().getHits()[0].getScore(), equalTo(1f));
1404-
assertThat(response.getHits().getHits()[1].getId(), equalTo("3"));
1405-
assertThat(response.getHits().getHits()[1].getScore(), equalTo(1f));
1406-
assertThat(response.getHits().getHits()[2].getId(), equalTo("4"));
1407-
assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
1408-
14091399
response = minMaxQuery(ScoreMode.None, 1, null);
14101400

14111401
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
@@ -1434,7 +1424,7 @@ public void testMinMaxChildren() throws Exception {
14341424

14351425
assertThat(response.getHits().getTotalHits().value, equalTo(0L));
14361426

1437-
response = minMaxQuery(ScoreMode.None, 0, 4);
1427+
response = minMaxQuery(ScoreMode.None, 1, 4);
14381428

14391429
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
14401430
assertThat(response.getHits().getHits()[0].getId(), equalTo("2"));
@@ -1444,7 +1434,7 @@ public void testMinMaxChildren() throws Exception {
14441434
assertThat(response.getHits().getHits()[2].getId(), equalTo("4"));
14451435
assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
14461436

1447-
response = minMaxQuery(ScoreMode.None, 0, 3);
1437+
response = minMaxQuery(ScoreMode.None, 1, 3);
14481438

14491439
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
14501440
assertThat(response.getHits().getHits()[0].getId(), equalTo("2"));
@@ -1454,7 +1444,7 @@ public void testMinMaxChildren() throws Exception {
14541444
assertThat(response.getHits().getHits()[2].getId(), equalTo("4"));
14551445
assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
14561446

1457-
response = minMaxQuery(ScoreMode.None, 0, 2);
1447+
response = minMaxQuery(ScoreMode.None, 1, 2);
14581448

14591449
assertThat(response.getHits().getTotalHits().value, equalTo(2L));
14601450
assertThat(response.getHits().getHits()[0].getId(), equalTo("2"));
@@ -1472,16 +1462,6 @@ public void testMinMaxChildren() throws Exception {
14721462
assertThat(e.getMessage(), equalTo("[has_child] 'max_children' is less than 'min_children'"));
14731463

14741464
// Score mode = SUM
1475-
response = minMaxQuery(ScoreMode.Total, 0, null);
1476-
1477-
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
1478-
assertThat(response.getHits().getHits()[0].getId(), equalTo("4"));
1479-
assertThat(response.getHits().getHits()[0].getScore(), equalTo(6f));
1480-
assertThat(response.getHits().getHits()[1].getId(), equalTo("3"));
1481-
assertThat(response.getHits().getHits()[1].getScore(), equalTo(3f));
1482-
assertThat(response.getHits().getHits()[2].getId(), equalTo("2"));
1483-
assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
1484-
14851465
response = minMaxQuery(ScoreMode.Total, 1, null);
14861466

14871467
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
@@ -1510,7 +1490,7 @@ public void testMinMaxChildren() throws Exception {
15101490

15111491
assertThat(response.getHits().getTotalHits().value, equalTo(0L));
15121492

1513-
response = minMaxQuery(ScoreMode.Total, 0, 4);
1493+
response = minMaxQuery(ScoreMode.Total, 1, 4);
15141494

15151495
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
15161496
assertThat(response.getHits().getHits()[0].getId(), equalTo("4"));
@@ -1520,7 +1500,7 @@ public void testMinMaxChildren() throws Exception {
15201500
assertThat(response.getHits().getHits()[2].getId(), equalTo("2"));
15211501
assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
15221502

1523-
response = minMaxQuery(ScoreMode.Total, 0, 3);
1503+
response = minMaxQuery(ScoreMode.Total, 1, 3);
15241504

15251505
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
15261506
assertThat(response.getHits().getHits()[0].getId(), equalTo("4"));
@@ -1530,7 +1510,7 @@ public void testMinMaxChildren() throws Exception {
15301510
assertThat(response.getHits().getHits()[2].getId(), equalTo("2"));
15311511
assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
15321512

1533-
response = minMaxQuery(ScoreMode.Total, 0, 2);
1513+
response = minMaxQuery(ScoreMode.Total, 1, 2);
15341514

15351515
assertThat(response.getHits().getTotalHits().value, equalTo(2L));
15361516
assertThat(response.getHits().getHits()[0].getId(), equalTo("3"));
@@ -1548,16 +1528,6 @@ public void testMinMaxChildren() throws Exception {
15481528
assertThat(e.getMessage(), equalTo("[has_child] 'max_children' is less than 'min_children'"));
15491529

15501530
// Score mode = MAX
1551-
response = minMaxQuery(ScoreMode.Max, 0, null);
1552-
1553-
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
1554-
assertThat(response.getHits().getHits()[0].getId(), equalTo("4"));
1555-
assertThat(response.getHits().getHits()[0].getScore(), equalTo(3f));
1556-
assertThat(response.getHits().getHits()[1].getId(), equalTo("3"));
1557-
assertThat(response.getHits().getHits()[1].getScore(), equalTo(2f));
1558-
assertThat(response.getHits().getHits()[2].getId(), equalTo("2"));
1559-
assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
1560-
15611531
response = minMaxQuery(ScoreMode.Max, 1, null);
15621532

15631533
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
@@ -1586,7 +1556,7 @@ public void testMinMaxChildren() throws Exception {
15861556

15871557
assertThat(response.getHits().getTotalHits().value, equalTo(0L));
15881558

1589-
response = minMaxQuery(ScoreMode.Max, 0, 4);
1559+
response = minMaxQuery(ScoreMode.Max, 1, 4);
15901560

15911561
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
15921562
assertThat(response.getHits().getHits()[0].getId(), equalTo("4"));
@@ -1596,7 +1566,7 @@ public void testMinMaxChildren() throws Exception {
15961566
assertThat(response.getHits().getHits()[2].getId(), equalTo("2"));
15971567
assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
15981568

1599-
response = minMaxQuery(ScoreMode.Max, 0, 3);
1569+
response = minMaxQuery(ScoreMode.Max, 1, 3);
16001570

16011571
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
16021572
assertThat(response.getHits().getHits()[0].getId(), equalTo("4"));
@@ -1606,7 +1576,7 @@ public void testMinMaxChildren() throws Exception {
16061576
assertThat(response.getHits().getHits()[2].getId(), equalTo("2"));
16071577
assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
16081578

1609-
response = minMaxQuery(ScoreMode.Max, 0, 2);
1579+
response = minMaxQuery(ScoreMode.Max, 1, 2);
16101580

16111581
assertThat(response.getHits().getTotalHits().value, equalTo(2L));
16121582
assertThat(response.getHits().getHits()[0].getId(), equalTo("3"));
@@ -1624,16 +1594,6 @@ public void testMinMaxChildren() throws Exception {
16241594
assertThat(e.getMessage(), equalTo("[has_child] 'max_children' is less than 'min_children'"));
16251595

16261596
// Score mode = AVG
1627-
response = minMaxQuery(ScoreMode.Avg, 0, null);
1628-
1629-
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
1630-
assertThat(response.getHits().getHits()[0].getId(), equalTo("4"));
1631-
assertThat(response.getHits().getHits()[0].getScore(), equalTo(2f));
1632-
assertThat(response.getHits().getHits()[1].getId(), equalTo("3"));
1633-
assertThat(response.getHits().getHits()[1].getScore(), equalTo(1.5f));
1634-
assertThat(response.getHits().getHits()[2].getId(), equalTo("2"));
1635-
assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
1636-
16371597
response = minMaxQuery(ScoreMode.Avg, 1, null);
16381598

16391599
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
@@ -1662,7 +1622,7 @@ public void testMinMaxChildren() throws Exception {
16621622

16631623
assertThat(response.getHits().getTotalHits().value, equalTo(0L));
16641624

1665-
response = minMaxQuery(ScoreMode.Avg, 0, 4);
1625+
response = minMaxQuery(ScoreMode.Avg, 1, 4);
16661626

16671627
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
16681628
assertThat(response.getHits().getHits()[0].getId(), equalTo("4"));
@@ -1672,7 +1632,7 @@ public void testMinMaxChildren() throws Exception {
16721632
assertThat(response.getHits().getHits()[2].getId(), equalTo("2"));
16731633
assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
16741634

1675-
response = minMaxQuery(ScoreMode.Avg, 0, 3);
1635+
response = minMaxQuery(ScoreMode.Avg, 1, 3);
16761636

16771637
assertThat(response.getHits().getTotalHits().value, equalTo(3L));
16781638
assertThat(response.getHits().getHits()[0].getId(), equalTo("4"));
@@ -1682,7 +1642,7 @@ public void testMinMaxChildren() throws Exception {
16821642
assertThat(response.getHits().getHits()[2].getId(), equalTo("2"));
16831643
assertThat(response.getHits().getHits()[2].getScore(), equalTo(1f));
16841644

1685-
response = minMaxQuery(ScoreMode.Avg, 0, 2);
1645+
response = minMaxQuery(ScoreMode.Avg, 1, 2);
16861646

16871647
assertThat(response.getHits().getTotalHits().value, equalTo(2L));
16881648
assertThat(response.getHits().getHits()[0].getId(), equalTo("3"));

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ protected void initializeAdditionalMappings(MapperService mapperService) throws
141141
*/
142142
@Override
143143
protected HasChildQueryBuilder doCreateTestQueryBuilder() {
144-
int min = randomIntBetween(0, Integer.MAX_VALUE / 2);
144+
int min = randomIntBetween(1, Integer.MAX_VALUE / 2);
145145
int max = randomIntBetween(min, Integer.MAX_VALUE);
146146

147147
QueryBuilder innerQueryBuilder = new MatchAllQueryBuilder();
@@ -215,10 +215,10 @@ public void testIllegalValues() {
215215
int positiveValue = randomIntBetween(0, Integer.MAX_VALUE);
216216
HasChildQueryBuilder foo = hasChildQuery("foo", query, ScoreMode.None); // all good
217217
e = expectThrows(IllegalArgumentException.class, () -> foo.minMaxChildren(randomIntBetween(Integer.MIN_VALUE, -1), positiveValue));
218-
assertEquals("[has_child] requires non-negative 'min_children' field", e.getMessage());
218+
assertEquals("[has_child] requires positive 'min_children' field", e.getMessage());
219219

220220
e = expectThrows(IllegalArgumentException.class, () -> foo.minMaxChildren(positiveValue, randomIntBetween(Integer.MIN_VALUE, -1)));
221-
assertEquals("[has_child] requires non-negative 'max_children' field", e.getMessage());
221+
assertEquals("[has_child] requires positive 'max_children' field", e.getMessage());
222222

223223
e = expectThrows(IllegalArgumentException.class, () -> foo.minMaxChildren(positiveValue, positiveValue - 10));
224224
assertEquals("[has_child] 'max_children' is less than 'min_children'", e.getMessage());

0 commit comments

Comments
 (0)