diff --git a/.changeset/fix-multi-series-mixed-value-types.md b/.changeset/fix-multi-series-mixed-value-types.md new file mode 100644 index 0000000000..5de81fda5f --- /dev/null +++ b/.changeset/fix-multi-series-mixed-value-types.md @@ -0,0 +1,6 @@ +--- +'@hyperdx/common-utils': patch +'@hyperdx/app': patch +--- + +Fix multi-series metric charts mixing float and integer aggregations (e.g. histogram quantile + histogram count) failing with "No value columns found in result column metadata". The composed UNION ALL query now normalizes every series value to Float64, so the merged column type is deterministic instead of erroring with NO_COMMON_TYPE or producing a Variant(Float64, Int64) column depending on the ClickHouse server's `use_variant_as_common_type` setting. As a defensive layer, all-numeric `Variant(...)` result columns (e.g. from raw-SQL charts) are now also classified as numeric. 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 0b71db01e1..e2d303a24c 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)", * EXCEPT (\`__hdx_value\`, \`__hdx_series_idx\`) FROM (SELECT *, 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 * REPLACE (toFloat64(\`__hdx_value\`) AS \`__hdx_value\`), 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,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 ( + ) 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 * REPLACE (toFloat64(\`__hdx_value\`) AS \`__hdx_value\`), 1 AS \`__hdx_series_idx\` FROM (WITH Source AS ( SELECT *, cityHash64(ScopeAttributes, ResourceAttributes, Attributes) AS AttributesHash @@ -796,7 +796,7 @@ exports[`renderChartConfig multi-series metric charts (composed query) composes `; exports[`renderChartConfig multi-series metric charts (composed query) pads group columns across gauge and histogram branch classes 1`] = ` -"SELECT anyOrNullIf(\`__hdx_value\`, \`__hdx_series_idx\` = 0) AS "avg(metric.alpha)", anyOrNullIf(\`__hdx_value\`, \`__hdx_series_idx\` = 1) AS "quantile(metric.latency)", * EXCEPT (\`__hdx_value\`, \`__hdx_series_idx\`) FROM (SELECT *, 0 AS \`__hdx_series_idx\`, [] AS \`group\` 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 * REPLACE (toFloat64(\`__hdx_value\`) AS \`__hdx_value\`), 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,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 ( + ) 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 toFloat64(\`__hdx_value\`) AS \`__hdx_value\`, NULL AS \`__hdx_group_pad_0\`, \`__hdx_time_bucket\`, 1 AS \`__hdx_series_idx\`, \`group\` FROM (WITH source AS ( SELECT MetricName, ExplicitBounds, @@ -917,7 +917,7 @@ group, `; 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)", * EXCEPT (\`__hdx_value\`, \`__hdx_series_idx\`) FROM (SELECT *, 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 * REPLACE (toFloat64(\`__hdx_value\`) AS \`__hdx_value\`), 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 *, 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 * REPLACE (toFloat64(\`__hdx_value\`) AS \`__hdx_value\`), 1 AS \`__hdx_series_idx\` FROM (WITH Source AS ( SELECT *, cityHash64(ScopeAttributes, ResourceAttributes, Attributes) AS AttributesHash diff --git a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts index ff02a1b5a8..7e04b06437 100644 --- a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts +++ b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts @@ -1050,7 +1050,7 @@ describe('queryChartConfig Integration Tests', () => { }); type MetricSelect = { - aggFn: 'avg' | 'sum' | 'quantile' | 'increase'; + aggFn: 'avg' | 'sum' | 'quantile' | 'increase' | 'count'; aggCondition: string; aggConditionLanguage: 'sql'; valueExpression: string; @@ -1081,6 +1081,28 @@ describe('queryChartConfig Integration Tests', () => { metricType: MetricsDataType.Sum, }); + const histQuantileSelect = ( + metricName: string, + level: number, + ): MetricSelect => ({ + aggFn: 'quantile', + level, + aggCondition: '', + aggConditionLanguage: 'sql', + valueExpression: 'Value', + metricName, + metricType: MetricsDataType.Histogram, + }); + + const histCountSelect = (metricName: string): MetricSelect => ({ + aggFn: 'count', + aggCondition: '', + aggConditionLanguage: 'sql', + valueExpression: '', + metricName, + metricType: MetricsDataType.Histogram, + }); + const baseConfig = ( overrides: Partial, ): ChartConfigWithOptDateRange => @@ -1681,6 +1703,87 @@ describe('queryChartConfig Integration Tests', () => { expectGap(col(histRowOut, 'avg(grpmix.gauge)')); }); + // Regression (HDX-5077 follow-up): series whose native value types have + // no least supertype — histogram quantile (Float64) vs histogram count + // (Int64) / scalar count (UInt64) — must still merge. Without per-branch + // Float64 normalization the UNION ALL either fails with NO_COMMON_TYPE + // (use_variant_as_common_type = 0) or produces Variant(Float64, Int64) + // columns (the modern default) that consumers don't classify as numeric. + // The meta assertions pin the normalized type directly (not just + // convertCHDataTypeToJSType, which also tolerates numeric Variants as a + // defensive layer) because a Variant here would break servers running + // with use_variant_as_common_type = 0 before any JS ever sees it. + const expectFloat64ValueColumns = ( + meta: Array<{ name: string; type: string }> | undefined, + expectedNames: string[], + ) => { + expectNumericValueColumns(meta, expectedNames); + for (const name of expectedNames) { + const column = meta?.find(m => m.name === name); + expect(['Float64', 'Nullable(Float64)']).toContain(column?.type); + } + }; + + it('merges histogram quantile (Float64) with histogram count (Int64)', async () => { + const result = await runConfig( + baseConfig({ + select: [ + histQuantileSelect('grpmix.latency', 0.5), + histCountSelect('grpmix.latency'), + ], + }), + ); + + expectFloat64ValueColumns(result.meta, [ + 'quantile(grpmix.latency)', + 'count(grpmix.latency)', + ]); + + const rows = rowsByBucket(result.data); + expect([...rows.keys()].sort()).toEqual([bucket(0), bucket(1)]); + + // Bucket 0 is the cumulative zero baseline: the count series emits a + // 0 delta row while the all-zero quantile emits no row (a gap). + expectGap(col(rows.get(bucket(0)), 'quantile(grpmix.latency)')); + expect(Number(col(rows.get(bucket(0)), 'count(grpmix.latency)'))).toBe(0); + + // Bucket 1 adds 10 observations in the (0, 10] bucket: p50 + // interpolates to 5 and the count delta is 10. + expect( + Number(col(rows.get(bucket(1)), 'quantile(grpmix.latency)')), + ).toBeCloseTo(5, 5); + expect(Number(col(rows.get(bucket(1)), 'count(grpmix.latency)'))).toBe( + 10, + ); + }); + + it('merges gauge avg (Float64) with histogram count (Int64)', async () => { + const result = await runConfig( + baseConfig({ + select: [ + gaugeSelect('grpmix.gauge'), + histCountSelect('grpmix.latency'), + ], + }), + ); + + expectFloat64ValueColumns(result.meta, [ + 'avg(grpmix.gauge)', + 'count(grpmix.latency)', + ]); + + const rows = rowsByBucket(result.data); + expect([...rows.keys()].sort()).toEqual([bucket(0), bucket(1)]); + + expectGap(col(rows.get(bucket(0)), 'avg(grpmix.gauge)')); + expect(Number(col(rows.get(bucket(0)), 'count(grpmix.latency)'))).toBe(0); + + expect(col(rows.get(bucket(1)), 'avg(grpmix.gauge)')).toBe(42); + expect(Number(col(rows.get(bucket(1)), 'count(grpmix.latency)'))).toBe( + 10, + ); + }); + it('merges grouped non-timeseries (table) rows on the group values', 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 bb76500086..d2a04164b2 100644 --- a/packages/common-utils/src/__tests__/renderChartConfig.test.ts +++ b/packages/common-utils/src/__tests__/renderChartConfig.test.ts @@ -3655,9 +3655,13 @@ describe('renderChartConfig', () => { // 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`'); + // wrappers keep the branch projection via SELECT * — normalizing only + // the value column to Float64 via REPLACE, so mixed-type series (e.g. + // Float64 quantile + Int64 count) never UNION into a Variant — and the + // outer pivot passes it through via * EXCEPT + GROUP BY ALL. + expect(sql).toContain( + 'SELECT * REPLACE (toFloat64(`__hdx_value`) AS `__hdx_value`), 0 AS `__hdx_series_idx`', + ); expect(sql).toContain( '* EXCEPT (`__hdx_value`, `__hdx_series_idx`) FROM', ); @@ -3730,8 +3734,15 @@ describe('renderChartConfig', () => { // column names win the union. expect(sql).toContain('[] AS `group`'); 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.indexOf( + 'SELECT * REPLACE (toFloat64(`__hdx_value`) AS `__hdx_value`), 0 AS `__hdx_series_idx`', + ), + ).toBeLessThan(sql.indexOf('NULL AS `__hdx_group_pad_0`')); + // The histogram branch re-projects the value column explicitly, with + // the same Float64 normalization. + expect(sql).toContain( + 'SELECT toFloat64(`__hdx_value`) AS `__hdx_value`, NULL AS `__hdx_group_pad_0`', ); expect(sql).toContain('GROUP BY ALL'); }); diff --git a/packages/common-utils/src/clickhouse/__tests__/index.test.ts b/packages/common-utils/src/clickhouse/__tests__/index.test.ts index a8bab44109..538208d327 100644 --- a/packages/common-utils/src/clickhouse/__tests__/index.test.ts +++ b/packages/common-utils/src/clickhouse/__tests__/index.test.ts @@ -123,4 +123,27 @@ describe('convertCHDataTypeToJSType', () => { it('should handle Nullable(Bool) as Bool', () => { expect(convertCHDataTypeToJSType('Nullable(Bool)')).toBe(JSDataType.Bool); }); + + // A UNION ALL over columns with no least supertype (e.g. Float64 and + // Int64) produces Variant(...) when the server runs with + // use_variant_as_common_type = 1. All-numeric variants chart as numbers. + it('should handle all-numeric Variant as Number', () => { + expect(convertCHDataTypeToJSType('Variant(Float64, Int64)')).toBe( + JSDataType.Number, + ); + expect(convertCHDataTypeToJSType('Variant(Int64, UInt64)')).toBe( + JSDataType.Number, + ); + expect(convertCHDataTypeToJSType('Variant(Float64, Nullable(Int32))')).toBe( + JSDataType.Number, + ); + }); + + it('should not classify mixed or non-numeric Variant', () => { + expect(convertCHDataTypeToJSType('Variant(Float64, String)')).toBeNull(); + expect( + convertCHDataTypeToJSType('Variant(String, Array(Int64))'), + ).toBeNull(); + expect(convertCHDataTypeToJSType('Variant()')).toBeNull(); + }); }); diff --git a/packages/common-utils/src/clickhouse/index.ts b/packages/common-utils/src/clickhouse/index.ts index f9d3e773f1..fc8694b424 100644 --- a/packages/common-utils/src/clickhouse/index.ts +++ b/packages/common-utils/src/clickhouse/index.ts @@ -94,6 +94,22 @@ export const convertCHDataTypeToJSType = ( return convertCHDataTypeToJSType(dataType.slice(15, -1)); } else if (dataType.startsWith('Nullable(')) { return convertCHDataTypeToJSType(dataType.slice(9, -1)); + } else if (dataType.startsWith('Variant(') && dataType.endsWith(')')) { + // A UNION ALL over branches whose column types have no least supertype + // (e.g. Float64 and Int64) unifies as Variant(...) when the server runs + // with use_variant_as_common_type = 1 (the modern default). Treat an + // all-numeric Variant as numeric so such columns still chart; mixed + // variants stay unclassified. + const memberTypes = splitAndTrimWithBracket(dataType.slice(8, -1)); + if ( + memberTypes.length > 0 && + memberTypes.every( + memberType => + convertCHDataTypeToJSType(memberType) === JSDataType.Number, + ) + ) { + return JSDataType.Number; + } } return null; diff --git a/packages/common-utils/src/core/renderChartConfig.ts b/packages/common-utils/src/core/renderChartConfig.ts index a5dd1128b7..983cc61ea9 100644 --- a/packages/common-utils/src/core/renderChartConfig.ts +++ b/packages/common-utils/src/core/renderChartConfig.ts @@ -2355,15 +2355,27 @@ async function renderMultiSeriesMetricChartConfig( // 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. + // + // Every wrapper normalizes the value column to Float64. Different + // aggregations produce different native types — quantile is Float64 while + // histogram count is Int64 and scalar count() is UInt64 — and Int64/UInt64 + // have no least supertype with Float64, so a mixed-type UNION ALL either + // fails with NO_COMMON_TYPE (use_variant_as_common_type = 0) or silently + // unifies as Variant(Float64, Int64) (the modern default), which the + // anyOrNullIf pivot then propagates to every output column and no consumer + // classifies as numeric. toFloat64 makes the union type deterministic + // (Float64, or Nullable(Float64) when a branch value is Nullable) and + // matches the JS-number semantics of the old node-side merge. const orderedBranches = [ ...branches.filter(branch => !branch.isHistogram), ...branches.filter(branch => branch.isHistogram), ]; + const normalizedValueColumn = `toFloat64(\`${MULTI_SERIES_VALUE_ALIAS}\`) AS \`${MULTI_SERIES_VALUE_ALIAS}\``; 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}\``); + columns.push(normalizedValueColumn); if (hasScalarGroups) { for (let j = 0; j < scalarGroupCount; j++) { columns.push(`NULL AS \`__hdx_group_pad_${j}\``); @@ -2377,7 +2389,7 @@ async function renderMultiSeriesMetricChartConfig( columns.push(`\`${GROUP_ALIAS}\``); } } else { - columns.push('*'); + columns.push(`* REPLACE (${normalizedValueColumn})`); columns.push(idxColumn); if (hasHistogramGroup) { // Empty-array pad ([] is Array(Nothing), the supertype-compatible