Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/fix-multi-series-mixed-value-types.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
105 changes: 104 additions & 1 deletion packages/common-utils/src/__tests__/queryChartConfig.int.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>,
): ChartConfigWithOptDateRange =>
Expand Down Expand Up @@ -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({
Expand Down
21 changes: 16 additions & 5 deletions packages/common-utils/src/__tests__/renderChartConfig.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
);
Expand Down Expand Up @@ -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');
});
Expand Down
23 changes: 23 additions & 0 deletions packages/common-utils/src/clickhouse/__tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
16 changes: 16 additions & 0 deletions packages/common-utils/src/clickhouse/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
) {
Comment thread
greptile-apps[bot] marked this conversation as resolved.
Resolved
return JSDataType.Number;
}
}

return null;
Expand Down
16 changes: 14 additions & 2 deletions packages/common-utils/src/core/renderChartConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}\``);
Expand All @@ -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
Expand Down
Loading