Skip to content

Conversation

@alex-spies
Copy link
Contributor

@alex-spies alex-spies commented Dec 2, 2024

Essentially a forward-port of #117832

Since #104958, AggregateExec cannot have aliases in its aggregates - because any such aliases are turned into Evals.

But before that, a STATS command like

...
| rename languages as l
stats c = count(e) by l

would result in an AggregateExec like

AggregateExec[[l{r}#1],[COUNT(emp_no{f}#4,true[BOOLEAN]) AS c, languages{f}#5 AS l],INITIAL,[l{r}#1, count{r}#10, seen{r}#11],nu = AggregateExec[[l{r}#1],[COUNT(emp_no{f}#4,true[BOOLEAN]) AS c, languages{f}#5 AS l],INITIAL,[l{r}#1, count{r}#10, seen{r}#11],nu
ll]  

where the renaming languages{f}#5 AS l is pushed into the aggregates and the groupings refer to it via the reference attribute [l{r}#1].

AbstractPhysicalOperationProviders.groupingPhysicalOperation still addresses this remnant of times long past by checking whether AggregateExec has any Aliases in its aggregates, and adjusting the aggregation's layout so it points to the source attribute languages{f}#5, not the renamed attribute l{r}#1 (because the renamed attribute is not present in the source layout!)

Let's get rid of this dead code since it is confusing.

@alex-spies alex-spies added >refactoring test-full-bwc Trigger full BWC version matrix tests :Analytics/ES|QL AKA ESQL v9.0.0 labels Dec 2, 2024
@alex-spies alex-spies requested review from astefan and costin December 2, 2024 14:38
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Dec 2, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

@alex-spies alex-spies marked this pull request as draft December 2, 2024 15:08
@alex-spies
Copy link
Contributor Author

Reverting to draft: some union type tests are failing, so it seems this code path is still used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL >refactoring Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) test-full-bwc Trigger full BWC version matrix tests v9.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants