-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Make sure that IdFieldType#isAggregatable is accurate. #62903
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
Conversation
f2d3083 to
4240bd2
Compare
Before, it always returned 'true' even when the setting "indices.id_field_data.enabled" was false.
4240bd2 to
bdb4f8b
Compare
|
@elasticmachine run elasticsearch-ci/2 |
| + "you can re-enable it by updating the dynamic cluster setting: " | ||
| + IndicesService.INDICES_ID_FIELD_DATA_ENABLED_SETTING.getKey()); | ||
| } | ||
| deprecationLogger.deprecate("id_field_data", ID_FIELD_DATA_DEPRECATION_MESSAGE); |
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 kept the deprecation message here so we don't issue a warning on every call to field caps.
jimczi
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
|
Pinging @elastic/es-search (:Search/Mapping) |
|
The refactor #63197 was merged concurrently, and happens to fix the bug. So after merging with master, this PR is now a test-only change that verifies the fix. |
|
@elasticmachine run elasticsearch-ci/1 |
Before, it always returned 'true' even when the setting "indices.id_field_data.enabled" was false. Fixes #62897.
Before, it always returned 'true' even when the setting
"indices.id_field_data.enabled" was false.
Fixes #62897.