-
Notifications
You must be signed in to change notification settings - Fork 25.4k
Adds transport-only flag to always include indices in the field caps transport response #133074
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
Adds transport-only flag to always include indices in the field caps transport response #133074
Conversation
Pinging @elastic/es-search-foundations (Team:Search Foundations) |
Hi @piergm, I've created a changelog YAML for you. |
…-not-present-everywhere
…-not-present-everywhere
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.
Cool change :)
server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java
Outdated
Show resolved
Hide resolved
server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java
Show resolved
Hide resolved
@@ -53,6 +53,7 @@ public final class FieldCapabilitiesRequest extends LegacyActionRequest implemen | |||
private String[] types = Strings.EMPTY_ARRAY; | |||
private boolean includeUnmapped = false; | |||
private boolean includeEmptyFields = true; | |||
private boolean includeIndices = false; |
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.
Doesn't this need to be added to the writeTo method and the StreamInput constructor? I'm a bit confused how the tests are passing.
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.
Generally I think one of the two things should happen:
- It is added to the serializations as you noted above, and we have TransportVersion bumped.
- It's not serialized and then marked
transient
and a comment is added as to why we don't need to serialize it.
In this particular case I think we do need to serialize it so (1) should happen.
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.
Also should it be added to equals/hashCode? And to corresponding createTestInstance/mutateInstance in FieldCapabilitiesRequestTests (I think it should).
If/when the new flag is added to equality check we might also have to serialize it in order to fix AbstractWireSerializingTestCase
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.
Hey!
Doesn't this need to be added to the writeTo method and the StreamInput constructor? I'm a bit confused how the tests are passing.
No need to serialize it since we are already sending the indices information that are currently used only for mapping conflicts.
It's not serialized and then marked transient and a comment is added as to why we don't need to serialize it.
++ on 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.
Thanks! This looks good from es|ql client view.
…-not-present-everywhere
…-not-present-everywhere
When calling field_caps with an expression that resolves to 2+ indices we currently return in the field caps response on a per-field basis the index where that particular field was found only if there is a mapping conflict (eg: index1 field
foo
is text and in index2 fieldfoo
is a long).With the following change we allow internal users of the FieldCaps action to set
includeIndices
a boolean flag that if set to true will always return the indices array.This allows FieldCaps internal consumers to understand on which indices a certain field was found.
Let's say we have two indices: "index1" with mapping foo1=keyword and foo2=keyword and "index2" with mapping foo2=keyword.
Then when requesting
GET index*/_field_caps?fields=*
withincludeIndices
set to true, the field caps response will be:Note: With
includeIndices==false
we keep the current behaviour to return "indices" only if there is a mapping conflict.