-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Add more comprehensive tests for field aliases in queries + aggregations. #31565
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add more comprehensive tests for field aliases in queries + aggregations. #31565
Conversation
|
Pinging @elastic/es-search-aggs |
4890c86 to
4271c55
Compare
f03c51c to
0cf17d4
Compare
0cf17d4 to
1dff8c4
Compare
25c1fbe to
f11b855
Compare
2d95123 to
40b5790
Compare
0e1b3a2 to
fc38631
Compare
|
@jpountz this is ready for review (but is not time-sensitive, since I'll be away for a week). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been a bit concerned that this requirement (to fetch the name of the concrete field type) can be 'trappy' for other developers. I haven't been able to see a way to avoid this, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks ok to me
fc38631 to
ef2d6fc
Compare
jpountz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks ok to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe call it newLegacyExistsQuery instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
ef2d6fc to
384a33a
Compare
…ons. (#31565) * Make sure that significant terms aggregations work with field aliases. * Add a test for ValuesSourceConfig. * Allow for subclasses of AggregatorTestCase to provide field aliases. * Add tests for nested and reverse_nested aggregations. * Add an integration test for nested queries. * Add an integration test for 'more like this' queries. * Add tests for querying and loading meta-fields. * Add unit tests for the relevant query builders. * Add integration tests for geo polygon and shape queries. * Fix static analysis violations. * Document that aliases cannot be used in a query lookup path.
…ons. (#31565) * Make sure that significant terms aggregations work with field aliases. * Add a test for ValuesSourceConfig. * Allow for subclasses of AggregatorTestCase to provide field aliases. * Add tests for nested and reverse_nested aggregations. * Add an integration test for nested queries. * Add an integration test for 'more like this' queries. * Add tests for querying and loading meta-fields. * Add unit tests for the relevant query builders. * Add integration tests for geo polygon and shape queries. * Fix static analysis violations. * Document that aliases cannot be used in a query lookup path.
* Add basic support for field aliases in index mappings. (#31287) * Allow for aliases when fetching stored fields. (#31411) * Add tests around accessing field aliases in scripts. (#31417) * Add documentation around field aliases. (#31538) * Add validation for field alias mappings. (#31518) * Return both concrete fields and aliases in DocumentFieldMappers#getMapper. (#31671) * Make sure that field-level security is enforced when using field aliases. (#31807) * Add more comprehensive tests for field aliases in queries + aggregations. (#31565) * Remove the deprecated method DocumentFieldMappers#getFieldMapper. (#32148)
* Add basic support for field aliases in index mappings. (#31287) * Allow for aliases when fetching stored fields. (#31411) * Add tests around accessing field aliases in scripts. (#31417) * Return both concrete fields and aliases in DocumentFieldMappers#getMapper. (#31671) * Add documentation around field aliases. (#31538) * Add validation for field alias mappings. (#31518) * Make sure that field-level security is enforced when using field aliases. (#31807) * Add more comprehensive tests for field aliases in queries + aggregations. (#31565) * Remove the deprecated method DocumentFieldMappers#getFieldMapper. (#32148) * Ensure that field aliases cannot be used in multi-fields. (#32219) * Make sure that field aliases count towards the total fields limit. (#32222) * Fix a test bug around nested aggregations and field aliases. (#32287) * Make sure the _uid field is correctly loaded in scripts. * Fix the failing test case FieldLevelSecurityTests#testParentChild_parentField. * Enforce that field aliases can only be specified on indexes with a single type.
This PR also fixes some issues uncovered by the tests, largely times when the wrong field name was being used to construct a
Query. I went with unit tests, except for certain query types where integration tests were needed to verify the right behavior.