ESQL: Make field fusion generic #137382
ESQL: Make field fusion generic #137382
Conversation
Speeds up queries like ``` FROM foo | STATS SUM(LENGTH(field)) ``` by fusing the `LENGTH` into the loading of the `field` if it has doc values. Running a fairly simple test: https://gist.github.com/nik9000/9dac067f8ce29875a4fb0f0359a75091 I'm seeing that query drop from 48ms to 28ms. So, like, 40% faster. More importantly, this makes the mechanism for fusing functions into field loading generic. All you have to do is implement `BlockLoaderExpression` on your expression and return non-null from `tryFuse`.
|
Hi @nik9000, I've created a changelog YAML for you. |
| * "fusing" the expression into the load. Or null if the fusion isn't possible. | ||
| */ | ||
| @Nullable | ||
| Fuse tryFuse(SearchStats stats); |
There was a problem hiding this comment.
Let's try to find another name - we already have Fuse as a command. ExpressionFieldLoader?
There was a problem hiding this comment.
Is FusedExpression ok? Or still too indicative?
There was a problem hiding this comment.
Naming... 😅
I come from staring at FUSE enough that it carries a lot of weight.
For me, this feature involves BlockLoaders. And Expressions that are applied to them. I understand that fuse means getting together those two, but it's not something I would think of immediately without more context.
I'd prefer to be overly explicit here, and call this BlockLoaderExpression or something similar that helps me bridge those two concepts together. But, naming...
| BlockLoaderExpression.Fuse fuse | ||
| ) { | ||
| // Only replace if exactly one side is a literal and the other a field attribute | ||
| if ((similarityFunction.left() instanceof Literal ^ similarityFunction.right() instanceof Literal) == false) { |
There was a problem hiding this comment.
Nice! It's much better to let the Expression deal with the details and make this generic 👍
| */ | ||
| public boolean pushable() { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
This bothers me. I needed this because without it we'd try to push this:
FROM foo
| WHERE LENGTH(kwd) < 10
to the index. Now, we might be able to do that with a specialized lucene query. But we don't have one of those. Without those change instead what happens is:
LENGTH(kwd)becomes$$kwd$length$hash$.- We identify
$$kwd$length$hash$ < 10as pushable.
This tells us we can't push it. But it's kind of picky. If SearchStats took EsField it could check this easy enough. That might be a good solution to this.
...sql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalLogicalPlanOptimizerTests.java
Show resolved
Hide resolved
Adds special purpose `BlockLoader` implementations for the `MV_MIN` and `MV_MAX` functions for `keyword` fields with doc values. These are a noop for single valued keywords but should be *much* faster for multivalued keywords. These aren't plugged in yet. We can plug them in and performance test them in elastic#137382. And they give us two more functions we can use to demonstrate elastic#137382.
| } | ||
|
|
||
| public void testLengthInWhereAndEval() { | ||
| assumeFalse("fix me", true); |
There was a problem hiding this comment.
QL friends: This one looks fun!
elasticsearchmachine
added
the
Team:Analytics
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
|
This is ready for folks to look at! I left two open questions around planning as self-comments. |
|
I think I'd also like to write a really basic integration test that uses proves the fusion is happening. |
|
A little test: https://gist.github.com/nik9000/3a8eb7065d20c6f36d0c71fad80d1d2c With few unique values the perf goes from ~85ms to ~13ms. If there's enough values to trigger #137217 (comment) then it's slower. So, as always, @dnhatn was right. I'll make the sort, iter, uniq bit he suggested. |
3 participants
Speeds up queries like
by fusing the
LENGTHinto the loading of thefieldif it has doc values. Running a fairly simple test:https://gist.github.com/nik9000/9dac067f8ce29875a4fb0f0359a75091 I'm seeing that query drop from 48ms to 28ms. So, like, 40% faster.
More importantly, this makes the mechanism for fusing functions into field loading generic. All you have to do is implement
BlockLoaderExpressionon your expression and return non-null fromtryFuse.