diff --git a/.changeset/composed-outer-clauses.md b/.changeset/composed-outer-clauses.md new file mode 100644 index 0000000000..b8026c23e6 --- /dev/null +++ b/.changeset/composed-outer-clauses.md @@ -0,0 +1,5 @@ +--- +'@hyperdx/common-utils': patch +--- + +HAVING, ORDER BY and LIMIT on multi-series metric charts now apply to the final joined result instead of leaking into each per-series branch. They reference the chart's output columns — operand aliases, formula names/aliases, the ratio column, group-by columns and the time bucket — so a HAVING like `"err rate" > 0.5` filters the joined rows, ORDER BY actually orders the result (previously it was applied per branch and then discarded by the join), and LIMIT/OFFSET paginate one consistent group set across all series. diff --git a/packages/common-utils/src/__tests__/__snapshots__/renderChartConfig.test.ts.snap b/packages/common-utils/src/__tests__/__snapshots__/renderChartConfig.test.ts.snap index 74899add02..c8bb034ece 100644 --- a/packages/common-utils/src/__tests__/__snapshots__/renderChartConfig.test.ts.snap +++ b/packages/common-utils/src/__tests__/__snapshots__/renderChartConfig.test.ts.snap @@ -890,6 +890,130 @@ exports[`renderChartConfig multi-series metric charts (composed query) formulas ) AS "__hdx_value",toStartOfInterval(toDateTime(__hdx_time_bucket2), INTERVAL 1 minute) AS \`__hdx_time_bucket\` FROM Bucketed WHERE (__hdx_time_bucket2 >= fromUnixTimestamp64Milli(1739318400000) AND __hdx_time_bucket2 <= fromUnixTimestamp64Milli(1739491200000)) GROUP BY toStartOfInterval(toDateTime(__hdx_time_bucket2), INTERVAL 1 minute) AS \`__hdx_time_bucket\` ORDER BY toStartOfInterval(toDateTime(__hdx_time_bucket2), INTERVAL 1 minute) AS \`__hdx_time_bucket\`)) GROUP BY ALL ORDER BY \`__hdx_time_bucket\` SETTINGS short_circuit_function_evaluation = 'force_enable', optimize_read_in_order = 0, cast_keep_nullable = 1, additional_result_filter = 'x != 2', count_distinct_implementation = 'uniqCombined64', async_insert_busy_timeout_min_ms = 20000" `; +exports[`renderChartConfig multi-series metric charts (composed query) outer HAVING / ORDER BY / LIMIT filters a share_of_total ratio through a wrapper, not HAVING (window function) 1`] = ` +"SELECT * FROM (SELECT coalesce(anyOrNullIf(\`__hdx_value\`, \`__hdx_series_idx\` = 0), 0) / nullif(sum(anyOrNullIf(\`__hdx_value\`, \`__hdx_series_idx\` = 1)) OVER (), 0) AS "avg(metric.alpha)/avg(metric.beta)", * EXCEPT (\`__hdx_value\`, \`__hdx_series_idx\`) FROM (SELECT * REPLACE (toFloat64(\`__hdx_value\`) AS \`__hdx_value\`), 0 AS \`__hdx_series_idx\` FROM (WITH Source AS ( + SELECT + *, + cityHash64(ScopeAttributes, ResourceAttributes, Attributes) AS AttributesHash + FROM default.otel_metrics_gauge + WHERE (TimeUnix >= fromUnixTimestamp64Milli(1739318400000) AND TimeUnix <= fromUnixTimestamp64Milli(1739491200000)) AND ((MetricName = 'metric.alpha')) + ),Bucketed AS ( + SELECT + toStartOfInterval(toDateTime(TimeUnix), INTERVAL 1 hour) AS \`__hdx_time_bucket2\`, + AttributesHash, + last_value(Value) AS LastValue, + any(ScopeAttributes) AS ScopeAttributes, + any(ResourceAttributes) AS ResourceAttributes, + any(Attributes) AS Attributes, + any(ResourceSchemaUrl) AS ResourceSchemaUrl, + any(ScopeName) AS ScopeName, + any(ScopeVersion) AS ScopeVersion, + any(ScopeDroppedAttrCount) AS ScopeDroppedAttrCount, + any(ScopeSchemaUrl) AS ScopeSchemaUrl, + any(ServiceName) AS ServiceName, + any(MetricDescription) AS MetricDescription, + any(MetricUnit) AS MetricUnit, + any(StartTimeUnix) AS StartTimeUnix, + any(Flags) AS Flags + FROM Source + GROUP BY AttributesHash, __hdx_time_bucket2 + ORDER BY AttributesHash, __hdx_time_bucket2 + ) SELECT avg( + toFloat64OrDefault(toString(LastValue)) + ) AS "__hdx_value",ServiceName FROM Bucketed WHERE (__hdx_time_bucket2 >= fromUnixTimestamp64Milli(1739318400000) AND __hdx_time_bucket2 <= fromUnixTimestamp64Milli(1739491200000)) GROUP BY ServiceName) UNION ALL SELECT * REPLACE (toFloat64(\`__hdx_value\`) AS \`__hdx_value\`), 1 AS \`__hdx_series_idx\` FROM (WITH Source AS ( + SELECT + *, + cityHash64(ScopeAttributes, ResourceAttributes, Attributes) AS AttributesHash + FROM default.otel_metrics_gauge + WHERE (TimeUnix >= fromUnixTimestamp64Milli(1739318400000) AND TimeUnix <= fromUnixTimestamp64Milli(1739491200000)) AND ((MetricName = 'metric.beta')) + ),Bucketed AS ( + SELECT + toStartOfInterval(toDateTime(TimeUnix), INTERVAL 1 hour) AS \`__hdx_time_bucket2\`, + AttributesHash, + last_value(Value) AS LastValue, + any(ScopeAttributes) AS ScopeAttributes, + any(ResourceAttributes) AS ResourceAttributes, + any(Attributes) AS Attributes, + any(ResourceSchemaUrl) AS ResourceSchemaUrl, + any(ScopeName) AS ScopeName, + any(ScopeVersion) AS ScopeVersion, + any(ScopeDroppedAttrCount) AS ScopeDroppedAttrCount, + any(ScopeSchemaUrl) AS ScopeSchemaUrl, + any(ServiceName) AS ServiceName, + any(MetricDescription) AS MetricDescription, + any(MetricUnit) AS MetricUnit, + any(StartTimeUnix) AS StartTimeUnix, + any(Flags) AS Flags + FROM Source + GROUP BY AttributesHash, __hdx_time_bucket2 + ORDER BY AttributesHash, __hdx_time_bucket2 + ) SELECT avg( + toFloat64OrDefault(toString(LastValue)) + ) AS "__hdx_value",ServiceName FROM Bucketed WHERE (__hdx_time_bucket2 >= fromUnixTimestamp64Milli(1739318400000) AND __hdx_time_bucket2 <= fromUnixTimestamp64Milli(1739491200000)) GROUP BY ServiceName)) GROUP BY ALL) WHERE "avg(metric.alpha)/avg(metric.beta)" >= 0.2 ORDER BY "avg(metric.alpha)/avg(metric.beta)" DESC LIMIT 2 SETTINGS short_circuit_function_evaluation = 'force_enable', optimize_read_in_order = 0, cast_keep_nullable = 1, additional_result_filter = 'x != 2', count_distinct_implementation = 'uniqCombined64', async_insert_busy_timeout_min_ms = 20000" +`; + +exports[`renderChartConfig multi-series metric charts (composed query) outer HAVING / ORDER BY / LIMIT renders having, orderBy and limit once, on the outer joined statement only 1`] = ` +"SELECT anyOrNullIf(\`__hdx_value\`, \`__hdx_series_idx\` = 0) AS "avg(metric.alpha)", anyOrNullIf(\`__hdx_value\`, \`__hdx_series_idx\` = 1) AS "avg(metric.beta)", * EXCEPT (\`__hdx_value\`, \`__hdx_series_idx\`) FROM (SELECT * REPLACE (toFloat64(\`__hdx_value\`) AS \`__hdx_value\`), 0 AS \`__hdx_series_idx\` FROM (WITH Source AS ( + SELECT + *, + cityHash64(ScopeAttributes, ResourceAttributes, Attributes) AS AttributesHash + FROM default.otel_metrics_gauge + WHERE (TimeUnix >= fromUnixTimestamp64Milli(1739318400000) AND TimeUnix <= fromUnixTimestamp64Milli(1739491200000)) AND ((MetricName = 'metric.alpha')) + ),Bucketed AS ( + SELECT + toStartOfInterval(toDateTime(TimeUnix), INTERVAL 1 hour) AS \`__hdx_time_bucket2\`, + AttributesHash, + last_value(Value) AS LastValue, + any(ScopeAttributes) AS ScopeAttributes, + any(ResourceAttributes) AS ResourceAttributes, + any(Attributes) AS Attributes, + any(ResourceSchemaUrl) AS ResourceSchemaUrl, + any(ScopeName) AS ScopeName, + any(ScopeVersion) AS ScopeVersion, + any(ScopeDroppedAttrCount) AS ScopeDroppedAttrCount, + any(ScopeSchemaUrl) AS ScopeSchemaUrl, + any(ServiceName) AS ServiceName, + any(MetricDescription) AS MetricDescription, + any(MetricUnit) AS MetricUnit, + any(StartTimeUnix) AS StartTimeUnix, + any(Flags) AS Flags + FROM Source + GROUP BY AttributesHash, __hdx_time_bucket2 + ORDER BY AttributesHash, __hdx_time_bucket2 + ) SELECT avg( + toFloat64OrDefault(toString(LastValue)) + ) AS "__hdx_value",ServiceName FROM Bucketed WHERE (__hdx_time_bucket2 >= fromUnixTimestamp64Milli(1739318400000) AND __hdx_time_bucket2 <= fromUnixTimestamp64Milli(1739491200000)) GROUP BY ServiceName) UNION ALL SELECT * REPLACE (toFloat64(\`__hdx_value\`) AS \`__hdx_value\`), 1 AS \`__hdx_series_idx\` FROM (WITH Source AS ( + SELECT + *, + cityHash64(ScopeAttributes, ResourceAttributes, Attributes) AS AttributesHash + FROM default.otel_metrics_gauge + WHERE (TimeUnix >= fromUnixTimestamp64Milli(1739318400000) AND TimeUnix <= fromUnixTimestamp64Milli(1739491200000)) AND ((MetricName = 'metric.beta')) + ),Bucketed AS ( + SELECT + toStartOfInterval(toDateTime(TimeUnix), INTERVAL 1 hour) AS \`__hdx_time_bucket2\`, + AttributesHash, + last_value(Value) AS LastValue, + any(ScopeAttributes) AS ScopeAttributes, + any(ResourceAttributes) AS ResourceAttributes, + any(Attributes) AS Attributes, + any(ResourceSchemaUrl) AS ResourceSchemaUrl, + any(ScopeName) AS ScopeName, + any(ScopeVersion) AS ScopeVersion, + any(ScopeDroppedAttrCount) AS ScopeDroppedAttrCount, + any(ScopeSchemaUrl) AS ScopeSchemaUrl, + any(ServiceName) AS ServiceName, + any(MetricDescription) AS MetricDescription, + any(MetricUnit) AS MetricUnit, + any(StartTimeUnix) AS StartTimeUnix, + any(Flags) AS Flags + FROM Source + GROUP BY AttributesHash, __hdx_time_bucket2 + ORDER BY AttributesHash, __hdx_time_bucket2 + ) SELECT avg( + toFloat64OrDefault(toString(LastValue)) + ) AS "__hdx_value",ServiceName FROM Bucketed WHERE (__hdx_time_bucket2 >= fromUnixTimestamp64Milli(1739318400000) AND __hdx_time_bucket2 <= fromUnixTimestamp64Milli(1739491200000)) GROUP BY ServiceName)) GROUP BY ALL HAVING "avg(metric.alpha)" > 10 ORDER BY "avg(metric.beta)" DESC LIMIT 5 OFFSET 10 SETTINGS short_circuit_function_evaluation = 'force_enable', optimize_read_in_order = 0, cast_keep_nullable = 1, additional_result_filter = 'x != 2', count_distinct_implementation = 'uniqCombined64', async_insert_busy_timeout_min_ms = 20000" +`; + exports[`renderChartConfig multi-series metric charts (composed query) pads group columns across gauge and histogram branch classes 1`] = ` "SELECT anyOrNullIf(\`__hdx_value\`, \`__hdx_series_idx\` = 0) AS "avg(metric.alpha)", anyOrNullIf(\`__hdx_value\`, \`__hdx_series_idx\` = 1) AS "quantile(metric.latency)", * EXCEPT (\`__hdx_value\`, \`__hdx_series_idx\`) FROM (SELECT * REPLACE (toFloat64(\`__hdx_value\`) AS \`__hdx_value\`), 0 AS \`__hdx_series_idx\`, [] AS \`group\` FROM (WITH Source AS ( SELECT diff --git a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts index 0493df923a..d7c188434d 100644 --- a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts +++ b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts @@ -2028,5 +2028,357 @@ describe('queryChartConfig Integration Tests', () => { expect(Number(col(byService.get('svc-b'), 'ratio'))).toBe(0); }); }); + + // HAVING / ORDER BY / LIMIT apply to the final joined result and + // reference its output columns — not each per-series branch, where the + // output names don't exist and each series would be + // filtered/ordered/truncated independently. + // + // Fixture recap (grpratio.* grouped by ServiceName, table shape — gauge + // reads the last value per series when time is ungrouped): + // avg(grpratio.err): svc-a 2, svc-b 6, svc-d 5, svc-c gap + // avg(grpratio.total): svc-a 5, svc-b 12, svc-c 8, svc-d gap + describe('outer HAVING / ORDER BY / LIMIT', () => { + const grpRatioTable = (overrides: Partial) => + baseConfig({ + displayType: DisplayType.Table, + granularity: undefined, + select: [gaugeSelect('grpratio.err'), gaugeSelect('grpratio.total')], + groupBy: [{ aggCondition: '', valueExpression: 'ServiceName' }], + ...overrides, + }); + + const services = (data: unknown) => + (data as Row[]).map(r => col(r, 'ServiceName')); + + it('filters the joined rows with HAVING on an operand output column', async () => { + const result = await runConfig( + grpRatioTable({ + having: '"avg(grpratio.total)" > 5', + havingLanguage: 'sql', + }), + ); + + // svc-a (4.5) fails the predicate; svc-d has err data but a NULL + // total, and NULL > 5 filters out. A per-branch HAVING could never + // drop svc-d — its err branch has no total column to inspect. + expect(services(result.data).sort()).toEqual(['svc-b', 'svc-c']); + }); + + it('filters with HAVING on a formula output column', async () => { + const result = await runConfig( + grpRatioTable({ + formulas: [{ expression: 'A / B', alias: 'err rate' }], + having: '"err rate" >= 0.5', + havingLanguage: 'sql', + }), + ); + + // Rates: svc-a 2/5=0.4, svc-b 0.5, svc-c 0/8=0, svc-d gap. + expect(services(result.data)).toEqual(['svc-b']); + expect(Number(col((result.data as Row[])[0], 'err rate'))).toBeCloseTo( + 0.5, + 5, + ); + }); + + it('orders by a plain group column across the joined result', async () => { + const result = await runConfig( + grpRatioTable({ + orderBy: [{ valueExpression: 'ServiceName', ordering: 'DESC' }], + }), + ); + + expect(services(result.data)).toEqual([ + 'svc-d', + 'svc-c', + 'svc-b', + 'svc-a', + ]); + }); + + it('orders by an output value column and paginates the joined result with LIMIT/OFFSET', async () => { + const orderBy = [ + { + valueExpression: '"avg(grpratio.total)"', + ordering: 'DESC' as const, + }, + ]; + + // Full order: svc-b (12), svc-c (8), svc-a (4.5), svc-d (NULL — + // ClickHouse sorts NULLS LAST by default). + const page1 = await runConfig( + grpRatioTable({ orderBy, limit: { limit: 2 } }), + ); + expect(services(page1.data)).toEqual(['svc-b', 'svc-c']); + + // The second page continues the SAME joined ordering — page windows + // are disjoint and the group universe is consistent across series. + // (A per-branch LIMIT truncated each series to its own arbitrary + // groups before the join, so pages neither aligned nor partitioned.) + const page2 = await runConfig( + grpRatioTable({ orderBy, limit: { limit: 2, offset: 2 } }), + ); + expect(services(page2.data)).toEqual(['svc-a', 'svc-d']); + + // The joined row is intact on every page: svc-d keeps its err value + // and its total gap. + const svcD = (page2.data as Row[])[1]; + expect(Number(col(svcD, 'avg(grpratio.err)'))).toBe(5); + expectGap(col(svcD, 'avg(grpratio.total)')); + }); + + it('orders time-series rows by bucket first, user sort second', async () => { + const result = await runConfig( + baseConfig({ + select: [gaugeSelect('grp.one'), gaugeSelect('grp.two')], + groupBy: [{ aggCondition: '', valueExpression: 'ServiceName' }], + orderBy: [{ valueExpression: 'ServiceName', ordering: 'DESC' }], + }), + ); + + // All rows share bucket 0; the user sort breaks the tie in reverse + // service order. + expect(services(result.data)).toEqual(['svc-c', 'svc-b', 'svc-a']); + }); + + it('resolves an expression group-by in ORDER BY via its derived output name', async () => { + // The passthrough column for an expression group-by keeps its + // ClickHouse-derived name. The contract for referencing it from + // ORDER BY/HAVING is the (quoted) output name — the raw map-access + // expression is not resolvable in the outer scope, where the source + // columns no longer exist. + const result = await runConfig( + grpRatioTable({ + groupBy: [ + { + aggCondition: '', + valueExpression: "ResourceAttributes['service.name']", + }, + ], + orderBy: [ + { + valueExpression: `"arrayElement(ResourceAttributes, 'service.name')"`, + ordering: 'DESC', + }, + ], + }), + ); + + const DERIVED_NAME = "arrayElement(ResourceAttributes, 'service.name')"; + expect((result.data as Row[]).map(r => col(r, DERIVED_NAME))).toEqual([ + 'svc-d', + 'svc-c', + 'svc-b', + 'svc-a', + ]); + }); + + it('orders by an aliased expression group-by through the alias', async () => { + const result = await runConfig( + grpRatioTable({ + groupBy: [ + { + aggCondition: '', + valueExpression: "ResourceAttributes['service.name']", + alias: 'service', + }, + ], + orderBy: [{ valueExpression: 'service', ordering: 'ASC' }], + }), + ); + + expect((result.data as Row[]).map(r => col(r, 'service'))).toEqual([ + 'svc-a', + 'svc-b', + 'svc-c', + 'svc-d', + ]); + }); + + it('filters and orders the ratio output column', async () => { + const result = await runConfig( + grpRatioTable({ + seriesReturnType: 'ratio', + having: '"avg(grpratio.err)/avg(grpratio.total)" >= 0.3', + havingLanguage: 'sql', + orderBy: [ + { + valueExpression: '"avg(grpratio.err)/avg(grpratio.total)"', + ordering: 'DESC', + }, + ], + }), + ); + + // Rates: svc-a 0.4, svc-b 0.5, svc-c 0, svc-d gap (NULL fails the + // predicate). + expect(services(result.data)).toEqual(['svc-b', 'svc-a']); + }); + + it('filters a share_of_total ratio after its window function evaluates', async () => { + // share_of_total is built on sum(...) OVER (...), which ClickHouse + // prohibits inside HAVING — the filter runs as WHERE on a wrapper + // around the joined result instead. + const RATIO = 'avg(grpratio.err)/avg(grpratio.total)'; + const result = await runConfig( + grpRatioTable({ + seriesReturnType: 'ratio', + ratioMode: 'share_of_total', + having: `"${RATIO}" >= 0.15`, + havingLanguage: 'sql', + orderBy: [{ valueExpression: `"${RATIO}"`, ordering: 'DESC' }], + }), + ); + + // Denominator total across ALL groups (gauge reads the last value + // per series on the ungrouped-time table shape, so svc-a's total is + // 5): 5 + 12 + 8 = 25. Shares: svc-a 2/25 = 0.08, svc-b 6/25 = 0.24, + // svc-c 0, svc-d 5/25 = 0.2 — so >= 0.15 keeps b and d. + expect(services(result.data)).toEqual(['svc-b', 'svc-d']); + // The share divides by the pre-filter total (25), proving the window + // evaluated over the full joined result before the filter. + const rows = result.data as Row[]; + expect(Number(col(rows[0], RATIO))).toBeCloseTo(6 / 25, 5); + expect(Number(col(rows[1], RATIO))).toBeCloseTo(5 / 25, 5); + }); + + it('partitions the share_of_total window per bucket on a time series, then filters', async () => { + const RATIO = 'avg(grpratio.err)/avg(grpratio.total)'; + const result = await runConfig( + baseConfig({ + select: [ + gaugeSelect('grpratio.err'), + gaugeSelect('grpratio.total'), + ], + groupBy: [{ aggCondition: '', valueExpression: 'ServiceName' }], + seriesReturnType: 'ratio', + ratioMode: 'share_of_total', + having: `"${RATIO}" >= 0.2`, + havingLanguage: 'sql', + orderBy: [{ valueExpression: `"${RATIO}"`, ordering: 'DESC' }], + }), + ); + + // Per-bucket totals: bucket 0 = 4 + 12 + 8 = 24 (shares a 1/24, + // b 0.25, c 0, d 5/24); bucket 1 = 5 (share a 2/5 = 0.4). The + // >= 0.2 filter keeps (b0, b), (b0, d) and (b1, a); rows stay + // bucket-ordered first (outermost ORDER BY, outside the wrapper), + // share-descending within the bucket. + const rows = result.data as Row[]; + expect( + rows.map(r => [ + String(col(r, '__hdx_time_bucket')), + col(r, 'ServiceName'), + ]), + ).toEqual([ + [bucket(0), 'svc-b'], + [bucket(0), 'svc-d'], + [bucket(1), 'svc-a'], + ]); + expect(Number(col(rows[0], RATIO))).toBeCloseTo(6 / 24, 5); + expect(Number(col(rows[1], RATIO))).toBeCloseTo(5 / 24, 5); + expect(Number(col(rows[2], RATIO))).toBeCloseTo(2 / 5, 5); + }); + + it('filters number-shape results with HAVING (no GROUP BY)', async () => { + const numberConfig = (having: string) => + baseConfig({ + displayType: DisplayType.Number, + granularity: undefined, + select: [gaugeSelect('tbl.one'), gaugeSelect('tbl.two')], + having, + havingLanguage: 'sql', + }); + + // The number shape has no passthrough columns, so the outer query is + // one implicit global aggregation — HAVING filters its single row. + // Values: avg(tbl.one) = 15, avg(tbl.two) = 100. + const kept = await runConfig(numberConfig('"avg(tbl.two)" > 50')); + expect(kept.data as Row[]).toHaveLength(1); + expect(col((kept.data as Row[])[0], 'avg(tbl.one)')).toBe(15); + + const dropped = await runConfig(numberConfig('"avg(tbl.two)" > 200')); + expect(dropped.data as Row[]).toHaveLength(0); + }); + + it('filters time-series (bucket, group) rows with HAVING', async () => { + const result = await runConfig( + baseConfig({ + select: [gaugeSelect('grp.one'), gaugeSelect('grp.two')], + groupBy: [{ aggCondition: '', valueExpression: 'ServiceName' }], + having: '"avg(grp.one)" >= 2', + havingLanguage: 'sql', + }), + ); + + // Joined bucket-0 rows: svc-a (grp.one 1), svc-b (grp.one 2), + // svc-c (grp.one NULL — grp.two only). Only svc-b passes; NULL fails + // the predicate like any SQL comparison. + const rows = result.data as Row[]; + expect(rows).toHaveLength(1); + expect(col(rows[0], 'ServiceName')).toBe('svc-b'); + expect(String(col(rows[0], '__hdx_time_bucket'))).toBe(bucket(0)); + expect(col(rows[0], 'avg(grp.one)')).toBe(2); + }); + + it('applies HAVING across heterogeneous branch classes (gauge + histogram)', async () => { + const result = await runConfig( + baseConfig({ + select: [ + gaugeSelect('grpmix.gauge'), + histQuantileSelect('grpmix.latency', 0.5), + ], + groupBy: [{ aggCondition: '', valueExpression: 'ServiceName' }], + having: '"avg(grpmix.gauge)" >= 0', + havingLanguage: 'sql', + }), + ); + + // Gauge and histogram rows never share a merge key (plain vs Array + // group columns), so the histogram row carries a NULL gauge value + // and the HAVING on the gauge column drops it. + const rows = result.data as Row[]; + expect(rows).toHaveLength(1); + expect(col(rows[0], 'ServiceName')).toBe('svc-a'); + expect(col(rows[0], 'avg(grpmix.gauge)')).toBe(42); + expectGap(col(rows[0], 'quantile(grpmix.latency)')); + }); + + it('orders by a formula output column', async () => { + const result = await runConfig( + grpRatioTable({ + formulas: [{ expression: 'A / B', alias: 'err rate' }], + orderBy: [{ valueExpression: '"err rate"', ordering: 'DESC' }], + limit: { limit: 2 }, + }), + ); + + // Rates (last-value gauge semantics on the table shape): svc-a + // 2/5 = 0.4, svc-b 0.5, svc-c 0, svc-d gap (NULL sorts last). + expect(services(result.data)).toEqual(['svc-b', 'svc-a']); + expect(Number(col((result.data as Row[])[0], 'err rate'))).toBeCloseTo( + 0.5, + 5, + ); + }); + + it('rejects HAVING on an operand hidden by showOperandSeries: false', async () => { + // The contract is "reference what the result outputs": with the + // operand series dropped from the projection, their names are not + // resolvable — the query fails instead of silently filtering on a + // column the chart doesn't show. + await expect( + runConfig( + grpRatioTable({ + formulas: [{ expression: 'A / B', alias: 'err rate' }], + showOperandSeries: false, + having: '"avg(grpratio.err)" > 1', + havingLanguage: 'sql', + }), + ), + ).rejects.toThrow(); + }); + }); }); }); diff --git a/packages/common-utils/src/__tests__/renderChartConfig.test.ts b/packages/common-utils/src/__tests__/renderChartConfig.test.ts index 5ca57d18cb..466571eb9e 100644 --- a/packages/common-utils/src/__tests__/renderChartConfig.test.ts +++ b/packages/common-utils/src/__tests__/renderChartConfig.test.ts @@ -3966,6 +3966,168 @@ describe('renderChartConfig', () => { expect(sql.match(/SETTINGS/g)).toHaveLength(1); }); + // HAVING / ORDER BY / LIMIT apply to the final joined result, where the + // user-facing output columns exist — never inside a per-series branch. + describe('outer HAVING / ORDER BY / LIMIT', () => { + it('renders having, orderBy and limit once, on the outer joined statement only', async () => { + const generatedSql = await renderChartConfig( + { + ...baseMultiSeriesConfig, + displayType: DisplayType.Table, + granularity: undefined, + groupBy: [{ aggCondition: '', valueExpression: 'ServiceName' }], + having: '"avg(metric.alpha)" > 10', + havingLanguage: 'sql', + orderBy: [ + { valueExpression: '"avg(metric.beta)"', ordering: 'DESC' }, + ], + limit: { limit: 5, offset: 10 }, + }, + mockMetadata, + querySettings, + ); + const sql = parameterizedQueryToSql(generatedSql); + expect(sql).toMatchSnapshot(); + + // Exactly one of each user clause in the whole composed statement — + // i.e. none leaked into the UNION ALL branches. (Bare ORDER BY also + // appears inside the gauge translation's internal CTE scaffolding, + // so count the user's exact clause text, not the keyword.) + const count = (needle: string) => sql.split(needle).length - 1; + expect(count('HAVING "avg(metric.alpha)" > 10')).toBe(1); + expect(count('ORDER BY "avg(metric.beta)" DESC')).toBe(1); + expect(count('LIMIT 5 OFFSET 10')).toBe(1); + expect(count('HAVING')).toBe(1); + // And on the outer scope: after the join's GROUP BY ALL, in + // HAVING -> ORDER BY -> LIMIT order. + const groupByIdx = sql.lastIndexOf('GROUP BY ALL'); + const havingIdx = sql.indexOf('HAVING "avg(metric.alpha)" > 10'); + const orderByIdx = sql.indexOf('ORDER BY "avg(metric.beta)" DESC'); + expect(groupByIdx).toBeGreaterThan(-1); + expect(havingIdx).toBeGreaterThan(groupByIdx); + expect(orderByIdx).toBeGreaterThan(havingIdx); + expect(sql.indexOf('LIMIT 5 OFFSET 10')).toBeGreaterThan(orderByIdx); + }); + + it('keeps time charts bucket-ordered first, with the user sort as tiebreaker', async () => { + const generatedSql = await renderChartConfig( + { + ...baseMultiSeriesConfig, + groupBy: [{ aggCondition: '', valueExpression: 'ServiceName' }], + orderBy: [{ valueExpression: 'ServiceName', ordering: 'ASC' }], + }, + mockMetadata, + querySettings, + ); + const sql = parameterizedQueryToSql(generatedSql); + expect(sql).toContain('ORDER BY `__hdx_time_bucket`,ServiceName ASC'); + }); + + it('filters a share_of_total ratio through a wrapper, not HAVING (window function)', async () => { + const generatedSql = await renderChartConfig( + { + ...baseMultiSeriesConfig, + displayType: DisplayType.Table, + granularity: undefined, + groupBy: [{ aggCondition: '', valueExpression: 'ServiceName' }], + seriesReturnType: 'ratio', + ratioMode: 'share_of_total', + having: '"avg(metric.alpha)/avg(metric.beta)" >= 0.2', + havingLanguage: 'sql', + orderBy: [ + { + valueExpression: '"avg(metric.alpha)/avg(metric.beta)"', + ordering: 'DESC', + }, + ], + limit: { limit: 2 }, + }, + mockMetadata, + querySettings, + ); + const sql = parameterizedQueryToSql(generatedSql); + expect(sql).toMatchSnapshot(); + + // The share_of_total projection is a window function, which + // ClickHouse rejects inside HAVING — the filter runs as WHERE on a + // wrapper around the joined result instead. + expect(sql).not.toContain('HAVING'); + const whereIdx = sql.indexOf( + 'WHERE "avg(metric.alpha)/avg(metric.beta)" >= 0.2', + ); + expect(whereIdx).toBeGreaterThan(sql.lastIndexOf('GROUP BY ALL')); + // Filter, then order, then limit — on the outermost statement. + const orderByIdx = sql.indexOf( + 'ORDER BY "avg(metric.alpha)/avg(metric.beta)" DESC', + ); + expect(orderByIdx).toBeGreaterThan(whereIdx); + expect(sql.indexOf('LIMIT 2')).toBeGreaterThan(orderByIdx); + }); + + it('renders a string-form orderBy on the outer statement', async () => { + // SortSpecificationList also accepts a raw SQL string (saved configs + // may carry one), not just the structured array form. + const generatedSql = await renderChartConfig( + { + ...baseMultiSeriesConfig, + displayType: DisplayType.Table, + granularity: undefined, + groupBy: [{ aggCondition: '', valueExpression: 'ServiceName' }], + orderBy: '"avg(metric.alpha)" DESC', + }, + mockMetadata, + querySettings, + ); + const sql = parameterizedQueryToSql(generatedSql); + const count = (needle: string) => sql.split(needle).length - 1; + expect(count('ORDER BY "avg(metric.alpha)" DESC')).toBe(1); + expect( + sql.indexOf('ORDER BY "avg(metric.alpha)" DESC'), + ).toBeGreaterThan(sql.lastIndexOf('GROUP BY ALL')); + }); + + it('renders HAVING on the number shape without a GROUP BY ALL', async () => { + // With no passthrough columns the outer query is one implicit global + // aggregation — HAVING is valid there without any GROUP BY. + const generatedSql = await renderChartConfig( + { + ...baseMultiSeriesConfig, + displayType: DisplayType.Number, + granularity: undefined, + having: '"avg(metric.alpha)" > 10', + havingLanguage: 'sql', + }, + mockMetadata, + querySettings, + ); + const sql = parameterizedQueryToSql(generatedSql); + expect(sql).not.toContain('GROUP BY ALL'); + const count = (needle: string) => sql.split(needle).length - 1; + expect(count('HAVING "avg(metric.alpha)" > 10')).toBe(1); + }); + + it('lets HAVING reference a formula output column', async () => { + const generatedSql = await renderChartConfig( + { + ...baseMultiSeriesConfig, + displayType: DisplayType.Table, + granularity: undefined, + groupBy: [{ aggCondition: '', valueExpression: 'ServiceName' }], + formulas: [{ expression: 'A / B', alias: 'err rate' }], + having: '"err rate" > 0.5', + havingLanguage: 'sql', + }, + mockMetadata, + querySettings, + ); + const sql = parameterizedQueryToSql(generatedSql); + expect(sql.match(/HAVING/g)).toHaveLength(1); + expect(sql.indexOf('HAVING "err rate" > 0.5')).toBeGreaterThan( + sql.lastIndexOf('GROUP BY ALL'), + ); + }); + }); + // Formula projection over the pivoted per-series columns. describe('formulas', () => { const pivot = (idx: number) => diff --git a/packages/common-utils/src/core/renderChartConfig.ts b/packages/common-utils/src/core/renderChartConfig.ts index f27240d9b3..a2cd42ec14 100644 --- a/packages/common-utils/src/core/renderChartConfig.ts +++ b/packages/common-utils/src/core/renderChartConfig.ts @@ -2378,12 +2378,18 @@ async function renderMultiSeriesMetricChartConfig( const branches = await Promise.all( select.map(async (s, splitIdx) => { // Formulas belong to the composed outer projection only — a branch - // carrying them would recurse back into this function. + // carrying them would recurse back into this function. HAVING, + // ORDER BY and LIMIT apply to the final joined result, not to each + // series independently, so they render on the outer statement. Only + // row-level filters (where/filters/aggCondition) stay per-branch. const branchConfig: ChartConfigWithOptDateRangeEx = { ...chartConfig, select: [{ ...s, alias: MULTI_SERIES_VALUE_ALIAS }], formulas: undefined, showOperandSeries: undefined, + having: undefined, + orderBy: undefined, + limit: undefined, }; const rendered = await renderChartConfig( branchConfig, @@ -2543,13 +2549,50 @@ async function renderMultiSeriesMetricChartConfig( const settings = mergeSettingsClauses(branches.map(b => b.settingsClause)); - return concatChSql(' ', [ + // HAVING / ORDER BY / LIMIT apply to the final joined result and + // reference its output columns (operand aliases, formula names, the ratio + // column, group passthroughs, the time bucket). HAVING is valid even + // without GROUP BY ALL (the no-group number-chart shape is an implicit + // global aggregation). + const having = await renderHaving(chartConfig, metadata); + + // Time charts stay bucket-ordered first, with the user's sort as a + // tiebreaker within each bucket. renderOrderBy is not reusable here: it + // re-renders the bucket expression over raw timestamp columns, which + // don't exist in this outer scope — only the fixed bucket alias does. + const orderBy = concatChSql( + ',', + hasGranularity ? chSql`\`${FIXED_TIME_BUCKET_EXPR_ALIAS}\`` : chSql``, + chartConfig.orderBy != null + ? renderSortSpecificationList(chartConfig.orderBy) + : [], + ); + + const limit = renderLimit(chartConfig); + + const core = concatChSql(' ', [ chSql`SELECT ${{ UNSAFE_RAW_SQL: projection.join(', ') }}`, chSql`FROM (${unionSql})`, hasPassthroughColumns ? chSql`GROUP BY ALL` : chSql``, - hasGranularity - ? chSql`ORDER BY \`${FIXED_TIME_BUCKET_EXPR_ALIAS}\`` - : chSql``, + ]); + + // The share_of_total ratio is the one projection built on a window + // function, which ClickHouse prohibits in HAVING — so filter it through + // a wrapper instead: WHERE on the wrapped result evaluates after the + // window, with the same filter-the-output-rows semantics. ORDER BY/LIMIT + // follow on the outermost statement either way. + const usesWindowProjection = + isRatio && chartConfig.ratioMode === 'share_of_total'; + const filtered = !having?.sql + ? core + : usesWindowProjection + ? chSql`SELECT * FROM (${core}) WHERE ${having}` + : concatChSql(' ', [core, chSql`HAVING ${having}`]); + + return concatChSql(' ', [ + filtered, + orderBy.sql ? chSql`ORDER BY ${orderBy}` : chSql``, + limit?.sql ? chSql`LIMIT ${limit}` : chSql``, settings !== '' ? chSql`SETTINGS ${{ UNSAFE_RAW_SQL: settings }}` : chSql``, ]); }