Add tests for the supported query types. #35319
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jtibshirani
added
>test
|
Pinging @elastic/es-search-aggs |
11 tasks
jtibshirani
commented
Nov 6, 2018
| boolean includeLower, | ||
| boolean includeUpper, | ||
| QueryShardContext context) { | ||
| if (lowerTerm == null || upperTerm == null) { |
There was a problem hiding this comment.
We could fix this instead of disallowing unbounded range queries, but I wasn't sure the extra complexity was worth it. It's not clear how useful this range functionality will be right now, especially without support for numerics.
There was a problem hiding this comment.
I feel like this limitation is ok for now. We can revisit this later if necessary
jtibshirani
force-pushed
the
json-field-tests
branch
4 times, most recently
from
909c276
to
f693e46
Compare
colings86
reviewed
Nov 7, 2018
There was a problem hiding this comment.
I left a comment but otherwise I think this is good. I'd like @romseygeek to take a look too though
| boolean includeLower, | ||
| boolean includeUpper, | ||
| QueryShardContext context) { | ||
| if (lowerTerm == null || upperTerm == null) { |
There was a problem hiding this comment.
I feel like this limitation is ok for now. We can revisit this later if necessary
| TermRangeQuery expected = new TermRangeQuery("field", | ||
| new BytesRef("key\0lower"), | ||
| new BytesRef("key\0upper"), false, false); | ||
| assertEquals(expected, ft.rangeQuery("lower", "upper", false, false, null)); |
There was a problem hiding this comment.
should we also check that we do the right thing if the bounds are inclusive?
jtibshirani
force-pushed
the
object-fields
branch
from
e4ee31c
to
2ffb459
Compare
jtibshirani
force-pushed
the
json-field-tests
branch
from
f693e46
to
956a75d
Compare
romseygeek
reviewed
Nov 7, 2018
| .startObject() | ||
| .startObject("headers") | ||
| .field("content-type", "application/json") | ||
| .endObject() |
There was a problem hiding this comment.
Not especially related to the current PR, but how is case normalization being handled? The normal HTTP header here is Content-type with a capital C; is there a lowercase filter being applied anywhere, or is that being ignored for the moment?
There was a problem hiding this comment.
We don't apply any lowercasing to the object keys. This is consistent with the way normal field names are handled in the mapping, which are not lowercased (you could have two distinct fields for example, one named Content-Type and the other content-type). Do you think an option to automatically case-fold the keys would be useful?
Separately for field values (not keys), keyword fields provide a normalizer option. This is something we could consider, although it might need to be designed differently as json fields can contain non-string values.
romseygeek
approved these changes
Nov 7, 2018
|
Thank you both for reviewing, I plan to fix the failing test and merge. |
jtibshirani
force-pushed
the
json-field-tests
branch
from
cb19e81
to
7a3c18e
Compare
This affects some of the checks in tests, but doesn't have an actual effect on indexing (because there we rely on FieldType#docValuesType).
jtibshirani
added a commit
to jtibshirani/elasticsearch
that referenced
this pull request
Nov 8, 2018
* Add tests for the supported query types. * Disallow unbounded range queries on keyed JSON fields. * Make sure MappedFieldType#hasDocValues always returns false.
jtibshirani
added a commit
to jtibshirani/elasticsearch
that referenced
this pull request
Nov 9, 2018
* Add tests for the supported query types. * Disallow unbounded range queries on keyed JSON fields. * Make sure MappedFieldType#hasDocValues always returns false.
jtibshirani
added a commit
that referenced
this pull request
Nov 30, 2018
* Add tests for the supported query types. * Disallow unbounded range queries on keyed JSON fields. * Make sure MappedFieldType#hasDocValues always returns false.
jtibshirani
added a commit
that referenced
this pull request
Dec 5, 2018
* Add tests for the supported query types. * Disallow unbounded range queries on keyed JSON fields. * Make sure MappedFieldType#hasDocValues always returns false.
jtibshirani
added a commit
that referenced
this pull request
Dec 13, 2018
* Add tests for the supported query types. * Disallow unbounded range queries on keyed JSON fields. * Make sure MappedFieldType#hasDocValues always returns false.
jtibshirani
added a commit
that referenced
this pull request
Mar 6, 2019
* Add tests for the supported query types. * Disallow unbounded range queries on keyed JSON fields. * Make sure MappedFieldType#hasDocValues always returns false.
jtibshirani
added a commit
that referenced
this pull request
Mar 7, 2019
* Add tests for the supported query types. * Disallow unbounded range queries on keyed JSON fields. * Make sure MappedFieldType#hasDocValues always returns false.
jtibshirani
added a commit
that referenced
this pull request
Mar 9, 2019
* Add tests for the supported query types. * Disallow unbounded range queries on keyed JSON fields. * Make sure MappedFieldType#hasDocValues always returns false.
jtibshirani
added a commit
that referenced
this pull request
Mar 11, 2019
* Add tests for the supported query types. * Disallow unbounded range queries on keyed JSON fields. * Make sure MappedFieldType#hasDocValues always returns false.
jtibshirani
added a commit
that referenced
this pull request
Mar 14, 2019
* Add tests for the supported query types. * Disallow unbounded range queries on keyed JSON fields. * Make sure MappedFieldType#hasDocValues always returns false.
jtibshirani
added a commit
that referenced
this pull request
Mar 19, 2019
* Add tests for the supported query types. * Disallow unbounded range queries on keyed JSON fields. * Make sure MappedFieldType#hasDocValues always returns false.
jtibshirani
added a commit
to jtibshirani/elasticsearch
that referenced
this pull request
Mar 19, 2019
* Add tests for the supported query types. * Disallow unbounded range queries on keyed JSON fields. * Make sure MappedFieldType#hasDocValues always returns false.
jtibshirani
added a commit
that referenced
this pull request
Mar 28, 2019
* Add tests for the supported query types. * Disallow unbounded range queries on keyed JSON fields. * Make sure MappedFieldType#hasDocValues always returns false.
jtibshirani
added a commit
that referenced
this pull request
Apr 10, 2019
* Add tests for the supported query types. * Disallow unbounded range queries on keyed JSON fields. * Make sure MappedFieldType#hasDocValues always returns false.
jtibshirani
added a commit
that referenced
this pull request
Apr 17, 2019
* Add tests for the supported query types. * Disallow unbounded range queries on keyed JSON fields. * Make sure MappedFieldType#hasDocValues always returns false.
jtibshirani
added a commit
that referenced
this pull request
May 1, 2019
* Add tests for the supported query types. * Disallow unbounded range queries on keyed JSON fields. * Make sure MappedFieldType#hasDocValues always returns false.
jtibshirani
added a commit
that referenced
this pull request
May 24, 2019
* Add tests for the supported query types. * Disallow unbounded range queries on keyed JSON fields. * Make sure MappedFieldType#hasDocValues always returns false.
jtibshirani
added a commit
that referenced
this pull request
May 29, 2019
* Add tests for the supported query types. * Disallow unbounded range queries on keyed JSON fields. * Make sure MappedFieldType#hasDocValues always returns false.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The following query types should now be covered:
term,termsandterms_setprefixrangematchandmulti_matchquery_stringandsimple_query_stringexistsI preferred unit tests as much as possible, but certain query types (like
matchandquery_string) were more natural to check through integration tests.