-
Notifications
You must be signed in to change notification settings - Fork 25.6k
ESQL: Fix constant keyword optimization #129278
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
Fixes the ESQL's detection of `constant_keyword` fields. We unplugged it when we changed a function signature because we didn't have an `@Override` annotation. This plugs it back in and adds it to the integration tests we use for pushing queries to lucene. When you do `| WHERE constant_keyword_field == "itsvalue"` then the whole is removed from the query plan because *all* documents are equal.
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
|
Hi @nik9000, I've created a changelog YAML for you. |
|
|
||
| @Override | ||
| public String toString() { | ||
| return "LocalSourceOperator"; |
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.
Fix the signature rendering for the test.
| case "match_only_text", "semantic_text" -> true; | ||
| ComputeSignature dataNodeSignature = switch (type) { | ||
| case "auto", "constant_keyword", "text" -> ComputeSignature.FILTER_IN_QUERY; | ||
| case "match_only_text", "semantic_text" -> ComputeSignature.FILTER_IN_COMPUTE; |
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.
Because I was changing a bunch of stuff I put these in alphabetical order while I went. We're going to keep adding stuff to this in the next few months, I think. So sorted order helps.
| String val = null; | ||
| for (SearchExecutionContext ctx : contexts) { | ||
| MappedFieldType f = ctx.getFieldType(name); | ||
| MappedFieldType f = ctx.getFieldType(name.string()); |
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.
Real fix is all in this file.
dnhatn
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
|
I'm going to add |
luigidellaquila
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, thanks!
alex-spies
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.
Ooh, this is such a nice test improvement. Thanks a lot @nik9000 !
|
oooh conflicts! working |
|
@nik9000 , I think we forgot the |
|
Ah, yeah. I thought I had put backport-pending. It's running locally now |
Fixes the ESQL's detection of `constant_keyword` fields. We unplugged it when we changed a function signature because we didn't have an `@Override` annotation. This plugs it back in and adds it to the integration tests we use for pushing queries to lucene. When you do `| WHERE constant_keyword_field == "itsvalue"` then the whole is removed from the query plan because *all* documents are equal.
|
Backport: #129354 |
* ESQL: Fix constant keyword optimization (#129278) Fixes the ESQL's detection of `constant_keyword` fields. We unplugged it when we changed a function signature because we didn't have an `@Override` annotation. This plugs it back in and adds it to the integration tests we use for pushing queries to lucene. When you do `| WHERE constant_keyword_field == "itsvalue"` then the whole is removed from the query plan because *all* documents are equal. * Old lucene is different
Fixes the ESQL's detection of
constant_keywordfields. We unplugged it when we changed a function signature because we didn't have an@Overrideannotation. This plugs it back in and adds it to the integration tests we use for pushing queries to lucene. When you do| WHERE constant_keyword_field == "itsvalue"then the whole is removed from the query plan because all documents are equal.