Match can't be used after STATS .. BY until filters can be pushed down #116642
Conversation
carlosdelest
added
Team:Analytics
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
elasticsearchmachine
removed
the
Team:Search Relevance
ioanatia
left a comment
ioanatia
left a comment
There was a problem hiding this comment.
makes sense to me - I left a suggestion
| condition, | ||
| Match.class, | ||
| lp -> (lp instanceof Limit == false), | ||
| lp -> (lp instanceof Limit == false) && (lp instanceof Aggregate == false), |
There was a problem hiding this comment.
This makes sense to me - but in these types of situations I prefer to have an "allow list" of commands that can be used before match, rather than check for the ones that cannot be used.
This is what we do for qstr and IMO it's a bit safer, because we do not have to worry when we introduce a new command that we need to immediately update this check here. Worst case when we add a new command we have a limitation in place that we can lift any time.
The other way around, where we have a "deny" list like we do now, when we add a new command, the worst case is that the ES|QL query fails in a way we did not predict.
There was a problem hiding this comment.
That makes sense. As this is temporary until #115311 is fixed, I can revisit the implementation when we change this again.
carlosdelest
added
the
auto-backport
💔 Backport failedThe backport operation could not be completed due to the following error: You can use sqren/backport to manually backport by running |
…n below STATS (elastic#116642) (cherry picked from commit e2e8a46)
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
|
I assume that this will resolve #115353, right? |
@ChrisHegarty correct, thanks for the catch! Closed #115353 |
4 participants
Until #115311 allows filters to be pushed down past STATS .. BY, MATCH can't work after STATS.
STATSproduces either a single column that is the result of a computation done in the compute engine, or the additional grouping column specified viaBY.This PR provides a better error message in the meanwhile. We can revert this once #115311 is fixed.