From 0efa560e54086977e0abef3f80bf0c5baa001b45 Mon Sep 17 00:00:00 2001 From: aryainguz Date: Sat, 18 Jul 2026 17:40:26 +0530 Subject: [PATCH 1/7] refactor: compute metric ratios via native ClickHouse CTE --- .changeset/refactor-metric-ratios-cte.md | 5 ++ .../__tests__/queryChartConfig.int.test.ts | 60 ++++++++++++++++ packages/common-utils/src/clickhouse/index.ts | 70 +++++++++++++++++-- 3 files changed, 130 insertions(+), 5 deletions(-) create mode 100644 .changeset/refactor-metric-ratios-cte.md diff --git a/.changeset/refactor-metric-ratios-cte.md b/.changeset/refactor-metric-ratios-cte.md new file mode 100644 index 0000000000..ee88c0bfec --- /dev/null +++ b/.changeset/refactor-metric-ratios-cte.md @@ -0,0 +1,5 @@ +--- +"@hyperdx/common-utils": patch +--- + +Refactor metric ratios to use native ClickHouse CTE diff --git a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts index 5f76e73149..19a3eec441 100644 --- a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts +++ b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts @@ -247,6 +247,66 @@ describe('queryChartConfig Integration Tests', () => { } }); + it('computes ratio via native CTE when seriesReturnType is "ratio"', async () => { + const config: ChartConfigWithOptDateRange = { + displayType: DisplayType.Line, + connection: 'test-connection', + from: { databaseName: DATABASE, tableName: TABLE_NAME }, + metricTables: { [MetricsDataType.Gauge]: TABLE_NAME } as any, + seriesReturnType: 'ratio', + select: [ + { + aggFn: 'avg', + aggCondition: '', + aggConditionLanguage: 'sql', + valueExpression: 'Value', + metricName: 'metric.alpha', + metricType: MetricsDataType.Gauge, + alias: 'avg(metric.alpha)', + }, + { + aggFn: 'avg', + aggCondition: '', + aggConditionLanguage: 'sql', + valueExpression: 'Value', + metricName: 'metric.beta', + metricType: MetricsDataType.Gauge, + alias: 'avg(metric.beta)', + }, + ], + groupBy: [{ aggCondition: '', valueExpression: 'ServiceName' }], + where: '', + whereLanguage: 'sql', + timestampValueExpression: 'TimeUnix', + dateRange: [new Date('2025-04-14'), new Date('2025-04-16')], + granularity: '1 minute', + limit: { limit: 100 }, + }; + + const result = await hdxClient.queryChartConfig({ + config, + metadata, + querySettings: undefined, + }); + + const metaNames = result.meta?.map(m => m.name) ?? []; + + // Check that the ratio is the first column + expect(metaNames[0]).toBe('avg(metric.alpha)/avg(metric.beta)'); + expect(metaNames).toContain('__hdx_time_bucket'); + expect(metaNames).toContain('ServiceName'); + + const data = result.data as any[]; + expect(data.length).toBeGreaterThan(0); + for (const row of data) { + expect(row['avg(metric.alpha)/avg(metric.beta)']).toBeDefined(); + // It might be a number or string depending on ClickHouse formatting for JSON, usually number for Float64 + expect( + Number.isNaN(Number(row['avg(metric.alpha)/avg(metric.beta)'])), + ).toBe(false); + } + }); + // Regression: a comma-separated string group-by (with a Map access) must split // per-column (not emit toString(col1, col2)); empty-string groups are kept. it('handles a multi-column string group-by (with Map access) under seriesLimit', async () => { diff --git a/packages/common-utils/src/clickhouse/index.ts b/packages/common-utils/src/clickhouse/index.ts index 3123baec9a..b5c7ca4317 100644 --- a/packages/common-utils/src/clickhouse/index.ts +++ b/packages/common-utils/src/clickhouse/index.ts @@ -664,6 +664,67 @@ export abstract class BaseClickhouseClient { const isTimeSeries = config.displayType === 'line'; + if ( + isBuilderChartConfig(config) && + config.seriesReturnType === 'ratio' && + queries.length === 2 && + Array.isArray(config.select) + ) { + const q0Alias = config.select[0].alias ?? 'q0_val'; + const q1Alias = config.select[1].alias ?? 'q1_val'; + const ratioAlias = `${q0Alias}/${q1Alias}`; + + const joinKeys: string[] = []; + if (isTimeSeries) { + joinKeys.push('__hdx_time_bucket'); + } + + if (config.groupBy) { + if (Array.isArray(config.groupBy)) { + for (const gb of config.groupBy) { + if (typeof gb === 'string') { + joinKeys.push(gb); + } else { + joinKeys.push(gb.alias || gb.valueExpression); + } + } + } else if (typeof config.groupBy === 'string') { + joinKeys.push(...splitAndTrimWithBracket(config.groupBy)); + } + } + + // De-duplicate join keys just in case + const uniqueJoinKeys = Array.from(new Set(joinKeys)); + + let ratioSql: ChSql; + const selectCols = [ + chSql`(q0.${{ Identifier: q0Alias }} / q1.${{ Identifier: q1Alias }}) AS ${{ Identifier: ratioAlias }}`, + ...uniqueJoinKeys.map(k => chSql`${{ Identifier: k }}`), + ]; + const selectClause = concatChSql(', ', selectCols); + + if (uniqueJoinKeys.length > 0) { + const joinKeysSql = uniqueJoinKeys.map( + k => chSql`${{ Identifier: k }}`, + ); + const usingClause = concatChSql(', ', joinKeysSql); + ratioSql = chSql`WITH q0 AS (${queries[0]}), q1 AS (${queries[1]}) SELECT ${selectClause} FROM q0 ANY LEFT JOIN q1 USING (${usingClause})`; + } else { + ratioSql = chSql`WITH q0 AS (${queries[0]}), q1 AS (${queries[1]}) SELECT ${selectClause} FROM q0 CROSS JOIN q1`; + } + + const resp = await this.query<'JSON'>({ + query: ratioSql.sql, + query_params: ratioSql.params, + format: 'JSON', + abort_signal: opts?.abort_signal, + connectionId: config.connection, + clickhouse_settings: opts?.clickhouse_settings, + }); + + return resp.json(); + } + const resultSets = await Promise.all( queries.map(async query => { const resp = await this.query<'JSON'>({ @@ -734,15 +795,14 @@ export abstract class BaseClickhouseClient { } } - const isRatio = - config.seriesReturnType === 'ratio' && resultSets.length === 2; - const _resultSet: ResponseJSON = { meta: Array.from(metaSet.values()), data: Array.from(tsBucketMap.values()), }; - // TODO: we should compute the ratio on the db side - return isRatio ? computeResultSetRatio(_resultSet) : _resultSet; + + // Native DB-side ratio calculation intercepts the ratio chart before this block. + // This legacy join block now only applies to multi-metric line charts. + return _resultSet; } throw new Error('No result sets'); } From e5e3ed43c7a6be58b4a907179d6e60bf280d9bc1 Mon Sep 17 00:00:00 2001 From: aryainguz Date: Sat, 18 Jul 2026 19:00:09 +0530 Subject: [PATCH 2/7] refactor: lint --- packages/common-utils/src/clickhouse/index.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/common-utils/src/clickhouse/index.ts b/packages/common-utils/src/clickhouse/index.ts index c93cd99e67..0652c015f4 100644 --- a/packages/common-utils/src/clickhouse/index.ts +++ b/packages/common-utils/src/clickhouse/index.ts @@ -216,14 +216,14 @@ export const chSql = ( return { ...acc, ...(value == null || - typeof value === 'string' || - 'UNSAFE_RAW_SQL' in value + typeof value === 'string' || + 'UNSAFE_RAW_SQL' in value ? {} : Array.isArray(value) ? value.reduce((acc, v) => { - Object.assign(acc, v.params); - return acc; - }, {}) + Object.assign(acc, v.params); + return acc; + }, {}) : 'params' in value ? value.params : 'Identifier' in value @@ -559,8 +559,8 @@ export const mergeResultSets = ({ for (const row of resultSet.data) { const _rowWithoutValue = numericColumnName ? Object.fromEntries( - Object.entries(row).filter(([key]) => key !== numericColumnName), - ) + 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 @@ -1049,9 +1049,9 @@ function selectColumnsToAliasMap( aliasMap[column.as] = column.expr.array_index && column.expr.array_index[0]?.brackets ? // alias with brackets, ex: ResourceAttributes['service.name'] as service_name - `${column.expr.column.expr.value}['${column.expr.array_index[0].index.value}']` + `${column.expr.column.expr.value}['${column.expr.array_index[0].index.value}']` : // normal alias - column.expr.column.expr.value; + column.expr.column.expr.value; } else if (column.expr.loc != null) { aliasMap[column.as] = parsedSql.slice( column.expr.loc.start.offset, From 652d2dbc00fe253fa268845fce13b96ceee349eb Mon Sep 17 00:00:00 2001 From: aryainguz Date: Sat, 18 Jul 2026 19:13:25 +0530 Subject: [PATCH 3/7] refactor: standardize groupBy configuration and include raw counts in ratio select clause --- packages/common-utils/src/clickhouse/index.ts | 30 ++++++++++++++----- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/packages/common-utils/src/clickhouse/index.ts b/packages/common-utils/src/clickhouse/index.ts index 0652c015f4..c13b6c53d2 100644 --- a/packages/common-utils/src/clickhouse/index.ts +++ b/packages/common-utils/src/clickhouse/index.ts @@ -860,16 +860,30 @@ export abstract class BaseClickhouseClient { } if (config.groupBy) { - if (Array.isArray(config.groupBy)) { - for (const gb of config.groupBy) { + if (typeof config.groupBy === 'string') { + config.groupBy = splitAndTrimWithBracket(config.groupBy).map(gb => ({ + type: 'string', + valueExpression: gb, + alias: gb, // Assign the raw expression as the alias so the CTE outputs exactly this column name + })); + } else if (Array.isArray(config.groupBy)) { + config.groupBy = config.groupBy.map(gb => { if (typeof gb === 'string') { - joinKeys.push(gb); - } else { - joinKeys.push(gb.alias || gb.valueExpression); + return { type: 'string', valueExpression: gb, alias: gb }; } + if (!gb.alias) { + return { ...gb, alias: gb.valueExpression }; + } + return gb; + }); + } + + for (const gb of config.groupBy) { + if (typeof gb === 'string') { + joinKeys.push(gb); + } else { + joinKeys.push(gb.alias || gb.valueExpression); } - } else if (typeof config.groupBy === 'string') { - joinKeys.push(...splitAndTrimWithBracket(config.groupBy)); } } @@ -879,6 +893,8 @@ export abstract class BaseClickhouseClient { let ratioSql: ChSql; const selectCols = [ chSql`(q0.${{ Identifier: q0Alias }} / q1.${{ Identifier: q1Alias }}) AS ${{ Identifier: ratioAlias }}`, + chSql`q0.${{ Identifier: q0Alias }} AS ${{ Identifier: q0Alias }}`, + chSql`q1.${{ Identifier: q1Alias }} AS ${{ Identifier: q1Alias }}`, ...uniqueJoinKeys.map(k => chSql`${{ Identifier: k }}`), ]; const selectClause = concatChSql(', ', selectCols); From ae2a6a1e1e79c50f9df7c8dda864c4b2dd7ff3b7 Mon Sep 17 00:00:00 2001 From: aryainguz Date: Sat, 18 Jul 2026 19:20:02 +0530 Subject: [PATCH 4/7] fix: prevent alias collision in ratio-based builder charts by appending suffix to duplicate q1 aliases --- packages/common-utils/src/clickhouse/index.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/common-utils/src/clickhouse/index.ts b/packages/common-utils/src/clickhouse/index.ts index c13b6c53d2..d1a765eeb4 100644 --- a/packages/common-utils/src/clickhouse/index.ts +++ b/packages/common-utils/src/clickhouse/index.ts @@ -847,12 +847,17 @@ export abstract class BaseClickhouseClient { if ( isBuilderChartConfig(config) && config.seriesReturnType === 'ratio' && + config.ratioMode !== 'share_of_total' && queries.length === 2 && Array.isArray(config.select) ) { const q0Alias = config.select[0].alias ?? 'q0_val'; - const q1Alias = config.select[1].alias ?? 'q1_val'; - const ratioAlias = `${q0Alias}/${q1Alias}`; + const originalQ1Alias = config.select[1].alias ?? 'q1_val'; + let q1AliasOut = originalQ1Alias; + if (q0Alias === originalQ1Alias) { + q1AliasOut = `${originalQ1Alias}__1`; + } + const ratioAlias = `${q0Alias}/${originalQ1Alias}`; const joinKeys: string[] = []; if (isTimeSeries) { @@ -892,9 +897,9 @@ export abstract class BaseClickhouseClient { let ratioSql: ChSql; const selectCols = [ - chSql`(q0.${{ Identifier: q0Alias }} / q1.${{ Identifier: q1Alias }}) AS ${{ Identifier: ratioAlias }}`, + chSql`(q0.${{ Identifier: q0Alias }} / q1.${{ Identifier: originalQ1Alias }}) AS ${{ Identifier: ratioAlias }}`, chSql`q0.${{ Identifier: q0Alias }} AS ${{ Identifier: q0Alias }}`, - chSql`q1.${{ Identifier: q1Alias }} AS ${{ Identifier: q1Alias }}`, + chSql`q1.${{ Identifier: originalQ1Alias }} AS ${{ Identifier: q1AliasOut }}`, ...uniqueJoinKeys.map(k => chSql`${{ Identifier: k }}`), ]; const selectClause = concatChSql(', ', selectCols); From 66fcce0036151f3d1597dec609dd1f929a747e84 Mon Sep 17 00:00:00 2001 From: aryainguz Date: Sat, 18 Jul 2026 19:25:18 +0530 Subject: [PATCH 5/7] feat: move groupBy alias normalization logic into initial chart config processing for ratio-based queries --- packages/common-utils/src/clickhouse/index.ts | 46 ++++++++++--------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/packages/common-utils/src/clickhouse/index.ts b/packages/common-utils/src/clickhouse/index.ts index d1a765eeb4..6759243c9f 100644 --- a/packages/common-utils/src/clickhouse/index.ts +++ b/packages/common-utils/src/clickhouse/index.ts @@ -833,9 +833,30 @@ export abstract class BaseClickhouseClient { }; querySettings: QuerySettings | undefined; }): Promise>> { - config = isBuilderChartConfig(config) - ? setChartSelectsAlias(config) - : config; + if (isBuilderChartConfig(config)) { + config = setChartSelectsAlias(config); + if (config.seriesReturnType === 'ratio' && config.ratioMode !== 'share_of_total') { + if (config.groupBy) { + if (typeof config.groupBy === 'string') { + config.groupBy = splitAndTrimWithBracket(config.groupBy).map(gb => ({ + type: 'string', + valueExpression: gb, + alias: gb, // Assign the raw expression as the alias so the CTE outputs exactly this column name + })); + } else if (Array.isArray(config.groupBy)) { + config.groupBy = config.groupBy.map(gb => { + if (typeof gb === 'string') { + return { type: 'string', valueExpression: gb, alias: gb }; + } + if (!gb.alias) { + return { ...gb, alias: gb.valueExpression }; + } + return gb; + }); + } + } + } + } const queries: ChSql[] = await Promise.all( splitChartConfigs(config).map(c => renderChartConfig(c, metadata, querySettings), @@ -863,26 +884,7 @@ export abstract class BaseClickhouseClient { if (isTimeSeries) { joinKeys.push('__hdx_time_bucket'); } - if (config.groupBy) { - if (typeof config.groupBy === 'string') { - config.groupBy = splitAndTrimWithBracket(config.groupBy).map(gb => ({ - type: 'string', - valueExpression: gb, - alias: gb, // Assign the raw expression as the alias so the CTE outputs exactly this column name - })); - } else if (Array.isArray(config.groupBy)) { - config.groupBy = config.groupBy.map(gb => { - if (typeof gb === 'string') { - return { type: 'string', valueExpression: gb, alias: gb }; - } - if (!gb.alias) { - return { ...gb, alias: gb.valueExpression }; - } - return gb; - }); - } - for (const gb of config.groupBy) { if (typeof gb === 'string') { joinKeys.push(gb); From 1f714bb84db819b8040b891b1ebbf5f27d1b438c Mon Sep 17 00:00:00 2001 From: aryainguz Date: Sat, 18 Jul 2026 19:47:51 +0530 Subject: [PATCH 6/7] fix: handle null values in ratio calculations and update join logic to FULL OUTER JOIN in ClickHouse queries --- packages/common-utils/src/clickhouse/index.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/common-utils/src/clickhouse/index.ts b/packages/common-utils/src/clickhouse/index.ts index 6759243c9f..a5c6a8f37d 100644 --- a/packages/common-utils/src/clickhouse/index.ts +++ b/packages/common-utils/src/clickhouse/index.ts @@ -899,8 +899,8 @@ export abstract class BaseClickhouseClient { let ratioSql: ChSql; const selectCols = [ - chSql`(q0.${{ Identifier: q0Alias }} / q1.${{ Identifier: originalQ1Alias }}) AS ${{ Identifier: ratioAlias }}`, - chSql`q0.${{ Identifier: q0Alias }} AS ${{ Identifier: q0Alias }}`, + chSql`(COALESCE(q0.${{ Identifier: q0Alias }}, 0) / q1.${{ Identifier: originalQ1Alias }}) AS ${{ Identifier: ratioAlias }}`, + chSql`COALESCE(q0.${{ Identifier: q0Alias }}, 0) AS ${{ Identifier: q0Alias }}`, chSql`q1.${{ Identifier: originalQ1Alias }} AS ${{ Identifier: q1AliasOut }}`, ...uniqueJoinKeys.map(k => chSql`${{ Identifier: k }}`), ]; @@ -911,7 +911,7 @@ export abstract class BaseClickhouseClient { k => chSql`${{ Identifier: k }}`, ); const usingClause = concatChSql(', ', joinKeysSql); - ratioSql = chSql`WITH q0 AS (${queries[0]}), q1 AS (${queries[1]}) SELECT ${selectClause} FROM q0 ANY LEFT JOIN q1 USING (${usingClause})`; + ratioSql = chSql`WITH q0 AS (${queries[0]}), q1 AS (${queries[1]}) SELECT ${selectClause} FROM q0 FULL OUTER JOIN q1 USING (${usingClause})`; } else { ratioSql = chSql`WITH q0 AS (${queries[0]}), q1 AS (${queries[1]}) SELECT ${selectClause} FROM q0 CROSS JOIN q1`; } From 4c2d7f0e413d5a121eacbe9553254bcb0242f172 Mon Sep 17 00:00:00 2001 From: aryainguz Date: Sat, 18 Jul 2026 20:04:59 +0530 Subject: [PATCH 7/7] refactor: remove redundant alias handling --- packages/common-utils/src/clickhouse/index.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/common-utils/src/clickhouse/index.ts b/packages/common-utils/src/clickhouse/index.ts index a5c6a8f37d..75dd477ad6 100644 --- a/packages/common-utils/src/clickhouse/index.ts +++ b/packages/common-utils/src/clickhouse/index.ts @@ -874,10 +874,6 @@ export abstract class BaseClickhouseClient { ) { const q0Alias = config.select[0].alias ?? 'q0_val'; const originalQ1Alias = config.select[1].alias ?? 'q1_val'; - let q1AliasOut = originalQ1Alias; - if (q0Alias === originalQ1Alias) { - q1AliasOut = `${originalQ1Alias}__1`; - } const ratioAlias = `${q0Alias}/${originalQ1Alias}`; const joinKeys: string[] = []; @@ -900,8 +896,6 @@ export abstract class BaseClickhouseClient { let ratioSql: ChSql; const selectCols = [ chSql`(COALESCE(q0.${{ Identifier: q0Alias }}, 0) / q1.${{ Identifier: originalQ1Alias }}) AS ${{ Identifier: ratioAlias }}`, - chSql`COALESCE(q0.${{ Identifier: q0Alias }}, 0) AS ${{ Identifier: q0Alias }}`, - chSql`q1.${{ Identifier: originalQ1Alias }} AS ${{ Identifier: q1AliasOut }}`, ...uniqueJoinKeys.map(k => chSql`${{ Identifier: k }}`), ]; const selectClause = concatChSql(', ', selectCols);