Make 0 as invalid value for min_children in has_child query
#33073
Conversation
|
Hi @joshfullmer, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in yout Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile? |
|
Done. I had recently changed my GitHub primary email, but neglected to change my commit email address. |
colings86
added
the
:Search/Search
|
Pinging @elastic/es-search-aggs |
|
@elasticmachine test this please |
1 similar comment
|
@elasticmachine test this please |
There was a problem hiding this comment.
@joshfullmer Thank you for your PR. Sorry that it got so late to review it.
Overall well, written, just a small comment about the error message.
Would you please be able to rebase your PR against upstream master so that we can make testing pass (just merge upstream master to your branch).
| if (minChildren < 0) { | ||
| throw new IllegalArgumentException("[" + NAME + "] requires non-negative 'min_children' field"); | ||
| if (minChildren < 1) { | ||
| throw new IllegalArgumentException("[" + NAME + "] requires non-negative, non-zero 'min_children' field"); |
There was a problem hiding this comment.
Instead of "non-negative, non-zero", we can just use "positive"
|
@joshfullmer Would you like to continue working on this? I think just a small rephrasing is left. If not, I can finish it myself, and merge your commit. |
|
@mayya-sharipova I just merged in master in case you want to kick of a new CI run. Should we get this in and do the rephrasing afterwards? |
|
@elasticmachine test this please |
1 similar comment
|
@elasticmachine test this please |
|
I am closing this PR because on inactivity, and there is still more work that needs to be done addressing failures in |
Related to issue #32949