ESQL: Fix columns ordering when pruning an INLINE STATS #136827
Conversation
This fixes the incorrect ordering of the columns that can occur when INLINE STATS is pruned entirely, due to its calculated value being discarded (dropped or overshadowned)
bpintea added auto-backport 
elasticsearchmachine added the Team:Analytics 
| Pinging @elastic/es-analytical-engine (Team:Analytics) |
| Hi @bpintea, I've created a changelog YAML for you. |
astefan left a comment
astefan left a comment There was a problem hiding this comment.
LGTM. Left some minor comments, except the one with projection.
Also, I would add some variations to stats computations keepers and droppers:
ROW a = null, b = 0, c = "x", d = 1 | INLINE STATS a = MAX(c), d = MIN(c), e = AVG(b) by b | EVAL a = c, d = 2 ROW a = null, b = 0, c = "x", d = 1 | INLINE STATS a = MAX(c), d = MIN(c), e = AVG(b), a = MIN(b) by b | EVAL a = c, d = 2 ROW a = null, b = 0, c = "x", d = 1 | INLINE STATS a = MAX(c), d = MIN(c), e = AVG(b), a = MIN(b), f = MAX(b) by b | EVAL a = c, d = 2 | /** | ||
| * Fix pruning of columns when shadowed in INLINE STATS | ||
| */ | ||
| INLINE_STATS_PRUNE_COLUMN_FIX(INLINESTATS_V11.enabled), |
There was a problem hiding this comment.
| INLINE_STATS_PRUNE_COLUMN_FIX(INLINESTATS_V11.enabled), | |
| INLINE_STATS_PRUNE_COLUMN_FIX(INLINE_STATS.enabled), |
| List<Attribute> newOutput = new ArrayList<>(ij.output()); | ||
| AttributeSet leftOutputSet = ij.left().outputSet(); | ||
| newOutput.removeIf(attr -> leftOutputSet.contains(attr) == false); | ||
| p = new EsqlProject(ij.source(), ij.left(), newOutput); |
There was a problem hiding this comment.
| p = new EsqlProject(ij.source(), ij.left(), newOutput); | |
| p = new Project(ij.source(), ij.left(), newOutput); |
| var right = pruneColumns(ij.right(), used, true); | ||
| if (right.output().isEmpty() || isLocalEmptyRelation(right)) { | ||
| p = ij.left(); | ||
| // InlineJoin updates the order of the output, so even if the computation is dropped, the groups need to be pulled to the end |
There was a problem hiding this comment.
I think this comment is putting an emphasis on re-ordering of the output elements, but in reality it's filtering the output based on the elements which are not needed anymore. The ordering is the same, only that some elements are filtered out.
There was a problem hiding this comment.
I've supplemented the comment.
| Thanks, Andrei.
I've added them, though with small completions, since all the computed values need to be overwritten -- only then does the bug show. |
bpintea added the auto-merge-without-approval 💔 Backport failed
You can use sqren/backport to manually backport by running |
This fixes the incorrect ordering of the columns that can occur when INLINE STATS is pruned entirely, due to its calculated value being discarded (overshadowed). Fixes elastic#136797
3 participants
This fixes the incorrect ordering of the columns that can occur when INLINE STATS is pruned entirely, due to its calculated value being discarded (overshadowed).
Fixes #136797