[ES|QL] Clean STATS mutate summarise method#250688
Conversation
| * definition: <AST node representing 'field'> | ||
| * } | ||
| */ | ||
| definition: ESQLProperNode; |
There was a problem hiding this comment.
I'm not totally kind in keeping this definition field here, IMO it's a derivative of arg, and we should provide a helper so the ones who need it can get it easily (similar to what I did with terminals).
For the moment added a better description to it as it was a bit difficult to differentiate between arg, definition, column and terminal.
There was a problem hiding this comment.
I dont like it either tbh, I agree with this ++
There was a problem hiding this comment.
Ok will move it in a another PR then, as doing it here will bring more conflicts with #250560
sddonne
added
Feature:ES|QL
|
Pinging @elastic/kibana-esql (Team:ESQL) |
💚 Build Succeeded
Metrics [docs]Public APIs missing comments
Async chunks
Public APIs missing exports
Page load bundle
|
| expectedGrouping: ['field1'], | ||
| }, | ||
| { | ||
| description: 'STATS with assigned aggregation and assigned grouping columns', |
There was a problem hiding this comment.
Seb add also a test for this
BY BUCKET(@timestamp,50,?_tstart,?_tend)
| */ | ||
| renamedColumnsPairs?: Set<[string, string]>; | ||
|
|
||
| grouping?: Set<FieldSummary>; |
There was a problem hiding this comment.
Add some descriptions here as I have at the others
| * definition: <AST node representing 'field'> | ||
| * } | ||
| */ | ||
| definition: ESQLProperNode; |
There was a problem hiding this comment.
I dont like it either tbh, I agree with this ++
eokoneyo
left a comment
eokoneyo
left a comment
There was a problem hiding this comment.
Tested changes locally, LGTM... thanks for this
Closes elastic#250156 ## Summary This PR gets rid of `mutate.commands.stats.summarizeCommand` in favour of adding `grouping` and `aggregations` to the `sumarizeQuery` method. * A bug that was producing wrong column names when using `WHERE` function within `STATS` has been solved. Cleanup of the summary fields * `usedFields` has been removed from the summary. * `column` has been removed from the summary. * `terminals` has been removed from the summary and a helper has been added to extract it. * Relevant clients of the summary has been carefully refactored to adapt to the new signature. --------- Co-authored-by: Stratou <efstratia.kalafateli@elastic.co>
5 participants
Closes #250156
Summary
This PR gets rid of
mutate.commands.stats.summarizeCommandin favour of addinggroupingandaggregationsto thesumarizeQuerymethod.WHEREfunction withinSTATShas been solved.Cleanup of the summary fields
usedFieldshas been removed from the summary.columnhas been removed from the summary.terminalshas been removed from the summary and a helper has been added to extract it.