ESQL: Push down filter passed lookup join#118410
Conversation
|
Hi @costin, I've created a changelog YAML for you. |
Improve the planner to detect filters that can be pushed down 'through' a LOOKUP JOIN by determining the conditions scoped to the left/main side and moving them closer to the source. Relates elastic#118305
costin
force-pushed
the
esql/push-filter-through-join
branch
from
9fc0020 to
5010855
Compare
alex-spies
left a comment
alex-spies
left a comment
There was a problem hiding this comment.
Yep, I believe this approach is simple, succinct and correct. Needs a couple tests and it's good to go IMO.
| AttributeSet rightOutput = right.outputSet(); | ||
|
|
||
| // first remove things that left scoped only | ||
| rest.removeIf(f -> f.references().subsetOf(leftOutput) && leftFilters.add(f)); |
...ain/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineFilters.java
Outdated
Show resolved
Hide resolved
...ain/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineFilters.java
Show resolved
Hide resolved
elasticsearchmachine
added
the
Team:Analytics
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
| 3 | Error | ||
| ; | ||
|
|
||
| lookupWithFieldOnJoinKey-Ignored |
There was a problem hiding this comment.
The filter on right side removes all data, likely due to considering the field as missing.
We have a bug for that so I just disabled the test for now.
| 10093 | 3 | Spanish | ||
| ; | ||
|
|
||
| lookupMessageWithFilterOnRightSideField-Ignored |
There was a problem hiding this comment.
The filter on right side removes all data, likely due to considering the field as missing.
We have a bug for that so I just disabled the test for now.
| // | ||
| // Lookup JOIN | ||
| // |
There was a problem hiding this comment.
I wanted to double check the pushdown happens inside the plan and since our CSV tests don't support (yet) lookup queries in non-Lucene mode, I've added added a bunch of testing her (such as the case with rename or breaking the filter up).
| assertThat(join.config().type(), equalTo(JoinTypes.LEFT)); | ||
| project = as(join.left(), Project.class); | ||
| var filter = as(project.child(), Filter.class); | ||
| // assert that the rename has been undone and |
| // | ||
|
|
||
| /** | ||
| * Filter on join keys should be pushed donw |
There was a problem hiding this comment.
| * Filter on join keys should be pushed donw | |
| * Filter on join keys should be pushed down |
astefan
left a comment
astefan
left a comment
There was a problem hiding this comment.
I have also tried another, more interesting, query which passed, if you'd like to add it to the csv-spec file. Either way LGTM.
The query is
FROM employees
| WHERE CASE(languages <= 1, 1, languages <= 2, 2, 3) >= 3
| RENAME languages AS language_code
| LOOKUP JOIN languages_lookup ON language_code
| SORT emp_no
| KEEP emp_no, language_code, language_name
| WHERE emp_no >= 10091 AND emp_no < 10094 OR language_code > 3
| // split the filter condition in 3 parts: | ||
| // 1. filter scoped to the left | ||
| // 2. filter scoped to the right | ||
| // 3. filter that requires both sides to be evaluated |
There was a problem hiding this comment.
Nit: I don't think this duplicated comment here is necessary, since the method itself has the same javadoc.
costin
added
the
auto-backport
💚 Backport successful
|
…18702) Improve the planner to detect filters that can be pushed down 'through' a LOOKUP JOIN by determining the conditions scoped to the left/main side and moving them closer to the source. Relates elastic#118305
5 participants
Improve the planner to detect filters that can be pushed down 'through'
a LOOKUP JOIN by determining the conditions scoped to the left/main
side and moving them closer to the source.
Relates #118305