From a035820a91524aa56154ad8ab444818e571faac6 Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Mon, 10 Aug 2026 21:25:56 -0700 Subject: [PATCH 1/2] feat: move multi-series metric merge computation to ClickHouse (HDX-5077) A metric chart with N select items used to fan out into N ClickHouse queries whose result sets were merged node/browser-side (mergeResultSets, computeResultSetRatio). renderChartConfig now composes the same N per-series branch queries into ONE statement: each branch is rendered with internal aliases (__hdx_value, __hdx_group_{j}), tagged with its series index, combined via UNION ALL, and pivoted back into one row per (group values, time bucket) with anyOrNullIf so missing rows stay NULL (a gap) instead of 0. Ratio charts (seriesReturnType 'ratio') and both ratioMode variants now divide in SQL (coalesce/nullif; share_of_total via a window sum over the bucket), resolving the long-standing TODOs to generate union CTEs and compute the ratio on the db side. - queryChartConfig renders once and queries once for every config; mergeResultSets, computeResultSetRatio, computeRatio and their helpers are deleted along with their unit suites (behavior is pinned by the HDX-5076 queryChartConfig integration baseline, which passes unchanged) - gauge/sum branches project group-by dimensions as individual columns while histogram branches keep their Array "group" column; each class pads the other's columns (NULL / []) so mixed-type charts stay type-compatible without joining rows across classes (parity) - same-alias series keep the __{splitIndex} suffix disambiguation, and the suffix is still stripped from the ratio column label - per-branch SETTINGS clauses are stripped and hoisted (deduped) to the composed query - sum 'increase' TopGroups ranking now strips group-by aliases (latent bug: an aliased groupBy rendered AS "alias" inside tuple(...), a syntax error; also required by the injected internal aliases) - "View SQL" for multi-series metric charts now shows the full composed query instead of only the first series --- .changeset/metrics-ch-side-merge.md | 6 + .../ChartEditorControls.tsx | 12 +- packages/cli/AGENTS.md | 4 +- packages/cli/src/shared/tileQuery.ts | 4 +- .../renderChartConfig.test.ts.snap | 245 ++++++ .../src/__tests__/clickhouse.test.ts | 696 ------------------ .../__tests__/queryChartConfig.int.test.ts | 10 +- .../src/__tests__/renderChartConfig.test.ts | 174 +++++ packages/common-utils/src/clickhouse/index.ts | 308 +------- .../common-utils/src/core/builderToRawSql.ts | 7 +- .../src/core/renderChartConfig.ts | 349 ++++++++- 11 files changed, 777 insertions(+), 1038 deletions(-) create mode 100644 .changeset/metrics-ch-side-merge.md diff --git a/.changeset/metrics-ch-side-merge.md b/.changeset/metrics-ch-side-merge.md new file mode 100644 index 0000000000..bc8bec004a --- /dev/null +++ b/.changeset/metrics-ch-side-merge.md @@ -0,0 +1,6 @@ +--- +'@hyperdx/common-utils': patch +'@hyperdx/app': patch +--- + +Multi-series metric charts now run as a single composed ClickHouse query instead of one query per series joined client-side. The per-series queries are combined via UNION ALL and pivoted back into one row per (group, time bucket) in SQL, including ratio charts (`seriesReturnType: 'ratio'`) and both `ratioMode` variants, which previously divided the two result sets in the browser/node. Result shape, column naming (including same-alias `__{index}` disambiguation), gap semantics, and ratio semantics are unchanged; charts with many series render with fewer round trips, and "View SQL" for multi-series metric charts now shows the full query instead of only the first series. diff --git a/packages/app/src/components/DBEditTimeChartForm/ChartEditorControls.tsx b/packages/app/src/components/DBEditTimeChartForm/ChartEditorControls.tsx index 14cc6dcd28..6ac77250c9 100644 --- a/packages/app/src/components/DBEditTimeChartForm/ChartEditorControls.tsx +++ b/packages/app/src/components/DBEditTimeChartForm/ChartEditorControls.tsx @@ -344,12 +344,12 @@ export function ChartEditorControls({ )} {/* Grouped ratios divide per-group by default; this opts into share-of-total (each group's contribution to the blended - rate). Only metric sources fan out to per-series queries - merged client-side (see mergeResultSets/ratioMode) — other - sources compute the ratio within-group in the DB, where - ratioMode has no effect — so restrict to metric sources. No - effect on ungrouped ratios either, so also gate on a Group - By. */} + rate). Only metric sources render the ratio over composed + per-series branches where ratioMode applies (see + renderMultiSeriesMetricChartConfig) — other sources compute + the ratio within-group via divide(), where ratioMode has no + effect — so restrict to metric sources. No effect on + ungrouped ratios either, so also gate on a Group By. */} {fields.length === 2 && seriesReturnType === 'ratio' && tableSource?.kind === SourceKind.Metric && diff --git a/packages/cli/AGENTS.md b/packages/cli/AGENTS.md index 96eacfc7a2..c11cfa773d 100644 --- a/packages/cli/AGENTS.md +++ b/packages/cli/AGENTS.md @@ -147,8 +147,8 @@ dashboard**: (`convertToTimeChartConfig` / `convertToNumberChartConfig` / `convertToTableChartConfig` / `convertToCategoricalChartConfig`). 2. `shared/tileQuery.ts` executes it via `clickhouseClient.queryChartConfig()` - from common-utils — internally - `setChartSelectsAlias → splitChartConfigs → renderChartConfig`, identical to + from common-utils — internally `setChartSelectsAlias → renderChartConfig` + (one composed statement, even for multi-series metric charts), identical to the web's `useQueriedChartConfig` core (minus chunking / MV optimization / PromQL). 3. `shared/chartData.ts` shapes the response (ports of diff --git a/packages/cli/src/shared/tileQuery.ts b/packages/cli/src/shared/tileQuery.ts index 794209fa5b..80b90de471 100644 --- a/packages/cli/src/shared/tileQuery.ts +++ b/packages/cli/src/shared/tileQuery.ts @@ -96,8 +96,8 @@ export async function fetchTileData({ }); // Same call the web frontend's useQueriedChartConfig makes — - // internally: setChartSelectsAlias → splitChartConfigs → - // renderChartConfig → query → join metric result sets. + // internally: setChartSelectsAlias → renderChartConfig → query (one + // composed statement, even for multi-series metric charts). const data = await clickhouseClient.queryChartConfig({ config: queriedConfig, metadata, 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 e1612ad24c..b7f39821c1 100644 --- a/packages/common-utils/src/__tests__/__snapshots__/renderChartConfig.test.ts.snap +++ b/packages/common-utils/src/__tests__/__snapshots__/renderChartConfig.test.ts.snap @@ -733,6 +733,251 @@ exports[`renderChartConfig k8s semantic convention migrations should handle metr ),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(1765670400000)) 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\` LIMIT 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) composes grouped multi-series gauges into one UNION ALL + pivot query 1`] = ` +"SELECT anyOrNullIf(\`__hdx_value\`, \`__hdx_series_idx\` = 0) AS "avg(metric.alpha)", anyOrNullIf(\`__hdx_value\`, \`__hdx_series_idx\` = 1) AS "avg(metric.beta)", \`__hdx_group_0\` AS "ServiceName", \`__hdx_time_bucket\` FROM (SELECT \`__hdx_value\`, \`__hdx_group_0\`, \`__hdx_time_bucket\`, 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 minute) 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 AS "__hdx_group_0",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 ServiceName AS "__hdx_group_0",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\`) UNION ALL SELECT \`__hdx_value\`, \`__hdx_group_0\`, \`__hdx_time_bucket\`, 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 minute) 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 AS "__hdx_group_0",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 ServiceName AS "__hdx_group_0",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 \`__hdx_group_0\`, \`__hdx_time_bucket\` 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) 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)", \`__hdx_group_0\` AS "ServiceName", \`group\`, \`__hdx_time_bucket\` FROM (SELECT \`__hdx_value\`, \`__hdx_group_0\`, [] AS \`group\`, \`__hdx_time_bucket\`, 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 minute) 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 AS "__hdx_group_0",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 ServiceName AS "__hdx_group_0",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\`) UNION ALL SELECT \`__hdx_value\`, NULL AS \`__hdx_group_0\`, \`group\`, \`__hdx_time_bucket\`, 1 AS \`__hdx_series_idx\` FROM (WITH source AS ( + SELECT + MetricName, + ExplicitBounds, + toStartOfInterval(toDateTime(TimeUnix), INTERVAL 1 minute) AS \`__hdx_time_bucket\`, + group, + sumForEach(deltas) as rates + FROM ( + SELECT + TimeUnix, + MetricName, + ExplicitBounds, + group, + attr_hash, + any(attr_hash) OVER prev_row AS prev_attr_hash, + count() OVER prev_row = 0 OR prev_attr_hash != attr_hash AS is_first_series_point, + any(bounds_hash) OVER prev_row AS prev_bounds_hash, + any(counts) OVER prev_row AS prev_counts, + counts, + multiIf( + AggregationTemporality = 2 AND is_first_series_point, + arrayWithConstant(length(counts), toInt64(0)), + + AggregationTemporality = 1 + OR bounds_hash != prev_bounds_hash + OR arrayExists((x) -> x.2 < x.1, arrayZip(prev_counts, counts)), + counts, + + counts - prev_counts + ) AS deltas + FROM ( + SELECT + TimeUnix, + MetricName, + AggregationTemporality, + ExplicitBounds, + ResourceAttributes, + Attributes,[ServiceName] as group, + cityHash64(ScopeAttributes, ResourceAttributes, Attributes) AS attr_hash, + cityHash64(ExplicitBounds) AS bounds_hash, + CAST(BucketCounts AS Array(Int64)) counts + FROM default.otel_metrics_histogram + WHERE (TimeUnix >= toStartOfInterval(fromUnixTimestamp64Milli(1739318400000), INTERVAL 1 minute) - INTERVAL 1 minute AND TimeUnix <= toStartOfInterval(fromUnixTimestamp64Milli(1739491200000), INTERVAL 1 minute) + INTERVAL 1 minute) AND ((MetricName = 'metric.latency')) + ORDER BY group, attr_hash, TimeUnix ASC + ) + WINDOW prev_row AS ( + PARTITION BY group + ORDER BY attr_hash, TimeUnix + ROWS BETWEEN 1 PRECEDING AND 1 PRECEDING + ) + ) + GROUP BY __hdx_time_bucket, MetricName, group, ExplicitBounds + ORDER BY __hdx_time_bucket + ),points AS ( + SELECT + \`__hdx_time_bucket\`, +group, + MetricName, + arrayZipUnaligned(arrayCumSum(rates), ExplicitBounds) as point, + length(point) as n + FROM source + ),metrics AS ( + SELECT + \`__hdx_time_bucket\`, +group, + MetricName, + point[n].1 AS total, + 0.5 * total AS rank, + arrayFirstIndex(x -> if(x.1 > rank, 1, 0), point) AS upper_idx, + point[upper_idx].1 AS upper_count, + ifNull(point[upper_idx].2, inf) AS upper_bound, + CASE + WHEN upper_idx > 1 THEN point[upper_idx - 1].2 + WHEN point[upper_idx].2 > 0 THEN 0 + ELSE inf + END AS lower_bound, + if ( + lower_bound = 0, + 0, + point[upper_idx - 1].1 + ) AS lower_count, + CASE + WHEN upper_bound = inf THEN point[upper_idx - 1].2 + WHEN lower_bound = inf THEN point[1].2 + ELSE lower_bound + (upper_bound - lower_bound) * ((rank - lower_count) / (upper_count - lower_count)) + END AS "__hdx_value" + FROM points + WHERE length(point) > 1 AND total > 0 + ) SELECT \`__hdx_time_bucket\`, group, "__hdx_value" FROM metrics WHERE (\`__hdx_time_bucket\` >= fromUnixTimestamp64Milli(1739318400000) AND \`__hdx_time_bucket\` <= fromUnixTimestamp64Milli(1739491200000)))) GROUP BY \`__hdx_group_0\`, \`group\`, \`__hdx_time_bucket\` 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) renders a metric ratio as a SQL-side division 1`] = ` +"SELECT coalesce(anyOrNullIf(\`__hdx_value\`, \`__hdx_series_idx\` = 0), 0) / nullif(anyOrNullIf(\`__hdx_value\`, \`__hdx_series_idx\` = 1), 0) AS "avg(metric.alpha)/avg(metric.beta)", \`__hdx_time_bucket\` FROM (SELECT \`__hdx_value\`, \`__hdx_time_bucket\`, 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 minute) 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",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\`) UNION ALL SELECT \`__hdx_value\`, \`__hdx_time_bucket\`, 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 minute) 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",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 \`__hdx_time_bucket\` 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 sample-weighted aggregations should handle complex sampleWeightExpression like SpanAttributes map access 1`] = `"SELECT sum(greatest(toUInt64OrZero(toString(SpanAttributes['SampleRate'])), 1)) FROM default.otel_traces WHERE (Timestamp >= fromUnixTimestamp64Milli(1739318400000) AND Timestamp <= fromUnixTimestamp64Milli(1739491200000)) SETTINGS 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 sample-weighted aggregations should handle mixed weighted and passthrough aggregations 1`] = ` diff --git a/packages/common-utils/src/__tests__/clickhouse.test.ts b/packages/common-utils/src/__tests__/clickhouse.test.ts index dcec597f09..8d3198fca4 100644 --- a/packages/common-utils/src/__tests__/clickhouse.test.ts +++ b/packages/common-utils/src/__tests__/clickhouse.test.ts @@ -1,17 +1,11 @@ -import { ResponseJSON } from '@clickhouse/client'; - import { ChSql, chSqlToAliasMap, - computeRatio, - computeResultSetRatio, convertCHDataTypeToJSType, JSDataType, - mergeResultSets, } from '@/clickhouse'; import { ClickhouseClient } from '@/clickhouse/node'; import { Metadata, MetadataCache } from '@/core/metadata'; -import { FIXED_TIME_BUCKET_EXPR_ALIAS } from '@/core/renderChartConfig'; describe('convertCHDataTypeToJSType - unit - type', () => { it('Date type', () => { @@ -312,696 +306,6 @@ describe('chSqlToAliasMap - resilient parsing of ClickHouse-specific SQL', () => }); }); -describe('computeRatio', () => { - it('should correctly compute ratio of two numbers', () => { - expect(computeRatio('10', '2')).toBe(5); - expect(computeRatio('3', '4')).toBe(0.75); - expect(computeRatio('0', '5')).toBe(0); - }); - - it('should return NaN when denominator is zero', () => { - expect(isNaN(computeRatio('10', '0'))).toBe(true); - }); - - it('should return NaN for non-numeric inputs', () => { - expect(isNaN(computeRatio('abc', '2'))).toBe(true); - expect(isNaN(computeRatio('10', 'xyz'))).toBe(true); - expect(isNaN(computeRatio('abc', 'xyz'))).toBe(true); - expect(isNaN(computeRatio('', '5'))).toBe(true); - }); - - it('should handle string representations of numbers', () => { - expect(computeRatio('10.5', '2')).toBe(5.25); - expect(computeRatio('-10', '5')).toBe(-2); - expect(computeRatio('10', '-5')).toBe(-2); - }); - - it('should handle number input types', () => { - expect(computeRatio(10, 2)).toBe(5); - expect(computeRatio(3, 4)).toBe(0.75); - expect(computeRatio(10.5, 2)).toBe(5.25); - expect(computeRatio(0, 5)).toBe(0); - expect(isNaN(computeRatio(10, 0))).toBe(true); - expect(computeRatio(-10, 5)).toBe(-2); - }); - - it('should handle mixed string and number inputs', () => { - expect(computeRatio('10', 2)).toBe(5); - expect(computeRatio(10, '2')).toBe(5); - expect(computeRatio(3, '4')).toBe(0.75); - expect(isNaN(computeRatio(10, ''))).toBe(true); - }); -}); - -describe('computeResultSetRatio', () => { - it('should compute ratio for a valid result set with timestamp column', () => { - const mockResultSet: ResponseJSON = { - meta: [ - { name: 'timestamp', type: 'DateTime' }, - { name: 'requests', type: 'UInt64' }, - { name: 'errors', type: 'UInt64' }, - ], - data: [ - { timestamp: '2025-04-15 10:00:00', requests: '100', errors: '10' }, - { timestamp: '2025-04-15 11:00:00', requests: '200', errors: '20' }, - ], - rows: 2, - statistics: { elapsed: 0.1, rows_read: 2, bytes_read: 100 }, - }; - - const result = computeResultSetRatio(mockResultSet, { - numeratorName: 'requests', - denominatorName: 'errors', - }); - - expect(result.meta.length).toBe(2); - expect(result.meta[0].name).toBe('requests/errors'); - expect(result.meta[0].type).toBe('Float64'); - expect(result.meta[1].name).toBe('timestamp'); - - expect(result.data.length).toBe(2); - expect(result.data[0]['requests/errors']).toBe(10); - expect(result.data[0].timestamp).toBe('2025-04-15 10:00:00'); - expect(result.data[1]['requests/errors']).toBe(10); - expect(result.data[1].timestamp).toBe('2025-04-15 11:00:00'); - }); - - it('should compute ratio for a valid result set without timestamp column', () => { - const mockResultSet: ResponseJSON = { - meta: [ - { name: 'requests', type: 'UInt64' }, - { name: 'errors', type: 'UInt64' }, - ], - data: [{ requests: '100', errors: '10' }], - rows: 1, - statistics: { elapsed: 0.1, rows_read: 1, bytes_read: 50 }, - }; - - const result = computeResultSetRatio(mockResultSet, { - numeratorName: 'requests', - denominatorName: 'errors', - }); - - expect(result.meta.length).toBe(1); - expect(result.meta[0].name).toBe('requests/errors'); - expect(result.meta[0].type).toBe('Float64'); - - expect(result.data.length).toBe(1); - expect(result.data[0]['requests/errors']).toBe(10); - expect(result.data[0].timestamp).toBeUndefined(); - }); - - it('should handle NaN values in ratio computation', () => { - const mockResultSet: ResponseJSON = { - meta: [ - { name: 'timestamp', type: 'DateTime' }, - { name: 'requests', type: 'UInt64' }, - { name: 'errors', type: 'UInt64' }, - ], - data: [ - { timestamp: '2025-04-15 10:00:00', requests: '100', errors: '0' }, - { timestamp: '2025-04-15 11:00:00', requests: 'invalid', errors: '20' }, - ], - rows: 2, - statistics: { elapsed: 0.1, rows_read: 2, bytes_read: 100 }, - }; - - const result = computeResultSetRatio(mockResultSet, { - numeratorName: 'requests', - denominatorName: 'errors', - }); - - expect(result.data.length).toBe(2); - expect(isNaN(result.data[0]['requests/errors'])).toBe(true); - expect(isNaN(result.data[1]['requests/errors'])).toBe(true); - }); - - it('should throw error when result set has insufficient columns', () => { - const mockResultSet: ResponseJSON = { - meta: [ - { name: 'timestamp', type: 'DateTime' }, - { name: 'requests', type: 'UInt64' }, - ], - data: [{ timestamp: '2025-04-15 10:00:00', requests: '100' }], - rows: 1, - statistics: { elapsed: 0.1, rows_read: 1, bytes_read: 50 }, - }; - - // Denominator operand does not exist in the meta -> can't compute. - expect(() => - computeResultSetRatio(mockResultSet, { - numeratorName: 'requests', - denominatorName: 'errors', - }), - ).toThrow(/Unable to compute ratio/); - }); - - it('computes a grouped ratio as each group own rate by default (per_group)', () => { - // Joined meta seeds the two value columns first, then the group + timestamp - // columns (mirrors queryChartConfig's merge output). - const mockResultSet: ResponseJSON = { - meta: [ - { name: 'errors', type: 'UInt64' }, - { name: 'total', type: 'UInt64' }, - { name: 'tenant', type: 'String' }, - { name: 'timestamp', type: 'DateTime' }, - ], - data: [ - { errors: '20', total: '100', tenant: 'acme', timestamp: 't0' }, - { errors: '30', total: '100', tenant: 'globex', timestamp: 't0' }, - { errors: '10', total: '100', tenant: 'acme', timestamp: 't1' }, - ], - rows: 3, - statistics: { elapsed: 0.1, rows_read: 3, bytes_read: 100 }, - }; - - const result = computeResultSetRatio(mockResultSet, { - numeratorName: 'errors', - denominatorName: 'total', - }); - - // ratio column + carried-through group + timestamp - expect(result.meta.map(m => m.name)).toEqual([ - 'errors/total', - 'tenant', - 'timestamp', - ]); - // Each group divided by its own denominator (per-group rate), not a shared - // bucket total. - expect(result.data).toEqual([ - { 'errors/total': 0.2, tenant: 'acme', timestamp: 't0' }, // 20/100 - { 'errors/total': 0.3, tenant: 'globex', timestamp: 't0' }, // 30/100 - { 'errors/total': 0.1, tenant: 'acme', timestamp: 't1' }, // 10/100 - ]); - }); - - it('computes a grouped ratio as each group share of the per-bucket total in share_of_total mode (lines sum to the overall)', () => { - const mockResultSet: ResponseJSON = { - meta: [ - { name: 'errors', type: 'UInt64' }, - { name: 'total', type: 'UInt64' }, - { name: 'tenant', type: 'String' }, - { name: 'timestamp', type: 'DateTime' }, - ], - data: [ - { errors: '20', total: '100', tenant: 'acme', timestamp: 't0' }, - { errors: '30', total: '100', tenant: 'globex', timestamp: 't0' }, - { errors: '10', total: '100', tenant: 'acme', timestamp: 't1' }, - ], - rows: 3, - statistics: { elapsed: 0.1, rows_read: 3, bytes_read: 100 }, - }; - - const result = computeResultSetRatio( - mockResultSet, - { numeratorName: 'errors', denominatorName: 'total' }, - 'share_of_total', - ); - - // Denominator is the per-bucket total across all groups (t0: 200, t1: 100), - // so each group is its contribution to the overall rate. - expect(result.data).toEqual([ - { 'errors/total': 0.1, tenant: 'acme', timestamp: 't0' }, // 20/200 - { 'errors/total': 0.15, tenant: 'globex', timestamp: 't0' }, // 30/200 - { 'errors/total': 0.1, tenant: 'acme', timestamp: 't1' }, // 10/100 - ]); - // t0 contributions sum to the overall error rate for that bucket (50/200). - const t0 = result.data.filter(r => r.timestamp === 't0'); - expect(t0.reduce((s, r) => s + r['errors/total'], 0)).toBeCloseTo(0.25); - }); - - it('renders a zero-error group as 0% (missing numerator), not N/A', () => { - const mockResultSet: ResponseJSON = { - meta: [ - { name: 'errors', type: 'UInt64' }, - { name: 'total', type: 'UInt64' }, - { name: 'tenant', type: 'String' }, - { name: 'timestamp', type: 'DateTime' }, - ], - data: [ - // has errors -> contributes 20/200 - { errors: '20', total: '100', tenant: 'acme', timestamp: 't0' }, - // denominator only (no rows in the error-filtered numerator query) -> 0% - { total: '100', tenant: 'globex', timestamp: 't0' }, - ], - rows: 2, - statistics: { elapsed: 0.1, rows_read: 2, bytes_read: 100 }, - }; - - const result = computeResultSetRatio( - mockResultSet, - { numeratorName: 'errors', denominatorName: 'total' }, - 'share_of_total', - ); - - expect(result.data[0]['errors/total']).toBe(0.1); // 20 / (100+100) - expect(result.data[1]['errors/total']).toBe(0); // 0 / 200 - }); - - it('does not let a group missing the denominator poison the bucket total', () => { - const mockResultSet: ResponseJSON = { - meta: [ - { name: 'errors', type: 'UInt64' }, - { name: 'total', type: 'UInt64' }, - { name: 'tenant', type: 'String' }, - { name: 'timestamp', type: 'DateTime' }, - ], - data: [ - // present in both numerator and denominator - { errors: '20', total: '100', tenant: 'acme', timestamp: 't0' }, - // numerator only (no matching denominator row) -> total is undefined. - // Must not turn the bucket total into NaN for the other groups. - { errors: '5', tenant: 'globex', timestamp: 't0' }, - ], - rows: 2, - statistics: { elapsed: 0.1, rows_read: 2, bytes_read: 100 }, - }; - - const result = computeResultSetRatio( - mockResultSet, - { numeratorName: 'errors', denominatorName: 'total' }, - 'share_of_total', - ); - - // Bucket total is 100 (only acme contributes a denominator), so acme's - // share is still well-defined rather than NaN. - expect(result.data[0]['errors/total']).toBe(0.2); // 20 / 100 - }); - - it('divides each group by the grand total in share_of_total mode when there is no timestamp column (Table/Number ratio)', () => { - // No timestamp column -> every row shares the '__all__' bucket, so a grouped - // non-time-series ratio in share_of_total mode is each group's share of the - // grand total across all groups. This locks that intended semantic. - const mockResultSet: ResponseJSON = { - meta: [ - { name: 'errors', type: 'UInt64' }, - { name: 'total', type: 'UInt64' }, - { name: 'tenant', type: 'String' }, - ], - data: [ - { errors: '20', total: '100', tenant: 'acme' }, - { errors: '30', total: '300', tenant: 'globex' }, - ], - rows: 2, - statistics: { elapsed: 0.1, rows_read: 2, bytes_read: 100 }, - }; - - const result = computeResultSetRatio( - mockResultSet, - { numeratorName: 'errors', denominatorName: 'total' }, - 'share_of_total', - ); - - // Grand total = 100 + 300 = 400, so each group is divided by 400. - expect(result.data).toEqual([ - { 'errors/total': 0.05, tenant: 'acme' }, // 20/400 - { 'errors/total': 0.075, tenant: 'globex' }, // 30/400 - ]); - }); - - it('divides each group by its own denominator in per_group mode (default)', () => { - const mockResultSet: ResponseJSON = { - meta: [ - { name: 'errors', type: 'UInt64' }, - { name: 'total', type: 'UInt64' }, - { name: 'tenant', type: 'String' }, - ], - data: [ - { errors: '20', total: '100', tenant: 'acme' }, - { errors: '30', total: '300', tenant: 'globex' }, - ], - rows: 2, - statistics: { elapsed: 0.1, rows_read: 2, bytes_read: 100 }, - }; - - const result = computeResultSetRatio(mockResultSet, { - numeratorName: 'errors', - denominatorName: 'total', - }); - - expect(result.data).toEqual([ - { 'errors/total': 0.2, tenant: 'acme' }, // 20/100 - { 'errors/total': 0.1, tenant: 'globex' }, // 30/300 - ]); - }); - - it('yields NaN in share_of_total mode when a bucket denominator total is zero', () => { - // Every group in the bucket has a zero denominator, so the bucket total is - // 0 and each share is a divide-by-zero -> NaN (computeRatio guards it). - const mockResultSet: ResponseJSON = { - meta: [ - { name: 'errors', type: 'UInt64' }, - { name: 'total', type: 'UInt64' }, - { name: 'tenant', type: 'String' }, - { name: 'timestamp', type: 'DateTime' }, - ], - data: [ - { errors: '5', total: '0', tenant: 'acme', timestamp: 't0' }, - { errors: '3', total: '0', tenant: 'globex', timestamp: 't0' }, - ], - rows: 2, - statistics: { elapsed: 0.1, rows_read: 2, bytes_read: 100 }, - }; - - const result = computeResultSetRatio( - mockResultSet, - { numeratorName: 'errors', denominatorName: 'total' }, - 'share_of_total', - ); - - expect(isNaN(result.data[0]['errors/total'])).toBe(true); - expect(isNaN(result.data[1]['errors/total'])).toBe(true); - }); - - it('yields NaN in per_group mode for a row missing its own denominator', () => { - // per_group divides each row by its own denominator; a row absent from the - // denominator split has undefined -> NaN, without poisoning other rows. - const mockResultSet: ResponseJSON = { - meta: [ - { name: 'errors', type: 'UInt64' }, - { name: 'total', type: 'UInt64' }, - { name: 'tenant', type: 'String' }, - { name: 'timestamp', type: 'DateTime' }, - ], - data: [ - { errors: '20', total: '100', tenant: 'acme', timestamp: 't0' }, - // numerator only, no denominator for this group - { errors: '5', tenant: 'globex', timestamp: 't0' }, - ], - rows: 2, - statistics: { elapsed: 0.1, rows_read: 2, bytes_read: 100 }, - }; - - const result = computeResultSetRatio(mockResultSet, { - numeratorName: 'errors', - denominatorName: 'total', - }); - - expect(result.data[0]['errors/total']).toBe(0.2); // 20/100 - expect(isNaN(result.data[1]['errors/total'])).toBe(true); // 5/undefined - }); - - it('buckets share_of_total by the real time bucket, not a Date-typed group-by dimension ordered ahead of it', () => { - // A DateTime group-by dimension (`day`) precedes the real time bucket in the - // meta. inferTimestampColumn would pick `day` (first Date-typed column) and - // sum the denominator over the wrong column; resolving by the query - // builder's FIXED_TIME_BUCKET_EXPR_ALIAS picks the true bucket instead. - const mockResultSet: ResponseJSON = { - meta: [ - { name: 'errors', type: 'UInt64' }, - { name: 'total', type: 'UInt64' }, - { name: 'day', type: 'Date' }, // group-by dimension, Date-typed - { name: FIXED_TIME_BUCKET_EXPR_ALIAS, type: 'DateTime' }, // real bucket - ], - data: [ - // Bucket t0 spans two distinct `day` values; the per-bucket total must - // be 100 + 300 = 400, not per-`day` (which the hijack would produce). - { - errors: '20', - total: '100', - day: '2025-04-15', - [FIXED_TIME_BUCKET_EXPR_ALIAS]: 't0', - }, - { - errors: '30', - total: '300', - day: '2025-04-16', - [FIXED_TIME_BUCKET_EXPR_ALIAS]: 't0', - }, - ], - rows: 2, - statistics: { elapsed: 0.1, rows_read: 2, bytes_read: 100 }, - }; - - const result = computeResultSetRatio( - mockResultSet, - { numeratorName: 'errors', denominatorName: 'total' }, - 'share_of_total', - ); - - // Divided by the true bucket total (400), not by each row's own `day` total. - expect(result.data[0]['errors/total']).toBe(0.05); // 20/400 - expect(result.data[1]['errors/total']).toBe(0.075); // 30/400 - }); -}); - -describe('mergeResultSets', () => { - // Two split queries (one per series) that share a group-by dimension and are - // NOT time-series (Table display, no timestamp column). - const groupedSplits = (): ResponseJSON[] => [ - { - meta: [ - { name: 'errors', type: 'UInt64' }, - { name: 'tenant', type: 'String' }, - ], - data: [ - { errors: '20', tenant: 'acme' }, - { errors: '30', tenant: 'globex' }, - ], - rows: 2, - statistics: { elapsed: 0.1, rows_read: 2, bytes_read: 100 }, - }, - { - meta: [ - { name: 'total', type: 'UInt64' }, - { name: 'tenant', type: 'String' }, - ], - data: [ - { total: '100', tenant: 'acme' }, - { total: '200', tenant: 'globex' }, - ], - rows: 2, - statistics: { elapsed: 0.1, rows_read: 2, bytes_read: 100 }, - }, - ]; - - it('merges the numerator and denominator rows for the same group into one row instead of clobbering', () => { - const merged = mergeResultSets({ - resultSets: groupedSplits(), - isTimeSeries: false, - isRatio: false, - }); - - // Each group keeps BOTH operands (the second split does not overwrite the - // first) — this is the merge step the grouped-ratio fix depends on. - expect(merged.meta?.map(m => m.name)).toEqual([ - 'errors', - 'total', - 'tenant', - ]); - expect(merged.data).toEqual([ - { errors: '20', total: '100', tenant: 'acme' }, - { errors: '30', total: '200', tenant: 'globex' }, - ]); - }); - - it('computes the grouped ratio end-to-end from the split result sets (per-group default)', () => { - const merged = mergeResultSets({ - resultSets: groupedSplits(), - isTimeSeries: false, - isRatio: true, - }); - - // Default per_group -> each group divided by its own denominator. - expect(merged.data).toEqual([ - { 'errors/total': 20 / 100, tenant: 'acme' }, - { 'errors/total': 30 / 200, tenant: 'globex' }, - ]); - }); - - it('passes ratioMode through to computeResultSetRatio (share_of_total)', () => { - const merged = mergeResultSets({ - resultSets: groupedSplits(), - isTimeSeries: false, - isRatio: true, - ratioMode: 'share_of_total', - }); - - // No timestamp -> share of the grand total (300): 20/300, 30/300. - expect(merged.data).toEqual([ - { 'errors/total': 20 / 300, tenant: 'acme' }, - { 'errors/total': 30 / 300, tenant: 'globex' }, - ]); - }); - - it('keeps grouped time-series rows distinct when computing a ratio (timestamp + group column)', () => { - // Both a real timestamp and a group dimension: rows at the same bucket but - // different groups must not collapse, and each group keeps its own operands. - const resultSets: ResponseJSON[] = [ - { - meta: [ - { name: 'errors', type: 'UInt64' }, - { name: 'tenant', type: 'String' }, - { name: 'timestamp', type: 'DateTime' }, - ], - data: [ - { errors: '20', tenant: 'acme', timestamp: 't0' }, - { errors: '30', tenant: 'globex', timestamp: 't0' }, - ], - rows: 2, - statistics: { elapsed: 0.1, rows_read: 2, bytes_read: 100 }, - }, - { - meta: [ - { name: 'total', type: 'UInt64' }, - { name: 'tenant', type: 'String' }, - { name: 'timestamp', type: 'DateTime' }, - ], - data: [ - { total: '100', tenant: 'acme', timestamp: 't0' }, - { total: '300', tenant: 'globex', timestamp: 't0' }, - ], - rows: 2, - statistics: { elapsed: 0.1, rows_read: 2, bytes_read: 100 }, - }, - ]; - - const merged = mergeResultSets({ - resultSets, - isTimeSeries: true, - isRatio: true, - }); - - // Two distinct groups in the same bucket, each divided by its own total. - expect(merged.data).toEqual([ - { 'errors/total': 0.2, tenant: 'acme', timestamp: 't0' }, // 20/100 - { 'errors/total': 0.1, tenant: 'globex', timestamp: 't0' }, // 30/300 - ]); - }); - - it('keeps operands distinct when both splits resolve to the same value-column alias', () => { - // A ratio of count(request) filtered / unfiltered: the alias omits the WHERE - // filter, so both splits emit a `count(request)` column. They must not - // collapse into one column (which would make the ratio uncomputable). - const resultSets: ResponseJSON[] = [ - { - meta: [ - { name: 'count(request)', type: 'UInt64' }, - { name: 'timestamp', type: 'DateTime' }, - ], - data: [{ 'count(request)': '5', timestamp: 't0' }], - rows: 1, - statistics: { elapsed: 0.1, rows_read: 1, bytes_read: 100 }, - }, - { - meta: [ - { name: 'count(request)', type: 'UInt64' }, - { name: 'timestamp', type: 'DateTime' }, - ], - data: [{ 'count(request)': '20', timestamp: 't0' }], - rows: 1, - statistics: { elapsed: 0.1, rows_read: 1, bytes_read: 100 }, - }, - ]; - - const merged = mergeResultSets({ - resultSets, - isTimeSeries: true, - isRatio: true, - }); - - // 5 / 20 — would throw "Unable to compute ratio" if the two operands - // collapsed into a single column. The label strips the disambiguation - // suffix so it reads count(request)/count(request). - expect(merged.data).toEqual([ - { 'count(request)/count(request)': 0.25, timestamp: 't0' }, - ]); - }); - - it('keeps colliding value columns distinct across splits that also carry a group dimension', () => { - // Same-alias operands (count(request) filtered / unfiltered) AND a group-by - // dimension carried through both splits. The colliding value column is - // renamed per split index while the group dimension merges, so each group - // keeps both operands and divides correctly. - const resultSets: ResponseJSON[] = [ - { - meta: [ - { name: 'count(request)', type: 'UInt64' }, - { name: 'tenant', type: 'String' }, - { name: 'timestamp', type: 'DateTime' }, - ], - data: [ - { 'count(request)': '5', tenant: 'acme', timestamp: 't0' }, - { 'count(request)': '9', tenant: 'globex', timestamp: 't0' }, - ], - rows: 2, - statistics: { elapsed: 0.1, rows_read: 2, bytes_read: 100 }, - }, - { - meta: [ - { name: 'count(request)', type: 'UInt64' }, - { name: 'tenant', type: 'String' }, - { name: 'timestamp', type: 'DateTime' }, - ], - data: [ - { 'count(request)': '20', tenant: 'acme', timestamp: 't0' }, - { 'count(request)': '30', tenant: 'globex', timestamp: 't0' }, - ], - rows: 2, - statistics: { elapsed: 0.1, rows_read: 2, bytes_read: 100 }, - }, - ]; - - const merged = mergeResultSets({ - resultSets, - isTimeSeries: true, - isRatio: true, - }); - - expect(merged.data).toEqual([ - { - 'count(request)/count(request)': 0.25, - tenant: 'acme', - timestamp: 't0', - }, // 5/20 - { - 'count(request)/count(request)': 0.3, - tenant: 'globex', - timestamp: 't0', - }, // 9/30 - ]); - }); - - it('returns the merged rows undivided when a ratio split has no numeric value column', () => { - // If one split yields no inferable numeric column, its operand name is ''. - // Reading positionally leaves denominatorName undefined; rather than throw - // "Unable to compute ratio" and fail the whole chart, the merge returns the - // rows undivided. - const resultSets: ResponseJSON[] = [ - { - meta: [ - { name: 'errors', type: 'UInt64' }, - { name: 'timestamp', type: 'DateTime' }, - ], - data: [{ errors: '20', timestamp: 't0' }], - rows: 1, - statistics: { elapsed: 0.1, rows_read: 1, bytes_read: 100 }, - }, - { - // No numeric column at all (e.g. an empty/failed split projection). - meta: [{ name: 'timestamp', type: 'DateTime' }], - data: [{ timestamp: 't0' }], - rows: 1, - statistics: { elapsed: 0.1, rows_read: 1, bytes_read: 100 }, - }, - ]; - - expect(() => - mergeResultSets({ resultSets, isTimeSeries: true, isRatio: true }), - ).not.toThrow(); - - const merged = mergeResultSets({ - resultSets, - isTimeSeries: true, - isRatio: true, - }); - - // No ratio column — the merged rows pass through undivided. - expect(merged.data).toEqual([{ errors: '20', timestamp: 't0' }]); - expect(merged.meta?.some(m => m.name.includes('/'))).toBe(false); - }); -}); - describe('processClickhouseSettings - optimization settings', () => { let client: ClickhouseClient; let mockQueryMethod: jest.Mock; diff --git a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts index 51195f3a72..0a04e7fdad 100644 --- a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts +++ b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts @@ -896,11 +896,11 @@ describe('queryChartConfig Integration Tests', () => { // Regression baseline for the multi-series metric merge (HDX-5076). // - // A metric chart with N select items fans out into N ClickHouse queries and - // the result sets are merged back together (today node-side via - // mergeResultSets; HDX-5077 moves the merge into a single composed ClickHouse - // query). These tests pin the OBSERVABLE CONTRACT of that merge end-to-end - // through queryChartConfig, so the reimplementation must reproduce it: + // A metric chart with N select items renders one per-series query per item + // and merges them back into one result set. These tests were written + // against the original node-side merge (mergeResultSets) and pin its + // OBSERVABLE CONTRACT end-to-end through queryChartConfig; the composed + // single-query implementation (HDX-5077) must — and does — reproduce it: // // - meta lists all value columns first, in select order (positional // contract of useChartNumberFormats); diff --git a/packages/common-utils/src/__tests__/renderChartConfig.test.ts b/packages/common-utils/src/__tests__/renderChartConfig.test.ts index 71af1a2aef..f4364a1e50 100644 --- a/packages/common-utils/src/__tests__/renderChartConfig.test.ts +++ b/packages/common-utils/src/__tests__/renderChartConfig.test.ts @@ -3593,4 +3593,178 @@ describe('renderChartConfig', () => { // attribute columns. Coverage of the variadic form lives in the regenerated // gauge / sum / histogram snapshots earlier in this file plus the // cross-scope integration test in packages/api/src/clickhouse/__tests__. + + // A metric chart with multiple select items renders one query per series and + // composes them into a single UNION ALL + pivot statement (HDX-5077). The + // end-to-end result-shape contract is pinned by the queryChartConfig + // integration tests; these snapshots pin the generated SQL structure. + describe('multi-series metric charts (composed query)', () => { + const metricTables = { + gauge: 'otel_metrics_gauge', + histogram: 'otel_metrics_histogram', + sum: 'otel_metrics_sum', + summary: 'otel_metrics_summary', + 'exponential histogram': 'otel_metrics_exponential_histogram', + }; + + const gaugeSelect = (metricName: string) => ({ + aggFn: 'avg' as const, + aggCondition: '', + aggConditionLanguage: 'sql' as const, + valueExpression: 'Value', + metricName, + metricType: MetricsDataType.Gauge, + }); + + const baseMultiSeriesConfig: ChartConfigWithOptDateRange = { + displayType: DisplayType.Line, + connection: 'test-connection', + metricTables, + from: { databaseName: 'default', tableName: '' }, + select: [gaugeSelect('metric.alpha'), gaugeSelect('metric.beta')], + where: '', + whereLanguage: 'sql', + timestampValueExpression: 'TimeUnix', + dateRange: [new Date('2025-02-12'), new Date('2025-02-14')], + granularity: '1 minute', + }; + + it('composes grouped multi-series gauges into one UNION ALL + pivot query', async () => { + const generatedSql = await renderChartConfig( + { + ...baseMultiSeriesConfig, + groupBy: [{ aggCondition: '', valueExpression: 'ServiceName' }], + }, + mockMetadata, + querySettings, + ); + const sql = parameterizedQueryToSql(generatedSql); + expect(sql).toMatchSnapshot(); + + // One composed statement: both branch SETTINGS hoisted to a single + // trailing clause, deduped. + expect(sql.match(/SETTINGS/g)).toHaveLength(1); + expect(sql.match(/optimize_read_in_order = 0/g)).toHaveLength(1); + // Value columns pivot under the user-facing aliases, NULL for gaps. + expect(sql).toContain( + 'anyOrNullIf(`__hdx_value`, `__hdx_series_idx` = 0) AS "avg(metric.alpha)"', + ); + expect(sql).toContain( + 'anyOrNullIf(`__hdx_value`, `__hdx_series_idx` = 1) AS "avg(metric.beta)"', + ); + // The group-by dimension keeps its user-facing name. + expect(sql).toContain('`__hdx_group_0` AS "ServiceName"'); + expect(sql).toContain( + 'GROUP BY `__hdx_group_0`, `__hdx_time_bucket` ORDER BY `__hdx_time_bucket`', + ); + }); + + it('renders a metric ratio as a SQL-side division', async () => { + const generatedSql = await renderChartConfig( + { + ...baseMultiSeriesConfig, + seriesReturnType: 'ratio', + }, + mockMetadata, + querySettings, + ); + const sql = parameterizedQueryToSql(generatedSql); + expect(sql).toMatchSnapshot(); + + // Missing numerator counts as 0; missing/zero denominator yields NULL. + expect(sql).toContain( + 'coalesce(anyOrNullIf(`__hdx_value`, `__hdx_series_idx` = 0), 0) / nullif(anyOrNullIf(`__hdx_value`, `__hdx_series_idx` = 1), 0) AS "avg(metric.alpha)/avg(metric.beta)"', + ); + }); + + it('divides by the per-bucket denominator total in share_of_total mode', async () => { + const generatedSql = await renderChartConfig( + { + ...baseMultiSeriesConfig, + seriesReturnType: 'ratio', + ratioMode: 'share_of_total', + groupBy: [{ aggCondition: '', valueExpression: 'ServiceName' }], + }, + mockMetadata, + querySettings, + ); + const sql = parameterizedQueryToSql(generatedSql); + expect(sql).toContain( + 'nullif(sum(anyOrNullIf(`__hdx_value`, `__hdx_series_idx` = 1)) OVER (PARTITION BY `__hdx_time_bucket`), 0)', + ); + }); + + it('pads group columns across gauge and histogram branch classes', async () => { + const generatedSql = await renderChartConfig( + { + ...baseMultiSeriesConfig, + select: [ + gaugeSelect('metric.alpha'), + { + aggFn: 'quantile', + level: 0.5, + aggCondition: '', + aggConditionLanguage: 'sql', + valueExpression: 'Value', + metricName: 'metric.latency', + metricType: MetricsDataType.Histogram, + }, + ], + groupBy: [{ aggCondition: '', valueExpression: 'ServiceName' }], + }, + mockMetadata, + querySettings, + ); + const sql = parameterizedQueryToSql(generatedSql); + expect(sql).toMatchSnapshot(); + + // The gauge branch pads the histogram's Array group column with [] and + // the histogram branch pads the scalar group column with NULL, so the + // UNION column lists line up while grouped rows keep distinct keys. + expect(sql).toContain('[] AS `group`'); + expect(sql).toContain('NULL AS `__hdx_group_0`'); + expect(sql).toContain( + 'GROUP BY `__hdx_group_0`, `group`, `__hdx_time_bucket`', + ); + }); + + it('suffixes colliding aliases with the split index', async () => { + const generatedSql = await renderChartConfig( + { + ...baseMultiSeriesConfig, + select: [ + { + ...gaugeSelect('metric.alpha'), + aggCondition: "ServiceName = 'svc-a'", + }, + gaugeSelect('metric.alpha'), + ], + }, + mockMetadata, + querySettings, + ); + const sql = parameterizedQueryToSql(generatedSql); + expect(sql).toContain('AS "avg(metric.alpha)"'); + expect(sql).toContain('AS "avg(metric.alpha)__1"'); + }); + + it('merges number-shape series without a GROUP BY into one implicit aggregation', async () => { + const generatedSql = await renderChartConfig( + { + ...baseMultiSeriesConfig, + displayType: DisplayType.Number, + granularity: undefined, + }, + mockMetadata, + querySettings, + ); + const sql = parameterizedQueryToSql(generatedSql); + // No grouping keys at all: the outer query is one implicit global + // aggregation. (The branches' internal `__hdx_time_bucket2` CTE alias + // is unrelated to the composed outer bucket.) + expect(sql).not.toContain('GROUP BY `__hdx_'); + expect(sql).not.toContain('`__hdx_time_bucket`'); + expect(sql.match(/SETTINGS/g)).toHaveLength(1); + }); + }); }); diff --git a/packages/common-utils/src/clickhouse/index.ts b/packages/common-utils/src/clickhouse/index.ts index 4bdd8ba097..5187c2b89e 100644 --- a/packages/common-utils/src/clickhouse/index.ts +++ b/packages/common-utils/src/clickhouse/index.ts @@ -9,24 +9,20 @@ import type { } from '@clickhouse/client-common'; import type { ClickHouseClient as WebClickHouseClient } from '@clickhouse/client-web'; import * as SQLParser from 'node-sql-parser'; -import objectHash from 'object-hash'; import { getMetadata, Metadata } from '@/core/metadata'; import { - FIXED_TIME_BUCKET_EXPR_ALIAS, renderChartConfig, setChartSelectsAlias, - splitChartConfigs, } from '@/core/renderChartConfig'; import { extractSettingsClauseFromEnd, hashCode, - isTimeSeriesDisplayType, replaceJsonExpressions, splitAndTrimWithBracket, } from '@/core/utils'; import { isBuilderChartConfig } from '@/guards'; -import { ChartConfigWithOptDateRange, QuerySettings, RatioMode } from '@/types'; +import { ChartConfigWithOptDateRange, QuerySettings } from '@/types'; // export @clickhouse/client-common types export type { @@ -364,253 +360,6 @@ export const extractColumnReferencesFromKey = (expr: string): string[] => { }); }; -const castToNumber = (value: string | number) => { - if (typeof value === 'string') { - if (value.trim() === '') { - return NaN; - } - return Number(value); - } - return value; -}; - -export const computeRatio = ( - numeratorInput: string | number, - denominatorInput: string | number, -) => { - const numerator = castToNumber(numeratorInput); - const denominator = castToNumber(denominatorInput); - - if (isNaN(numerator) || isNaN(denominator) || denominator === 0) { - return NaN; - } - - return numerator / denominator; -}; - -export const computeResultSetRatio = ( - resultSet: ResponseJSON, - // The numerator/denominator value columns. Passed explicitly by the only - // caller (mergeResultSets) so we don't depend on column order — group-by - // dimensions can be numeric and would otherwise be mistaken for an operand. - operands: { numeratorName: string; denominatorName: string }, - // How a grouped ratio divides; see RatioModeSchema. Defaults to per-group. - // Has no effect on ungrouped ratios (one row per bucket). - mode: RatioMode = 'per_group', -) => { - const _meta = resultSet.meta ?? []; - const _data = resultSet.data; - const numerator = _meta.find(m => m.name === operands.numeratorName); - const denominator = _meta.find(m => m.name === operands.denominatorName); - if (!numerator || !denominator) { - throw new Error( - `Unable to compute ratio - meta information: ${JSON.stringify(_meta)}.`, - ); - } - // Strip the collision-disambiguation suffix (see mergeResultSets) from the - // rendered label so a same-alias ratio reads `count(x)/count(x)`, not - // `count(x)/count(x)__1`. - const denominatorLabel = denominator.name.replace(/__\d+$/, ''); - const ratioColumnName = `${numerator.name}/${denominatorLabel}`; - // Carry through every non-operand column — the timestamp and any group-by - // dimensions — so a grouped ratio renders one series per group instead of - // collapsing into a single line. - const passthroughColumns = _meta.filter( - m => m.name !== numerator.name && m.name !== denominator.name, - ); - - // per_group: each row is divided by its own denominator (each group's own - // rate). share_of_total: each row is divided by the total of the denominator - // column across ALL groups in the same time bucket, so the grouped lines - // decompose the blended rate and sum to the ungrouped value. For an ungrouped - // ratio there's one row per bucket, so the bucket total equals that row's own - // denominator and both modes coincide. - const denominatorForRow = - mode === 'share_of_total' - ? buildBucketTotalDenominator(_data, _meta, denominator.name) - : (row: Record) => row[denominator.name] ?? NaN; - - return { - ...resultSet, - data: _data.map(row => { - // A group absent from the (filtered) numerator query contributes zero, not - // "no data" — so a zero-error group reads 0%, not N/A. - const numeratorValue = row[numerator.name] ?? 0; - return { - [ratioColumnName]: computeRatio(numeratorValue, denominatorForRow(row)), - ...Object.fromEntries( - passthroughColumns.map(c => [c.name, row[c.name]]), - ), - }; - }), - meta: [{ name: ratioColumnName, type: 'Float64' }, ...passthroughColumns], - }; -}; - -// Resolves the time-bucket column that groups rows into buckets. The query -// builder aliases the bucket FIXED_TIME_BUCKET_EXPR_ALIAS, so prefer that exact -// column and only fall back to the first Date-typed column for shapes without -// the alias. Without this preference a Date/DateTime group-by dimension ordered -// ahead of the real bucket would be mistaken for it, so share_of_total totals -// would be summed over the wrong column and the rendered shares silently wrong. -const resolveBucketColumn = (meta: Array<{ name: string; type: string }>) => - meta.find(m => m.name === FIXED_TIME_BUCKET_EXPR_ALIAS) ?? - inferTimestampColumn(meta); - -// Builds a per-row lookup returning the total of the denominator column across -// all rows sharing a time bucket (share-of-total mode). Rows with no timestamp -// column all share one bucket, so a non-time-series grouped ratio becomes each -// group's share of the grand total. -const buildBucketTotalDenominator = ( - data: Record[], - meta: { name: string; type: string }[], - denominatorName: string, -) => { - const timestampColumn = resolveBucketColumn(meta); - const bucketKey = (row: Record) => - timestampColumn ? String(row[timestampColumn.name]) : '__all__'; - const totalByBucket = new Map(); - for (const row of data) { - const value = row[denominatorName]; - // A group missing from the denominator split has an undefined value; - // castToNumber returns it as-is and Number.isNaN(undefined) is false, so - // guard explicitly or it would poison the whole bucket total with NaN. - const denom = value == null ? NaN : castToNumber(value); - if (!Number.isNaN(denom)) { - const key = bucketKey(row); - totalByBucket.set(key, (totalByBucket.get(key) ?? 0) + denom); - } - } - return (row: Record) => totalByBucket.get(bucketKey(row)) ?? NaN; -}; - -/** - * Joins the per-series result sets of a split metric query (one query per - * series) back into a single result set, merging rows that share a time bucket - * (and group-by dimensions, when grouped). When `isRatio` is set, the two - * series are divided via {@link computeResultSetRatio}. - * - * Exported so the merge — the root of the grouped-ratio fix — can be unit - * tested without a live ClickHouse. - */ -export const mergeResultSets = ({ - resultSets, - isTimeSeries, - isRatio, - ratioMode, -}: { - resultSets: ResponseJSON[]; - isTimeSeries: boolean; - isRatio: boolean; - ratioMode?: RatioMode; -}): ResponseJSON => { - const metaSet = new Map(); - const tsBucketMap = new Map>(); - - // Seed metaSet with each split's value column in resultSet order, so the - // joined meta is [value0, value1, ..., non-value columns]. This matches the - // order of config.select that useChartNumberFormats indexes into. - // - // Two splits can resolve to the SAME value-column alias (e.g. a ratio of - // count(request) filtered / unfiltered — the alias omits the WHERE filter). - // If we let them share a column, the row merge below would clobber one - // operand with the other and the ratio would be undefined. So rename a - // colliding value column per split index and remember the (possibly renamed) - // operand name so the ratio divides the right two columns. - const operandNames: string[] = []; - const renamedResultSets = resultSets.map((resultSet, splitIdx) => { - const valueColumn = inferNumericColumn(resultSet.meta ?? [])?.[0]; - if (!valueColumn) { - operandNames.push(''); - return resultSet; - } - const name = metaSet.has(valueColumn.name) - ? `${valueColumn.name}__${splitIdx}` - : valueColumn.name; - operandNames.push(name); - metaSet.set(name, { ...valueColumn, name }); - if (name === valueColumn.name) { - return resultSet; - } - return { - ...resultSet, - meta: (resultSet.meta ?? []).map(m => - m.name === valueColumn.name ? { ...m, name } : m, - ), - data: resultSet.data.map(row => { - const { [valueColumn.name]: value, ...rest } = row; - return { ...rest, [name]: value }; - }), - }; - }); - - // Add other (non-value) columns to metaSet and merge rows. - for (const resultSet of renamedResultSets) { - if (Array.isArray(resultSet.meta)) { - for (const meta of resultSet.meta) { - if (!metaSet.has(meta.name)) { - metaSet.set(meta.name, meta); - } - } - } - - const timestampColumn = resolveBucketColumn(resultSet.meta ?? []); - const numericColumn = inferNumericColumn(resultSet.meta ?? []); - const numericColumnName = numericColumn?.[0]?.name; - for (const row of resultSet.data) { - const _rowWithoutValue = numericColumnName - ? Object.fromEntries( - Object.entries(row).filter(([key]) => key !== numericColumnName), - ) - : { ...row }; - // When the series are grouped, two rows at the same time bucket but - // different group values must stay distinct — key by (bucket + group - // dims) via the hash of the row minus its value column. Without a - // group dimension this collapses to the timestamp (or a fixed key), - // preserving the original behavior. - const hasGroupCols = Object.keys(_rowWithoutValue).some( - key => key !== timestampColumn?.name, - ); - const mergeKey = hasGroupCols - ? objectHash(_rowWithoutValue) - : timestampColumn != null - ? row[timestampColumn.name] - : isTimeSeries - ? objectHash(_rowWithoutValue) - : '__FIXED_TIMESTAMP__'; - if (tsBucketMap.has(mergeKey)) { - tsBucketMap.set(mergeKey, { ...tsBucketMap.get(mergeKey), ...row }); - } else { - tsBucketMap.set(mergeKey, row); - } - } - } - - const merged: ResponseJSON = { - meta: Array.from(metaSet.values()), - data: Array.from(tsBucketMap.values()), - }; - - if (isRatio) { - // Read the operands positionally: a split with no inferable numeric value - // column pushed '' (see above), so compacting with filter(Boolean) would - // shift the surviving name into numeratorName and leave denominatorName - // undefined — throwing "Unable to compute ratio" and failing the whole - // chart response. If either operand is missing we can't divide, so fall - // through and return the merged rows undivided. - const [numeratorName, denominatorName] = operandNames; - if (numeratorName && denominatorName) { - // TODO: we should compute the ratio on the db side - return computeResultSetRatio( - merged, - { numeratorName, denominatorName }, - ratioMode, - ); - } - } - return merged; -}; - export interface QueryInputs { query: string; format?: Format; @@ -827,8 +576,13 @@ export abstract class BaseClickhouseClient { inputs: QueryInputs, ): Promise>; - // TODO: only used when multi-series 'metrics' is selected (no effects on the events chart) - // eventually we want to generate union CTEs on the db side instead of computing it on the client side + /** + * Render the chart config into a single ClickHouse query and return the + * JSON result set. Multi-series metric charts (which used to fan out into + * one query per series merged client-side) are composed into one UNION ALL + * + pivot statement by renderChartConfig, so every config is exactly one + * query round trip. + */ async queryChartConfig({ config, metadata, @@ -846,41 +600,17 @@ export abstract class BaseClickhouseClient { config = isBuilderChartConfig(config) ? setChartSelectsAlias(config) : config; - const queries: ChSql[] = await Promise.all( - splitChartConfigs(config).map(c => - renderChartConfig(c, metadata, querySettings), - ), - ); - - const isTimeSeries = isTimeSeriesDisplayType(config.displayType); - - const resultSets = await Promise.all( - queries.map(async query => { - const resp = await this.query<'JSON'>({ - query: query.sql, - query_params: query.params, - format: 'JSON', - abort_signal: opts?.abort_signal, - connectionId: config.connection, - clickhouse_settings: opts?.clickhouse_settings, - }); - return resp.json(); - }), - ); - - if (resultSets.length === 1) { - return resultSets[0]; - } - // metrics -> join resultSets - else if (isBuilderChartConfig(config) && resultSets.length > 1) { - return mergeResultSets({ - resultSets, - isTimeSeries, - isRatio: config.seriesReturnType === 'ratio' && resultSets.length === 2, - ratioMode: config.ratioMode, - }); - } - throw new Error('No result sets'); + const query = await renderChartConfig(config, metadata, querySettings); + + const resp = await this.query<'JSON'>({ + query: query.sql, + query_params: query.params, + format: 'JSON', + abort_signal: opts?.abort_signal, + connectionId: config.connection, + clickhouse_settings: opts?.clickhouse_settings, + }); + return resp.json(); } /** diff --git a/packages/common-utils/src/core/builderToRawSql.ts b/packages/common-utils/src/core/builderToRawSql.ts index b41ebf2e65..8c14e1d7c9 100644 --- a/packages/common-utils/src/core/builderToRawSql.ts +++ b/packages/common-utils/src/core/builderToRawSql.ts @@ -88,8 +88,11 @@ export async function renderBuilderConfigAsSqlTemplate( }; } - // Metric charts render one query per series (see splitChartConfigs), so only - // single-series metric charts can be converted to a single raw-SQL query. + // Multi-series metric charts compose per-series branch queries into a + // UNION ALL + pivot statement (see renderMultiSeriesMetricChartConfig). + // That composed shape hasn't been wired into the raw-SQL template macros + // yet ($__sourceTable(metricType) per branch), so only single-series metric + // charts can be converted to a raw-SQL query for now. const isMetric = config.metricTables != null; if (isMetric && Array.isArray(config.select) && config.select.length > 1) { return { diff --git a/packages/common-utils/src/core/renderChartConfig.ts b/packages/common-utils/src/core/renderChartConfig.ts index ecb9ea9e72..8d83d3ee37 100644 --- a/packages/common-utils/src/core/renderChartConfig.ts +++ b/packages/common-utils/src/core/renderChartConfig.ts @@ -22,11 +22,7 @@ import { pickBucketTimestampColumn, splitAndTrimWithBracket, } from '@/core/utils'; -import { - isBuilderChartConfig, - isPromqlChartConfig, - isRawSqlChartConfig, -} from '@/guards'; +import { isPromqlChartConfig, isRawSqlChartConfig } from '@/guards'; import { replaceMacros } from '@/macros'; import { buildTextIndexInfoLookup, @@ -42,7 +38,6 @@ import { BuilderChartConfigWithOptDateRange, ChartConfig, ChartConfigSchema, - ChartConfigWithOptDateRange, ChSqlSchema, CteChartConfig, DateRange, @@ -146,36 +141,50 @@ export const setChartSelectsAlias = ( return config; }; -export const splitChartConfigs = ( - config: ChartConfigWithOptDateRange, -): ChartConfigWithOptDateRangeEx[] => { - // only split metric queries for now - if ( - isBuilderChartConfig(config) && - isMetricChartConfig(config) && - Array.isArray(config.select) - ) { - const _configs: BuilderChartConfigWithOptDateRange[] = []; - // split the query into multiple queries - for (const select of config.select) { - _configs.push({ - ...config, - select: [select], - }); - } - return _configs; - } +// Internal aliases used to compose a multi-series metric query. Every +// per-series branch projects its value (and group-by dimensions) under these +// fixed names so the UNION ALL wrapper can match columns by position and the +// outer pivot can reference them without reverse-engineering ClickHouse's +// derived column names. +const MULTI_SERIES_VALUE_ALIAS = '__hdx_value'; +const MULTI_SERIES_IDX_ALIAS = '__hdx_series_idx'; +const multiSeriesGroupAlias = (index: number) => `__hdx_group_${index}`; + +// Histogram translations bake the group-by dimensions into a single Array +// column named GROUP_ALIAS instead of projecting them as individual columns +// (see translateHistogram), so grouped histogram rows never share a merge key +// with grouped gauge/sum rows. +const isHistogramClassSelect = (select: { + metricType?: MetricsDataType; +}): boolean => + select.metricType === MetricsDataType.Histogram || + select.metricType === MetricsDataType.ExponentialHistogram; - if ( - isRawSqlChartConfig(config) || - isPromqlChartConfig(config) || - isBuilderChartConfig(config) - ) { - return [config]; +/** + * Merge per-branch trailing SETTINGS clauses into one deduped item list for + * the composed query. ClickHouse rejects SETTINGS on a UNION ALL branch that + * is followed by further branches, so the clauses are stripped from each + * branch and hoisted here. Known settings (translate-injected internals plus + * source querySettings) are simple `key = value` pairs without commas. + */ +function mergeSettingsClauses(clauses: (string | undefined)[]): string { + const items: string[] = []; + const seen = new Set(); + for (const clause of clauses) { + if (!clause) { + continue; + } + const body = clause.replace(/^settings\s*/i, ''); + for (const rawItem of body.split(',')) { + const item = rawItem.trim(); + if (item !== '' && !seen.has(item)) { + seen.add(item); + items.push(item); + } + } } - - throw new Error(`Unexpected chart config type: ${JSON.stringify(config)}`); -}; + return items.join(', '); +} const INVERSE_OPERATOR_MAP = { '=': '!=', @@ -1975,9 +1984,14 @@ async function translateMetricChartConfig( if (shouldApplyIncreaseGroupLimit) { // Render the user's groupBy against the Bucketed CTE so column - // references resolve to the CTE's projection. + // references resolve to the CTE's projection. Aliases are stripped: + // these expressions go inside tuple(...) in the ranking CTE and the + // outer IN predicate, where an `AS "alias"` suffix is a syntax error + // (mirrors the strip in renderSeriesLimitCte). const groupByForRank = await renderSelectList( - chartConfig.groupBy!, + typeof chartConfig.groupBy === 'string' + ? chartConfig.groupBy + : chartConfig.groupBy!.map(col => ({ ...col, alias: undefined })), { ...chartConfig, from: { databaseName: '', tableName: 'Bucketed' }, @@ -2244,6 +2258,253 @@ export async function renderRawSqlChartConfig( }; } +/** + * Render a multi-series metric chart as ONE composed ClickHouse query. + * + * A metric chart config with N select items used to fan out into N separate + * queries whose result sets were merged node-side (mergeResultSets). This + * renders the same N per-series queries (each metric type keeps its own CTE + * scaffolding and physical table) but composes them via UNION ALL and pivots + * the tagged rows back into one row per (group values, time bucket): + * + * SELECT + * anyOrNullIf(__hdx_value, __hdx_series_idx = 0) AS "avg(metric.alpha)", + * anyOrNullIf(__hdx_value, __hdx_series_idx = 1) AS "avg(metric.beta)", + * __hdx_group_0 AS "ServiceName", ..., + * __hdx_time_bucket + * FROM ( + * SELECT ..., 0 AS __hdx_series_idx FROM () + * UNION ALL + * SELECT ..., 1 AS __hdx_series_idx FROM () + * ) + * GROUP BY __hdx_group_0, ..., __hdx_time_bucket + * ORDER BY __hdx_time_bucket + * + * Contract (pinned by the queryChartConfig integration tests): + * - meta lists all value columns first, in select order (the positional + * contract of useChartNumberFormats); + * - a series with no data at a joined row reads NULL (a gap), never 0; + * - two series resolving to the same alias are suffixed "__{splitIndex}"; + * - seriesReturnType 'ratio' (exactly two series) replaces the operand + * columns with a single "/" column: a missing numerator counts + * as 0, a missing or zero denominator yields NULL (a gap), and ratioMode + * 'share_of_total' divides by the per-bucket denominator total; + * - gauge/sum series project group-by dimensions as individual columns while + * histogram series keep their single Array GROUP_ALIAS column, so grouped + * histogram rows never join with grouped gauge/sum rows (each branch class + * pads the other's group columns with NULL / []). + */ +async function renderMultiSeriesMetricChartConfig( + rawChartConfig: BuilderChartConfigWithOptDateRangeEx, + metadata: Metadata, + querySettings: QuerySettings | undefined, +): Promise { + // Callers usually apply setChartSelectsAlias before rendering; apply it + // again (idempotently) so direct renderChartConfig callers get the same + // user-facing value-column names. + const chartConfig = setChartSelectsAlias(rawChartConfig); + const select = chartConfig.select; + if (typeof select === 'string') { + throw new Error('multi-series metric charts require an array select'); + } + + // User-facing output column names in select order. Two series can resolve + // to the same alias (e.g. the same aggregation filtered vs unfiltered); + // suffix collisions with the split index so the operands stay distinct. + const outputNames: string[] = []; + { + const seen = new Set(); + select.forEach((s, splitIdx) => { + const base = s.alias ?? ''; + const name = seen.has(base) ? `${base}__${splitIdx}` : base; + seen.add(name); + outputNames.push(name); + }); + } + + const hasGranularity = isUsingGranularity(chartConfig); + const includeGroupBy = + isUsingGroupBy(chartConfig) && chartConfig.selectGroupBy !== false; + + // Normalize the group-by into one column per dimension with an injected + // internal alias, so the wrapper can reference the branch output columns + // by a known name. The user-facing output name is the user's alias when + // set, otherwise the expression text. + type GroupByItem = Exclude< + NonNullable, + string + >[number]; + const groupColumns: Array<{ + item: GroupByItem; + outputName: string; + }> = []; + if (includeGroupBy) { + if (typeof chartConfig.groupBy === 'string') { + splitAndTrimWithBracket(chartConfig.groupBy).forEach((expr, j) => { + groupColumns.push({ + item: { + valueExpression: expr, + aggCondition: '', + alias: multiSeriesGroupAlias(j), + }, + outputName: expr, + }); + }); + } else { + chartConfig.groupBy.forEach((g, j) => { + groupColumns.push({ + item: { ...g, alias: multiSeriesGroupAlias(j) }, + outputName: + g.alias != null && g.alias.trim() !== '' + ? g.alias + : g.valueExpression, + }); + }); + } + } + + const branchIsHistogram = select.map(isHistogramClassSelect); + const hasScalarGroups = + includeGroupBy && + groupColumns.length > 0 && + branchIsHistogram.some(isHistogram => !isHistogram); + const hasHistogramGroup = + includeGroupBy && branchIsHistogram.some(isHistogram => isHistogram); + + // Render each per-series branch exactly as the single-series path would, + // but with internal aliases on the value and (for gauge/sum) the group-by + // columns. Trailing SETTINGS are stripped and hoisted to the outer query. + const branches = await Promise.all( + select.map(async s => { + const isHistogram = isHistogramClassSelect(s); + const branchConfig: ChartConfigWithOptDateRangeEx = { + ...chartConfig, + select: [{ ...s, alias: MULTI_SERIES_VALUE_ALIAS }], + // Histogram translations render the group-by inside their own + // Array-typed GROUP_ALIAS column; leave it untouched there. + groupBy: includeGroupBy + ? isHistogram + ? chartConfig.groupBy + : groupColumns.map(g => g.item) + : chartConfig.groupBy, + }; + const rendered = await renderChartConfig( + branchConfig, + metadata, + querySettings, + ); + const [sql, settingsClause] = extractSettingsClauseFromEnd(rendered.sql); + return { sql, params: rendered.params, settingsClause, isHistogram }; + }), + ); + + // Wrap every branch in a projection with an identical column list (UNION + // ALL matches columns by position): value, scalar group columns, histogram + // group array, time bucket, series index. Branches missing a column pad it + // with NULL / [] so mixed branch classes stay type-compatible while their + // rows keep distinct grouping keys. + const branchSelects = branches.map((branch, splitIdx) => { + const columns: string[] = [`\`${MULTI_SERIES_VALUE_ALIAS}\``]; + if (hasScalarGroups) { + groupColumns.forEach((_, j) => { + columns.push( + branch.isHistogram + ? `NULL AS \`${multiSeriesGroupAlias(j)}\`` + : `\`${multiSeriesGroupAlias(j)}\``, + ); + }); + } + if (hasHistogramGroup) { + columns.push( + branch.isHistogram ? `\`${GROUP_ALIAS}\`` : `[] AS \`${GROUP_ALIAS}\``, + ); + } + if (hasGranularity) { + columns.push(`\`${FIXED_TIME_BUCKET_EXPR_ALIAS}\``); + } + columns.push(`${splitIdx} AS \`${MULTI_SERIES_IDX_ALIAS}\``); + return chSql`SELECT ${{ UNSAFE_RAW_SQL: columns.join(', ') }} FROM (${{ + sql: branch.sql, + params: branch.params, + }})`; + }); + const unionSql = concatChSql(' UNION ALL ', branchSelects); + + // Pivot the tagged rows back into one column per series. anyOrNullIf + // returns NULL (a rendered gap) when a series has no row at this + // (group, bucket) key — a plain anyIf would default to 0. + const valueExprFor = (splitIdx: number) => + `anyOrNullIf(\`${MULTI_SERIES_VALUE_ALIAS}\`, \`${MULTI_SERIES_IDX_ALIAS}\` = ${splitIdx})`; + + const isRatio = + chartConfig.seriesReturnType === 'ratio' && select.length === 2; + + const projection: string[] = []; + if (isRatio) { + // A group absent from the (filtered) numerator contributes zero, not + // "no data" — so a zero-error group reads 0%, not N/A. A missing or zero + // denominator makes the quotient NULL, which renders as a gap. + const numerator = `coalesce(${valueExprFor(0)}, 0)`; + // per_group: each row divides by its own denominator. share_of_total: + // each row divides by the denominator total across ALL groups in the + // same time bucket, so the grouped lines decompose the blended rate and + // sum to the ungrouped value. + const denominator = + chartConfig.ratioMode === 'share_of_total' + ? `sum(${valueExprFor(1)}) OVER (${ + hasGranularity + ? `PARTITION BY \`${FIXED_TIME_BUCKET_EXPR_ALIAS}\`` + : '' + })` + : valueExprFor(1); + // Strip the collision-disambiguation suffix from the label so a + // same-alias ratio reads "avg(x)/avg(x)", not "avg(x)/avg(x)__1". + const ratioName = `${outputNames[0]}/${outputNames[1].replace(/__\d+$/, '')}`; + projection.push( + `${numerator} / nullif(${denominator}, 0) AS "${ratioName}"`, + ); + } else { + outputNames.forEach((outputName, splitIdx) => { + projection.push(`${valueExprFor(splitIdx)} AS "${outputName}"`); + }); + } + if (hasScalarGroups) { + groupColumns.forEach((g, j) => { + projection.push(`\`${multiSeriesGroupAlias(j)}\` AS "${g.outputName}"`); + }); + } + if (hasHistogramGroup) { + projection.push(`\`${GROUP_ALIAS}\``); + } + if (hasGranularity) { + projection.push(`\`${FIXED_TIME_BUCKET_EXPR_ALIAS}\``); + } + + // Join key: group dimensions plus the time bucket. With neither (a number + // chart), the implicit global aggregation merges everything into one row. + const groupByKeys = [ + ...(hasScalarGroups + ? groupColumns.map((_, j) => `\`${multiSeriesGroupAlias(j)}\``) + : []), + ...(hasHistogramGroup ? [`\`${GROUP_ALIAS}\``] : []), + ...(hasGranularity ? [`\`${FIXED_TIME_BUCKET_EXPR_ALIAS}\``] : []), + ]; + + const settings = mergeSettingsClauses(branches.map(b => b.settingsClause)); + + return concatChSql(' ', [ + chSql`SELECT ${{ UNSAFE_RAW_SQL: projection.join(', ') }}`, + chSql`FROM (${unionSql})`, + groupByKeys.length > 0 + ? chSql`GROUP BY ${{ UNSAFE_RAW_SQL: groupByKeys.join(', ') }}` + : chSql``, + hasGranularity + ? chSql`ORDER BY \`${FIXED_TIME_BUCKET_EXPR_ALIAS}\`` + : chSql``, + settings !== '' ? chSql`SETTINGS ${{ UNSAFE_RAW_SQL: settings }}` : chSql``, + ]); +} + export async function renderChartConfig( rawChartConfig: ChartConfigWithOptDateRangeEx, metadata: Metadata, @@ -2258,6 +2519,22 @@ export async function renderChartConfig( return renderRawSqlChartConfig(rawChartConfig, metadata); } + // A metric chart with multiple series composes one query per series into a + // single UNION ALL + pivot statement (each metric type needs its own CTE + // scaffolding, and different types read different physical tables). The + // per-series branches recurse through this function with a single select. + if ( + isMetricChartConfig(rawChartConfig) && + Array.isArray(rawChartConfig.select) && + rawChartConfig.select.length > 1 + ) { + return renderMultiSeriesMetricChartConfig( + rawChartConfig, + metadata, + querySettings, + ); + } + // metric types require more rewriting since we know more about the schema // but goes through the same generation process const translatedChartConfig = isMetricChartConfig(rawChartConfig) From 9aa6819628f9bd3cd73b3668703390d91ff8c39b Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Tue, 11 Aug 2026 23:25:29 -0700 Subject: [PATCH 2/2] fix: preserve ClickHouse-derived group column names in composed metric query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The k8s dashboard e2e caught a regression in the composed multi-series query: group-by columns were re-aliased to their expression text (ResourceAttributes['k8s.namespace.name']), but consumers — the Kubernetes dashboard's row lookups and external-API clients — read them by the name a single-series query produces, which for un-aliased expressions is ClickHouse's DERIVED name (arrayElement(ResourceAttributes, 'k8s.namespace.name')). The rename blanked the namespace/pod cells. Derived names can't be reproduced node-side, so stop renaming entirely: - branches render their group-by untouched (natural names preserved) - gauge/sum wrappers use SELECT * and are ordered first in the UNION (UNION ALL matches columns by position and takes names from its first branch); histogram wrappers are explicit with NULL pads whose names are never referenced - the outer pivot passes group/bucket columns through with * EXCEPT (__hdx_value, __hdx_series_idx) and groups via GROUP BY ALL (ClickHouse >= 22.12) instead of naming them Adds two int regression tests pinning the contract: un-aliased expression group-bys keep the arrayElement(...) derived name, and user-aliased ones keep the alias. --- .../renderChartConfig.test.ts.snap | 18 +- .../__tests__/queryChartConfig.int.test.ts | 62 ++++++ .../src/__tests__/renderChartConfig.test.ts | 22 +- .../src/core/renderChartConfig.ts | 206 +++++++++--------- 4 files changed, 185 insertions(+), 123 deletions(-) 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 b7f39821c1..0b71db01e1 100644 --- a/packages/common-utils/src/__tests__/__snapshots__/renderChartConfig.test.ts.snap +++ b/packages/common-utils/src/__tests__/__snapshots__/renderChartConfig.test.ts.snap @@ -734,7 +734,7 @@ exports[`renderChartConfig k8s semantic convention migrations should handle metr `; exports[`renderChartConfig multi-series metric charts (composed query) composes grouped multi-series gauges into one UNION ALL + pivot query 1`] = ` -"SELECT anyOrNullIf(\`__hdx_value\`, \`__hdx_series_idx\` = 0) AS "avg(metric.alpha)", anyOrNullIf(\`__hdx_value\`, \`__hdx_series_idx\` = 1) AS "avg(metric.beta)", \`__hdx_group_0\` AS "ServiceName", \`__hdx_time_bucket\` FROM (SELECT \`__hdx_value\`, \`__hdx_group_0\`, \`__hdx_time_bucket\`, 0 AS \`__hdx_series_idx\` FROM (WITH Source AS ( +"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 *, 0 AS \`__hdx_series_idx\` FROM (WITH Source AS ( SELECT *, cityHash64(ScopeAttributes, ResourceAttributes, Attributes) AS AttributesHash @@ -763,7 +763,7 @@ exports[`renderChartConfig multi-series metric charts (composed query) composes ORDER BY AttributesHash, __hdx_time_bucket2 ) SELECT avg( toFloat64OrDefault(toString(LastValue)) - ) AS "__hdx_value",ServiceName AS "__hdx_group_0",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 ServiceName AS "__hdx_group_0",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\`) UNION ALL SELECT \`__hdx_value\`, \`__hdx_group_0\`, \`__hdx_time_bucket\`, 1 AS \`__hdx_series_idx\` FROM (WITH Source AS ( + ) AS "__hdx_value",ServiceName,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 ServiceName,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\`) UNION ALL SELECT *, 1 AS \`__hdx_series_idx\` FROM (WITH Source AS ( SELECT *, cityHash64(ScopeAttributes, ResourceAttributes, Attributes) AS AttributesHash @@ -792,11 +792,11 @@ exports[`renderChartConfig multi-series metric charts (composed query) composes ORDER BY AttributesHash, __hdx_time_bucket2 ) SELECT avg( toFloat64OrDefault(toString(LastValue)) - ) AS "__hdx_value",ServiceName AS "__hdx_group_0",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 ServiceName AS "__hdx_group_0",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 \`__hdx_group_0\`, \`__hdx_time_bucket\` 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" + ) AS "__hdx_value",ServiceName,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 ServiceName,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) 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)", \`__hdx_group_0\` AS "ServiceName", \`group\`, \`__hdx_time_bucket\` FROM (SELECT \`__hdx_value\`, \`__hdx_group_0\`, [] AS \`group\`, \`__hdx_time_bucket\`, 0 AS \`__hdx_series_idx\` FROM (WITH Source AS ( +"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 *, 0 AS \`__hdx_series_idx\`, [] AS \`group\` FROM (WITH Source AS ( SELECT *, cityHash64(ScopeAttributes, ResourceAttributes, Attributes) AS AttributesHash @@ -825,7 +825,7 @@ exports[`renderChartConfig multi-series metric charts (composed query) pads grou ORDER BY AttributesHash, __hdx_time_bucket2 ) SELECT avg( toFloat64OrDefault(toString(LastValue)) - ) AS "__hdx_value",ServiceName AS "__hdx_group_0",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 ServiceName AS "__hdx_group_0",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\`) UNION ALL SELECT \`__hdx_value\`, NULL AS \`__hdx_group_0\`, \`group\`, \`__hdx_time_bucket\`, 1 AS \`__hdx_series_idx\` FROM (WITH source AS ( + ) AS "__hdx_value",ServiceName,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 ServiceName,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\`) UNION ALL SELECT \`__hdx_value\`, NULL AS \`__hdx_group_pad_0\`, \`__hdx_time_bucket\`, 1 AS \`__hdx_series_idx\`, \`group\` FROM (WITH source AS ( SELECT MetricName, ExplicitBounds, @@ -913,11 +913,11 @@ group, END AS "__hdx_value" FROM points WHERE length(point) > 1 AND total > 0 - ) SELECT \`__hdx_time_bucket\`, group, "__hdx_value" FROM metrics WHERE (\`__hdx_time_bucket\` >= fromUnixTimestamp64Milli(1739318400000) AND \`__hdx_time_bucket\` <= fromUnixTimestamp64Milli(1739491200000)))) GROUP BY \`__hdx_group_0\`, \`group\`, \`__hdx_time_bucket\` 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" + ) SELECT \`__hdx_time_bucket\`, group, "__hdx_value" FROM metrics WHERE (\`__hdx_time_bucket\` >= fromUnixTimestamp64Milli(1739318400000) AND \`__hdx_time_bucket\` <= fromUnixTimestamp64Milli(1739491200000)))) 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) renders a metric ratio as a SQL-side division 1`] = ` -"SELECT coalesce(anyOrNullIf(\`__hdx_value\`, \`__hdx_series_idx\` = 0), 0) / nullif(anyOrNullIf(\`__hdx_value\`, \`__hdx_series_idx\` = 1), 0) AS "avg(metric.alpha)/avg(metric.beta)", \`__hdx_time_bucket\` FROM (SELECT \`__hdx_value\`, \`__hdx_time_bucket\`, 0 AS \`__hdx_series_idx\` FROM (WITH Source AS ( +"SELECT coalesce(anyOrNullIf(\`__hdx_value\`, \`__hdx_series_idx\` = 0), 0) / nullif(anyOrNullIf(\`__hdx_value\`, \`__hdx_series_idx\` = 1), 0) AS "avg(metric.alpha)/avg(metric.beta)", * EXCEPT (\`__hdx_value\`, \`__hdx_series_idx\`) FROM (SELECT *, 0 AS \`__hdx_series_idx\` FROM (WITH Source AS ( SELECT *, cityHash64(ScopeAttributes, ResourceAttributes, Attributes) AS AttributesHash @@ -946,7 +946,7 @@ exports[`renderChartConfig multi-series metric charts (composed query) renders a ORDER BY AttributesHash, __hdx_time_bucket2 ) SELECT avg( toFloat64OrDefault(toString(LastValue)) - ) 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\`) UNION ALL SELECT \`__hdx_value\`, \`__hdx_time_bucket\`, 1 AS \`__hdx_series_idx\` FROM (WITH Source AS ( + ) 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\`) UNION ALL SELECT *, 1 AS \`__hdx_series_idx\` FROM (WITH Source AS ( SELECT *, cityHash64(ScopeAttributes, ResourceAttributes, Attributes) AS AttributesHash @@ -975,7 +975,7 @@ exports[`renderChartConfig multi-series metric charts (composed query) renders a ORDER BY AttributesHash, __hdx_time_bucket2 ) SELECT avg( toFloat64OrDefault(toString(LastValue)) - ) 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 \`__hdx_time_bucket\` 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" + ) 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 sample-weighted aggregations should handle complex sampleWeightExpression like SpanAttributes map access 1`] = `"SELECT sum(greatest(toUInt64OrZero(toString(SpanAttributes['SampleRate'])), 1)) FROM default.otel_traces WHERE (Timestamp >= fromUnixTimestamp64Milli(1739318400000) AND Timestamp <= fromUnixTimestamp64Milli(1739491200000)) SETTINGS 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"`; diff --git a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts index 0a04e7fdad..ff02a1b5a8 100644 --- a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts +++ b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts @@ -1345,6 +1345,68 @@ describe('queryChartConfig Integration Tests', () => { expect(col(svcC, 'avg(grp.two)')).toBe(30); }); + // Consumers read group columns by the exact name a single-series query + // would produce — for an un-aliased map access that is ClickHouse's + // DERIVED name (arrayElement(...)), not the expression text. The + // Kubernetes dashboard (KubernetesDashboardPage.tsx) and external-API + // clients both do row lookups like + // row["arrayElement(ResourceAttributes, 'k8s.namespace.name')"], so the + // merge must not rename these columns. Regression test for the k8s e2e + // failure where the composed query re-aliased group columns to their + // expression text and blanked the namespace/pod cells. + it('preserves ClickHouse-derived column names for expression group-bys', async () => { + const result = await runConfig( + baseConfig({ + select: [gaugeSelect('grp.one'), gaugeSelect('grp.two')], + groupBy: [ + { + aggCondition: '', + valueExpression: "ResourceAttributes['service.name']", + }, + ], + }), + ); + + expectNumericValueColumns(result.meta, ['avg(grp.one)', 'avg(grp.two)']); + const DERIVED_NAME = "arrayElement(ResourceAttributes, 'service.name')"; + expect(result.meta?.map(m => m.name)).toContain(DERIVED_NAME); + + // Same fixture as the ServiceName-grouped test above (the resource + // attribute mirrors ServiceName), keyed by the derived column name. + const rows = rowsByBucketAndGroup(result.data, DERIVED_NAME); + expect([...rows.keys()].sort()).toEqual([ + `${bucket(0)}|svc-a`, + `${bucket(0)}|svc-b`, + `${bucket(0)}|svc-c`, + ]); + const svcA = rows.get(`${bucket(0)}|svc-a`); + expect(col(svcA, 'avg(grp.one)')).toBe(1); + expect(col(svcA, 'avg(grp.two)')).toBe(10); + expect(col(rows.get(`${bucket(0)}|svc-b`), 'avg(grp.one)')).toBe(2); + expectGap(col(rows.get(`${bucket(0)}|svc-b`), 'avg(grp.two)')); + }); + + it('keeps a user alias on an expression group-by as the column name', async () => { + const result = await runConfig( + baseConfig({ + select: [gaugeSelect('grp.one'), gaugeSelect('grp.two')], + groupBy: [ + { + aggCondition: '', + valueExpression: "ResourceAttributes['service.name']", + alias: 'service', + }, + ], + }), + ); + + expect(result.meta?.map(m => m.name)).toContain('service'); + const rows = rowsByBucketAndGroup(result.data, 'service'); + expect(rows.size).toBe(3); + expect(col(rows.get(`${bucket(0)}|svc-b`), 'avg(grp.one)')).toBe(2); + expect(col(rows.get(`${bucket(0)}|svc-c`), 'avg(grp.two)')).toBe(30); + }); + it('computes an ungrouped metric ratio with 0-for-missing-numerator and gap-for-missing-denominator', async () => { const result = await runConfig( baseConfig({ diff --git a/packages/common-utils/src/__tests__/renderChartConfig.test.ts b/packages/common-utils/src/__tests__/renderChartConfig.test.ts index f4364a1e50..bb76500086 100644 --- a/packages/common-utils/src/__tests__/renderChartConfig.test.ts +++ b/packages/common-utils/src/__tests__/renderChartConfig.test.ts @@ -3652,11 +3652,16 @@ describe('renderChartConfig', () => { expect(sql).toContain( 'anyOrNullIf(`__hdx_value`, `__hdx_series_idx` = 1) AS "avg(metric.beta)"', ); - // The group-by dimension keeps its user-facing name. - expect(sql).toContain('`__hdx_group_0` AS "ServiceName"'); + // The group-by column passes through UNRENAMED (consumers look rows up + // by the single-series column name, which for expressions is + // ClickHouse's derived name and can't be reproduced node-side): the + // wrappers keep the branch projection via SELECT * and the outer pivot + // passes it through via * EXCEPT + GROUP BY ALL. + expect(sql).toContain('SELECT *, 0 AS `__hdx_series_idx`'); expect(sql).toContain( - 'GROUP BY `__hdx_group_0`, `__hdx_time_bucket` ORDER BY `__hdx_time_bucket`', + '* EXCEPT (`__hdx_value`, `__hdx_series_idx`) FROM', ); + expect(sql).toContain('GROUP BY ALL ORDER BY `__hdx_time_bucket`'); }); it('renders a metric ratio as a SQL-side division', async () => { @@ -3720,12 +3725,15 @@ describe('renderChartConfig', () => { // The gauge branch pads the histogram's Array group column with [] and // the histogram branch pads the scalar group column with NULL, so the - // UNION column lists line up while grouped rows keep distinct keys. + // UNION column lists line up positionally while grouped rows keep + // distinct keys. The scalar branch comes first so its (natural) group + // column names win the union. expect(sql).toContain('[] AS `group`'); - expect(sql).toContain('NULL AS `__hdx_group_0`'); - expect(sql).toContain( - 'GROUP BY `__hdx_group_0`, `group`, `__hdx_time_bucket`', + expect(sql).toContain('NULL AS `__hdx_group_pad_0`'); + expect(sql.indexOf('SELECT *, 0 AS `__hdx_series_idx`')).toBeLessThan( + sql.indexOf('NULL AS `__hdx_group_pad_0`'), ); + expect(sql).toContain('GROUP BY ALL'); }); it('suffixes colliding aliases with the split index', async () => { diff --git a/packages/common-utils/src/core/renderChartConfig.ts b/packages/common-utils/src/core/renderChartConfig.ts index 8d83d3ee37..61f2fbe8d1 100644 --- a/packages/common-utils/src/core/renderChartConfig.ts +++ b/packages/common-utils/src/core/renderChartConfig.ts @@ -142,13 +142,13 @@ export const setChartSelectsAlias = ( }; // Internal aliases used to compose a multi-series metric query. Every -// per-series branch projects its value (and group-by dimensions) under these -// fixed names so the UNION ALL wrapper can match columns by position and the -// outer pivot can reference them without reverse-engineering ClickHouse's -// derived column names. +// per-series branch projects its VALUE under this fixed name (and is tagged +// with its series index) so the outer pivot can reference them. Group-by and +// time-bucket columns deliberately keep their natural single-series names — +// consumers look rows up by ClickHouse's derived column names, which can't +// be reproduced node-side (see renderMultiSeriesMetricChartConfig). const MULTI_SERIES_VALUE_ALIAS = '__hdx_value'; const MULTI_SERIES_IDX_ALIAS = '__hdx_series_idx'; -const multiSeriesGroupAlias = (index: number) => `__hdx_group_${index}`; // Histogram translations bake the group-by dimensions into a single Array // column named GROUP_ALIAS instead of projecting them as individual columns @@ -2270,19 +2270,31 @@ export async function renderRawSqlChartConfig( * SELECT * anyOrNullIf(__hdx_value, __hdx_series_idx = 0) AS "avg(metric.alpha)", * anyOrNullIf(__hdx_value, __hdx_series_idx = 1) AS "avg(metric.beta)", - * __hdx_group_0 AS "ServiceName", ..., - * __hdx_time_bucket + * * EXCEPT (__hdx_value, __hdx_series_idx) * FROM ( - * SELECT ..., 0 AS __hdx_series_idx FROM () + * SELECT *, 0 AS __hdx_series_idx FROM () * UNION ALL - * SELECT ..., 1 AS __hdx_series_idx FROM () + * SELECT *, 1 AS __hdx_series_idx FROM () * ) - * GROUP BY __hdx_group_0, ..., __hdx_time_bucket + * GROUP BY ALL * ORDER BY __hdx_time_bucket * + * Only the per-series VALUE column gets an internal alias (__hdx_value); + * group-by columns keep whatever name the single-series query produces — + * the user's alias, a plain column name, or ClickHouse's derived name for + * an expression (e.g. arrayElement(ResourceAttributes, 'k8s.pod.name') for + * ResourceAttributes['k8s.pod.name']). Consumers such as the Kubernetes + * dashboard and external-API clients look rows up by those derived names, + * so they must survive the merge byte-for-byte. Since the names of + * un-aliased expression group-bys can't be reproduced node-side, the + * wrappers use SELECT * (position-matched by UNION ALL, which takes column + * names from its first branch) and the outer pivot uses * EXCEPT + + * GROUP BY ALL instead of naming them. + * * Contract (pinned by the queryChartConfig integration tests): * - meta lists all value columns first, in select order (the positional * contract of useChartNumberFormats); + * - group-by and time-bucket columns keep their single-series names; * - a series with no data at a joined row reads NULL (a gap), never 0; * - two series resolving to the same alias are suffixed "__{splitIndex}"; * - seriesReturnType 'ratio' (exactly two series) replaces the operand @@ -2326,67 +2338,32 @@ async function renderMultiSeriesMetricChartConfig( const includeGroupBy = isUsingGroupBy(chartConfig) && chartConfig.selectGroupBy !== false; - // Normalize the group-by into one column per dimension with an injected - // internal alias, so the wrapper can reference the branch output columns - // by a known name. The user-facing output name is the user's alias when - // set, otherwise the expression text. - type GroupByItem = Exclude< - NonNullable, - string - >[number]; - const groupColumns: Array<{ - item: GroupByItem; - outputName: string; - }> = []; - if (includeGroupBy) { - if (typeof chartConfig.groupBy === 'string') { - splitAndTrimWithBracket(chartConfig.groupBy).forEach((expr, j) => { - groupColumns.push({ - item: { - valueExpression: expr, - aggCondition: '', - alias: multiSeriesGroupAlias(j), - }, - outputName: expr, - }); - }); - } else { - chartConfig.groupBy.forEach((g, j) => { - groupColumns.push({ - item: { ...g, alias: multiSeriesGroupAlias(j) }, - outputName: - g.alias != null && g.alias.trim() !== '' - ? g.alias - : g.valueExpression, - }); - }); - } - } + // How many individual group-by columns a gauge/sum branch projects. Needed + // only so histogram branches can pad the same number of NULL columns — + // their names are never referenced. + const scalarGroupCount = !includeGroupBy + ? 0 + : typeof chartConfig.groupBy === 'string' + ? splitAndTrimWithBracket(chartConfig.groupBy).length + : chartConfig.groupBy.length; const branchIsHistogram = select.map(isHistogramClassSelect); const hasScalarGroups = includeGroupBy && - groupColumns.length > 0 && + scalarGroupCount > 0 && branchIsHistogram.some(isHistogram => !isHistogram); const hasHistogramGroup = includeGroupBy && branchIsHistogram.some(isHistogram => isHistogram); - // Render each per-series branch exactly as the single-series path would, - // but with internal aliases on the value and (for gauge/sum) the group-by - // columns. Trailing SETTINGS are stripped and hoisted to the outer query. + // Render each per-series branch exactly as the single-series path would — + // only the value column gets an internal alias. Trailing SETTINGS are + // stripped and hoisted to the outer query (ClickHouse rejects SETTINGS on + // a non-final UNION branch). const branches = await Promise.all( - select.map(async s => { - const isHistogram = isHistogramClassSelect(s); + select.map(async (s, splitIdx) => { const branchConfig: ChartConfigWithOptDateRangeEx = { ...chartConfig, select: [{ ...s, alias: MULTI_SERIES_VALUE_ALIAS }], - // Histogram translations render the group-by inside their own - // Array-typed GROUP_ALIAS column; leave it untouched there. - groupBy: includeGroupBy - ? isHistogram - ? chartConfig.groupBy - : groupColumns.map(g => g.item) - : chartConfig.groupBy, }; const rendered = await renderChartConfig( branchConfig, @@ -2394,35 +2371,58 @@ async function renderMultiSeriesMetricChartConfig( querySettings, ); const [sql, settingsClause] = extractSettingsClauseFromEnd(rendered.sql); - return { sql, params: rendered.params, settingsClause, isHistogram }; + return { + sql, + params: rendered.params, + settingsClause, + isHistogram: isHistogramClassSelect(s), + splitIdx, + }; }), ); - // Wrap every branch in a projection with an identical column list (UNION - // ALL matches columns by position): value, scalar group columns, histogram - // group array, time bucket, series index. Branches missing a column pad it - // with NULL / [] so mixed branch classes stay type-compatible while their - // rows keep distinct grouping keys. - const branchSelects = branches.map((branch, splitIdx) => { - const columns: string[] = [`\`${MULTI_SERIES_VALUE_ALIAS}\``]; - if (hasScalarGroups) { - groupColumns.forEach((_, j) => { - columns.push( - branch.isHistogram - ? `NULL AS \`${multiSeriesGroupAlias(j)}\`` - : `\`${multiSeriesGroupAlias(j)}\``, - ); - }); - } - if (hasHistogramGroup) { - columns.push( - branch.isHistogram ? `\`${GROUP_ALIAS}\`` : `[] AS \`${GROUP_ALIAS}\``, - ); - } - if (hasGranularity) { - columns.push(`\`${FIXED_TIME_BUCKET_EXPR_ALIAS}\``); + // Wrap every branch so the UNION ALL column lists line up by position: + // [value, , time bucket?, series index, group?] + // + // A gauge/sum branch naturally projects [value, groups..., bucket?] + // (renderSelect order), so SELECT * preserves that prefix — and, crucially, + // the original group column names. A histogram branch projects + // [bucket?, group, value] under fixed aliases, so it is re-projected + // explicitly, padding each scalar group slot with NULL (the pad names are + // irrelevant: UNION ALL takes names from its first branch). Scalar + // branches are ordered first so their names win; when every branch is a + // histogram there are no scalar group columns to preserve. + const orderedBranches = [ + ...branches.filter(branch => !branch.isHistogram), + ...branches.filter(branch => branch.isHistogram), + ]; + const branchSelects = orderedBranches.map(branch => { + const idxColumn = `${branch.splitIdx} AS \`${MULTI_SERIES_IDX_ALIAS}\``; + const columns: string[] = []; + if (branch.isHistogram) { + columns.push(`\`${MULTI_SERIES_VALUE_ALIAS}\``); + if (hasScalarGroups) { + for (let j = 0; j < scalarGroupCount; j++) { + columns.push(`NULL AS \`__hdx_group_pad_${j}\``); + } + } + if (hasGranularity) { + columns.push(`\`${FIXED_TIME_BUCKET_EXPR_ALIAS}\``); + } + columns.push(idxColumn); + if (hasHistogramGroup) { + columns.push(`\`${GROUP_ALIAS}\``); + } + } else { + columns.push('*'); + columns.push(idxColumn); + if (hasHistogramGroup) { + // Empty-array pad ([] is Array(Nothing), the supertype-compatible + // empty array) so scalar rows never share a grouping key with + // histogram rows. + columns.push(`[] AS \`${GROUP_ALIAS}\``); + } } - columns.push(`${splitIdx} AS \`${MULTI_SERIES_IDX_ALIAS}\``); return chSql`SELECT ${{ UNSAFE_RAW_SQL: columns.join(', ') }} FROM (${{ sql: branch.sql, params: branch.params, @@ -2468,36 +2468,28 @@ async function renderMultiSeriesMetricChartConfig( projection.push(`${valueExprFor(splitIdx)} AS "${outputName}"`); }); } - if (hasScalarGroups) { - groupColumns.forEach((g, j) => { - projection.push(`\`${multiSeriesGroupAlias(j)}\` AS "${g.outputName}"`); - }); - } - if (hasHistogramGroup) { - projection.push(`\`${GROUP_ALIAS}\``); - } - if (hasGranularity) { - projection.push(`\`${FIXED_TIME_BUCKET_EXPR_ALIAS}\``); - } - // Join key: group dimensions plus the time bucket. With neither (a number - // chart), the implicit global aggregation merges everything into one row. - const groupByKeys = [ - ...(hasScalarGroups - ? groupColumns.map((_, j) => `\`${multiSeriesGroupAlias(j)}\``) - : []), - ...(hasHistogramGroup ? [`\`${GROUP_ALIAS}\``] : []), - ...(hasGranularity ? [`\`${FIXED_TIME_BUCKET_EXPR_ALIAS}\``] : []), - ]; + // Pass the group and bucket columns through under their original names + // (which are not knowable node-side for expression group-bys), and group + // by exactly those columns. GROUP BY ALL expands to every non-aggregate + // SELECT expression, i.e. the * EXCEPT list; the pivot/ratio expressions + // contain aggregate functions and are excluded. With no passthrough + // columns at all (a number chart) the implicit global aggregation merges + // everything into one row. + const hasPassthroughColumns = + hasScalarGroups || hasHistogramGroup || hasGranularity; + if (hasPassthroughColumns) { + projection.push( + `* EXCEPT (\`${MULTI_SERIES_VALUE_ALIAS}\`, \`${MULTI_SERIES_IDX_ALIAS}\`)`, + ); + } const settings = mergeSettingsClauses(branches.map(b => b.settingsClause)); return concatChSql(' ', [ chSql`SELECT ${{ UNSAFE_RAW_SQL: projection.join(', ') }}`, chSql`FROM (${unionSql})`, - groupByKeys.length > 0 - ? chSql`GROUP BY ${{ UNSAFE_RAW_SQL: groupByKeys.join(', ') }}` - : chSql``, + hasPassthroughColumns ? chSql`GROUP BY ALL` : chSql``, hasGranularity ? chSql`ORDER BY \`${FIXED_TIME_BUCKET_EXPR_ALIAS}\`` : chSql``,