-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Expose if a field is a metadata field in the field capabilities response #69977
Conversation
This change exposes for each field in the _field_caps response if the field is a metadata field. This is needed for consumers of this API that want to filter these fields. Currently ML keeps a static list and QL checks that the family type starts with `_`. In order to ease the addition of new metadata fields, this change reworks the strategy in this solution and now only checks for the new flag. Note that the new flag is also applied at the coordinator level in a best-effort to apply the logic on older nodes in a mixed-version cluster.
Pinging @elastic/es-search (Team:Search) |
\cc @elastic/ml-core and @elastic/es-ql |
Awesome! Looks good from ML standpoint. |
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.
Not had a chance to run it yet but added a couple of comments
server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java
Show resolved
Hide resolved
...ml/src/main/java/org/elasticsearch/xpack/ml/dataframe/extractor/ExtractedFieldsDetector.java
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java
Show resolved
Hide resolved
.../src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesIndexAction.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/indices/IndicesService.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesTests.java
Outdated
Show resolved
Hide resolved
Nice improvement @jimczi . Indeed, QL filters out any fields starting with "_" and those potentially being added by mapper_size plugin. Thank you for simplifying this! |
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 just realized that I had an in-flight review from a few days ago, that I forgot to submit. super minor things. I am happy with the updates, thanks!
server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java
Outdated
Show resolved
Hide resolved
ci failed on a known issue |
@elasticmachine run elasticsearch-ci/1 |
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.
This is mostly ready, I left a couple of questions on naming and the versioning logic, LGTM otherwise.
server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java
Show resolved
Hide resolved
…nse (elastic#69977) This change exposes for each field in the _field_caps response if the field is a metadata field. This is needed for consumers of this API that want to filter these fields. Currently ML keeps a static list and QL checks that the family type starts with `_`. In order to ease the addition of new metadata fields, this change reworks the strategy in this solution and now only checks for the new flag. Note that the new flag is also applied at the coordinator level in a best-effort to apply the logic on older nodes in a mixed-version cluster.
This change exposes for each field in the _field_caps response if the field is a metadata field.
This is needed for consumers of this API that want to filter these fields. Currently ML keeps a static list
and QL checks that the family type starts with
_
. In order to ease the addition of new metadata fields, thischange reworks the strategy in this solution and now only checks for the new flag.
Note that the new flag is also applied at the coordinator level in a best-effort to apply the logic on older nodes
in a mixed-version cluster.