Skip to content

Commit 874dfb1

Browse files
[9.5] [Discover] [ES|QL] fix: stats by previous aggregated field (#290259) (#290297)
# Backport This will backport the following commits from `main` to `9.5`: - [[Discover] [ES|QL] fix: stats by previous aggregated field (#290259)](#290259) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Sebastian Delle Donne","email":"sebastian.delledonne@elastic.co"},"sourceCommit":{"committedDate":"2026-09-10T11:59:28Z","message":"[Discover] [ES|QL] fix: stats by previous aggregated field (#290259)\n\n- Closes https://github.com/elastic/kibana/issues/290060\n## Summary\n\nFixes an unhandled `TypeError: undefined is not an object (evaluating\n'groupFieldNode.arg')` thrown by `getESQLStatsQueryMeta` (used by\nDiscover's ES|QL \"cascade documents\" breakdown feature) when a query has\ntwo `STATS` commands and the second groups `BY` a field that the first\n`STATS` declared as an **aggregate** alias rather than a `BY`-grouping\nalias.\n\n### Repro\n\n```esql\nFROM kibana_sample_data_logs\n| STATS x = MAX(bytes)\n| STATS c = COUNT(*) BY x\n```\n\n### Solution\n`x` should be treated as a regular column, the code was incorrectly\nbranching into a special case as `x` was defined in the previous `STATS`\ncommand (expecting to be found as new group definition).\nAlso added a catch to avoid buggy UI when analysing the query fails,\n(discover was held in a permanent loading state before the fix)\n\n#### Before\n<img width=\"1724\" height=\"945\" alt=\"image\"\nsrc=\"https://github.com/user-attachments/assets/17ba47a5-84f3-48fc-b09f-bb6263d1ae07\"\n/>\n\n#### After\n<img width=\"1723\" height=\"902\" alt=\"image\"\nsrc=\"https://github.com/user-attachments/assets/c137f927-01e3-4705-8d0d-ad50663f08e0\"\n/>\n\n\n\n### Checklist\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios","sha":"d5e1a4e37970834570aaa52d0488081ded16123c","branchLabelMapping":{"^v9.6.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Discover","release_note:fix","Team:DataDiscovery","Team:ESQL","backport:version","v9.6.0","v9.5.5"],"title":"[Discover] [ES|QL] fix: stats by previous aggregated field","number":290259,"url":"https://github.com/elastic/kibana/pull/290259","mergeCommit":{"message":"[Discover] [ES|QL] fix: stats by previous aggregated field (#290259)\n\n- Closes https://github.com/elastic/kibana/issues/290060\n## Summary\n\nFixes an unhandled `TypeError: undefined is not an object (evaluating\n'groupFieldNode.arg')` thrown by `getESQLStatsQueryMeta` (used by\nDiscover's ES|QL \"cascade documents\" breakdown feature) when a query has\ntwo `STATS` commands and the second groups `BY` a field that the first\n`STATS` declared as an **aggregate** alias rather than a `BY`-grouping\nalias.\n\n### Repro\n\n```esql\nFROM kibana_sample_data_logs\n| STATS x = MAX(bytes)\n| STATS c = COUNT(*) BY x\n```\n\n### Solution\n`x` should be treated as a regular column, the code was incorrectly\nbranching into a special case as `x` was defined in the previous `STATS`\ncommand (expecting to be found as new group definition).\nAlso added a catch to avoid buggy UI when analysing the query fails,\n(discover was held in a permanent loading state before the fix)\n\n#### Before\n<img width=\"1724\" height=\"945\" alt=\"image\"\nsrc=\"https://github.com/user-attachments/assets/17ba47a5-84f3-48fc-b09f-bb6263d1ae07\"\n/>\n\n#### After\n<img width=\"1723\" height=\"902\" alt=\"image\"\nsrc=\"https://github.com/user-attachments/assets/c137f927-01e3-4705-8d0d-ad50663f08e0\"\n/>\n\n\n\n### Checklist\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios","sha":"d5e1a4e37970834570aaa52d0488081ded16123c"}},"sourceBranch":"main","suggestedTargetBranches":["9.5"],"targetPullRequestStates":[{"branch":"main","label":"v9.6.0","branchLabelMappingKey":"^v9.6.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/290259","number":290259,"mergeCommit":{"message":"[Discover] [ES|QL] fix: stats by previous aggregated field (#290259)\n\n- Closes https://github.com/elastic/kibana/issues/290060\n## Summary\n\nFixes an unhandled `TypeError: undefined is not an object (evaluating\n'groupFieldNode.arg')` thrown by `getESQLStatsQueryMeta` (used by\nDiscover's ES|QL \"cascade documents\" breakdown feature) when a query has\ntwo `STATS` commands and the second groups `BY` a field that the first\n`STATS` declared as an **aggregate** alias rather than a `BY`-grouping\nalias.\n\n### Repro\n\n```esql\nFROM kibana_sample_data_logs\n| STATS x = MAX(bytes)\n| STATS c = COUNT(*) BY x\n```\n\n### Solution\n`x` should be treated as a regular column, the code was incorrectly\nbranching into a special case as `x` was defined in the previous `STATS`\ncommand (expecting to be found as new group definition).\nAlso added a catch to avoid buggy UI when analysing the query fails,\n(discover was held in a permanent loading state before the fix)\n\n#### Before\n<img width=\"1724\" height=\"945\" alt=\"image\"\nsrc=\"https://github.com/user-attachments/assets/17ba47a5-84f3-48fc-b09f-bb6263d1ae07\"\n/>\n\n#### After\n<img width=\"1723\" height=\"902\" alt=\"image\"\nsrc=\"https://github.com/user-attachments/assets/c137f927-01e3-4705-8d0d-ad50663f08e0\"\n/>\n\n\n\n### Checklist\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios","sha":"d5e1a4e37970834570aaa52d0488081ded16123c"}},{"branch":"9.5","label":"v9.5.5","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Sebastian Delle Donne <sebastian.delledonne@elastic.co>
1 parent ac235c9 commit 874dfb1

2 files changed

Lines changed: 134 additions & 15 deletions

File tree

src/platform/packages/shared/kbn-esql-utils/src/utils/cascaded_documents_helpers/index.test.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
* License v3.0 only", or the "Server Side Public License, v 1".
88
*/
99

10+
import { EsqlQuery } from '@elastic/esql';
1011
import type { AggregateQuery } from '@kbn/es-query';
1112
import { type ESQLControlVariable, ESQLVariableType } from '@kbn/esql-types';
1213
import type { DataViewField } from '@kbn/data-views-plugin/common';
@@ -149,6 +150,39 @@ describe('cascaded documents helpers utils', () => {
149150
]);
150151
});
151152

153+
it('should return metadata when a group field references a field that was declared as an aggregate by a preceding command', () => {
154+
const queryString = `
155+
FROM kibana_sample_data_logs
156+
| STATS x = MAX(bytes)
157+
| STATS c = COUNT(*) BY x
158+
`;
159+
160+
const result = getESQLStatsQueryMeta(queryString);
161+
162+
expect(result.groupByFields).toEqual([{ field: 'x', type: 'column' }]);
163+
expect(result.appliedFunctions).toEqual([{ identifier: 'c', aggregation: 'COUNT' }]);
164+
});
165+
166+
it('should return empty metadata instead of throwing when query metadata computation fails unexpectedly', () => {
167+
const fromSrcSpy = jest.spyOn(EsqlQuery, 'fromSrc').mockImplementation(() => {
168+
throw new Error('unexpected parse failure');
169+
});
170+
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
171+
172+
try {
173+
expect(
174+
getESQLStatsQueryMeta('FROM kibana_sample_data_logs | STATS COUNT(*) BY host')
175+
).toEqual({
176+
groupByFields: [],
177+
appliedFunctions: [],
178+
});
179+
expect(consoleErrorSpy).toHaveBeenCalled();
180+
} finally {
181+
fromSrcSpy.mockRestore();
182+
consoleErrorSpy.mockRestore();
183+
}
184+
});
185+
152186
it('should return a single group by field if there is a where command following a STATS by command targeting a column specified as a grouping option in the operating stats command', () => {
153187
const queryString = `
154188
FROM kibana_sample_data_logs
@@ -367,6 +401,33 @@ describe('cascaded documents helpers utils', () => {
367401
);
368402
});
369403

404+
it('should construct a cascade leaf query when a later STATS groups by a prior aggregate alias', () => {
405+
const editorQuery: AggregateQuery = {
406+
esql: `
407+
FROM kibana_sample_data_logs
408+
| STATS x = MAX(bytes)
409+
| STATS c = COUNT(*) BY x
410+
`,
411+
};
412+
413+
const nodePath = ['x'];
414+
const nodePathMap = { x: '33' };
415+
416+
const cascadeQuery = constructCascadeQuery({
417+
query: editorQuery,
418+
dataView: dataViewMock,
419+
esqlVariables: [],
420+
nodeType,
421+
nodePath,
422+
nodePathMap,
423+
});
424+
425+
expect(cascadeQuery).toBeDefined();
426+
expect(cascadeQuery!.esql).toBe(
427+
'FROM kibana_sample_data_logs | INLINE STATS x = MAX(bytes) | INLINE STATS c = COUNT(*) BY x | WHERE x == 33'
428+
);
429+
});
430+
370431
it('generate a valid cascade leaf query for a valid stats command that has a parameter value for a grouping option', () => {
371432
const editorQuery: AggregateQuery = {
372433
esql: `
@@ -916,6 +977,22 @@ describe('cascaded documents helpers utils', () => {
916977
'FROM kibana_sample_data_logs | WHERE `agent.keyword` == "Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 1.1.4322)" | STATS count = COUNT(*) BY agent.keyword, extension.keyword | STATS avg = AVG(count) BY agent.keyword'
917978
);
918979
});
980+
981+
it('handles filtering on a group field that was declared as an aggregate by a preceding STATS', () => {
982+
expect(
983+
appendFilteringWhereClauseForCascadeLayout(
984+
'FROM kibana_sample_data_logs | STATS x = MAX(bytes) | STATS c = COUNT(*) BY x',
985+
[],
986+
dataViewMock,
987+
'x',
988+
33,
989+
'+',
990+
'integer'
991+
)
992+
).toBe(
993+
'FROM kibana_sample_data_logs | STATS x = MAX(bytes) | WHERE x == 33 | STATS c = COUNT(*) BY x'
994+
);
995+
});
919996
});
920997

921998
describe('handling for param fields', () => {

src/platform/packages/shared/kbn-esql-utils/src/utils/cascaded_documents_helpers/index.ts

Lines changed: 57 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -88,12 +88,33 @@ export interface ESQLStatsQueryMeta {
8888
appliedFunctions: AppliedStatsFunction[];
8989
}
9090

91+
const EMPTY_ESQL_STATS_QUERY_META: ESQLStatsQueryMeta = {
92+
groupByFields: [],
93+
appliedFunctions: [],
94+
};
95+
96+
const getStatsGroupingField = (
97+
summary: StatsCommandSummary,
98+
field: string
99+
): FieldSummary | undefined => summary.grouping[field] ?? summary.grouping[`\`${field}\``];
100+
91101
/**
92102
* This method is used to get the metadata on STATS command to drive the cascade experience from an ESQL query,
93103
* if a valid STATS command is found information about the group by fields and applied functions is returned.
94104
* This method will exclude queries contain commands that are not valid for the cascade experience,
95105
*/
96106
export const getESQLStatsQueryMeta = (queryString: string): ESQLStatsQueryMeta => {
107+
try {
108+
return computeESQLStatsQueryMeta(queryString);
109+
} catch (error) {
110+
// Unexpected AST/parse failures must not take down Discover (or other callers).
111+
// eslint-disable-next-line no-console
112+
console.error('Failed to compute ES|QL stats query metadata for cascade documents', error);
113+
return EMPTY_ESQL_STATS_QUERY_META;
114+
}
115+
};
116+
117+
const computeESQLStatsQueryMeta = (queryString: string): ESQLStatsQueryMeta => {
97118
const groupByFields: ESQLStatsQueryMeta['groupByFields'] = [];
98119
const appliedFunctions: ESQLStatsQueryMeta['appliedFunctions'] = [];
99120

@@ -163,9 +184,17 @@ export const getESQLStatsQueryMeta = (queryString: string): ESQLStatsQueryMeta =
163184
groupDeclarationStatsCommandLookupIndex
164185
))
165186
) {
166-
groupDeclarationStatsCommandIndex = groupDeclarationStatsCommandLookupIndex;
167-
// update the group field node to it's actual definition
168-
groupFieldNode = groupDeclarationCommandSummary.grouping[group.field];
187+
const resolvedGroupField = getStatsGroupingField(
188+
groupDeclarationCommandSummary,
189+
group.field
190+
);
191+
if (resolvedGroupField) {
192+
groupDeclarationStatsCommandIndex = groupDeclarationStatsCommandLookupIndex;
193+
// update the group field node to its actual grouping definition
194+
groupFieldNode = resolvedGroupField;
195+
}
196+
// If the preceding STATS created this field as an aggregate, keep the current
197+
// STATS grouping node (a column), the same way EVAL-created fields are handled.
169198
}
170199
}
171200

@@ -350,12 +379,17 @@ export const constructCascadeQuery = ({
350379
);
351380
}
352381

353-
fieldDeclarationCommandSummary = groupDeclarationCommandSummary
354-
? {
355-
...groupDeclarationCommandSummary,
356-
index: groupDeclarationCommandIndex,
357-
}
358-
: fieldDeclarationCommandSummary;
382+
// Only walk back to a preceding STATS when that command declared the field as a grouping
383+
// key (e.g. CATEGORIZE). Aggregate aliases are just columns on the operating STATS BY clause.
384+
if (
385+
groupDeclarationCommandSummary &&
386+
getStatsGroupingField(groupDeclarationCommandSummary, pathSegment)
387+
) {
388+
fieldDeclarationCommandSummary = {
389+
...groupDeclarationCommandSummary,
390+
index: groupDeclarationCommandIndex,
391+
};
392+
}
359393
}
360394

361395
const groupValue =
@@ -708,12 +742,20 @@ export const appendFilteringWhereClauseForCascadeLayout = <
708742
fieldDeclarationCommandSummary.index !== groupDeclarationCommandIndex
709743
) {
710744
filterTargetIsRuntimeField = true;
711-
// update the field declaration command summary to the stats command
712-
// that declared the field the filtering operation is targeting
713-
fieldDeclarationCommandSummary = {
714-
...getStatsCommandAtIndexSummary(ESQLQuery, groupDeclarationCommandIndex)!,
715-
index: groupDeclarationCommandIndex,
716-
};
745+
const declaredSummary = getStatsCommandAtIndexSummary(
746+
ESQLQuery,
747+
groupDeclarationCommandIndex
748+
);
749+
const declaredGroupingField =
750+
declaredSummary && getStatsGroupingField(declaredSummary, rawFieldName);
751+
if (declaredGroupingField && declaredSummary) {
752+
// update the field declaration command summary to the stats command
753+
// that declared the field the filtering operation is targeting
754+
fieldDeclarationCommandSummary = {
755+
...declaredSummary,
756+
index: groupDeclarationCommandIndex,
757+
};
758+
}
717759
}
718760
}
719761

0 commit comments

Comments
 (0)