From 0cee0a174f9a36040faaa4aed1b7a479167afbe6 Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Mon, 8 Jun 2026 18:52:40 -0700 Subject: [PATCH 01/10] fix(charts): cap group-by time-series to top-N series to prevent OOM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit High-cardinality group-by time charts could pull hundreds of thousands of series into a single tile (only ~60 were ever drawn), zero-filling a dense buckets×series matrix in memory and OOMing the browser tab. Add an opt-in query-level `seriesLimit`: when set on a group-by + granularity chart, renderChartConfig emits a TopGroups CTE that keeps only the top-N series by max value in any bucket and restricts the outer query to those groups. The time-chart display path sets seriesLimit=60 (matching HARD_LINES_LIMIT); alerts and other renderChartConfig consumers leave it unset, so their series evaluation is unchanged. --- packages/app/src/ChartUtils.tsx | 12 ++ packages/app/src/HDXMultiSeriesTimeChart.tsx | 10 +- .../__tests__/queryChartConfig.int.test.ts | 75 +++++++++++ .../src/__tests__/renderChartConfig.test.ts | 85 +++++++++++++ .../src/core/renderChartConfig.ts | 119 +++++++++++++++++- packages/common-utils/src/types.ts | 8 ++ 6 files changed, 305 insertions(+), 4 deletions(-) diff --git a/packages/app/src/ChartUtils.tsx b/packages/app/src/ChartUtils.tsx index 544c6762d5..d005da5ea2 100644 --- a/packages/app/src/ChartUtils.tsx +++ b/packages/app/src/ChartUtils.tsx @@ -103,6 +103,13 @@ function getTimeChartDateRange( : getAlignedDateRange(dateRange, granularity); } +// Max number of series on a time chart. Used as both the render-time line cap +// (re-exported as `HARD_LINES_LIMIT`) and the query-time `seriesLimit` set in +// `convertToTimeChartConfig` below. Keep them equal so we never fetch series +// that can't be drawn — high-cardinality group-bys would otherwise return +// hundreds of thousands of series and OOM the tab. +export const MAX_TIME_CHART_SERIES = 60; + export function convertToTimeChartConfig( config: ChartConfigWithDateRange, ): ChartConfigWithDateRange { @@ -133,6 +140,11 @@ export function convertToTimeChartConfig( dateRangeEndInclusive, granularity, limit: { limit: 100000 }, + // Cap high-cardinality group-by series at the query level so a single + // tile can't pull (and zero-fill) hundreds of thousands of series into + // memory. Only applies to group-by + granularity queries; ignored + // otherwise. See renderSeriesLimitCte in common-utils. + seriesLimit: MAX_TIME_CHART_SERIES, } : { ...config, diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 1a08dc1785..2745da8aae 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -29,7 +29,11 @@ import { ChartTooltipContainer, ChartTooltipItem, } from './components/charts/ChartTooltip'; -import { LineData, toStartOfInterval } from './ChartUtils'; +import { + LineData, + MAX_TIME_CHART_SERIES, + toStartOfInterval, +} from './ChartUtils'; import { FormatTime, useFormatTime } from './useFormatTime'; import styles from '../styles/HDXLineChart.module.scss'; @@ -317,7 +321,9 @@ const LegendRenderer = memo<{ ); }); -export const HARD_LINES_LIMIT = 60; +// Render-time cap on drawn series. Kept equal to the query-time series cap +// (see MAX_TIME_CHART_SERIES) so we never fetch series that can't be drawn. +export const HARD_LINES_LIMIT = MAX_TIME_CHART_SERIES; const StackedBarWithOverlap = (props: BarProps) => { const { x, y, width, height, fill } = props; diff --git a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts index 65e09e8d42..ffecc1ec7a 100644 --- a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts +++ b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts @@ -172,4 +172,79 @@ describe('queryChartConfig Integration Tests', () => { expect(metaNames).toContain('__hdx_time_bucket'); expect(metaNames.indexOf('__hdx_time_bucket')).toBeGreaterThanOrEqual(3); }); + + // Validates the seriesLimit cap end-to-end against real ClickHouse: the + // generated TopGroups CTE must be valid SQL and must restrict a + // high-cardinality group-by to the top N series by max value in any bucket. + it('caps high-cardinality group-by series to the top N via seriesLimit', async () => { + const SERIES_TABLE = 'logs_series_limit_int_test'; + await client.command({ + query: `CREATE OR REPLACE TABLE ${DATABASE}.${SERIES_TABLE} ( + Timestamp DateTime CODEC(ZSTD(1)), + ServiceName String CODEC(ZSTD(1)), + Value Float64 CODEC(ZSTD(1)) + ) ENGINE = MergeTree ORDER BY (ServiceName, Timestamp)`, + }); + + // 50 distinct series; Value == index so the top 5 by value are svc-45..49. + const rows = Array.from({ length: 50 }, (_, i) => ({ + Timestamp: '2025-04-15 00:10:00', + ServiceName: `svc-${String(i).padStart(2, '0')}`, + Value: i, + })); + await client.insert({ + table: `${DATABASE}.${SERIES_TABLE}`, + values: rows, + format: 'JSONEachRow', + }); + + try { + const config: ChartConfigWithOptDateRange = { + displayType: DisplayType.Line, + connection: 'test-connection', + from: { databaseName: DATABASE, tableName: SERIES_TABLE }, + select: [ + { + aggFn: 'max', + aggCondition: '', + aggConditionLanguage: 'sql', + valueExpression: 'Value', + }, + ], + groupBy: [{ aggCondition: '', valueExpression: 'ServiceName' }], + where: '', + whereLanguage: 'sql', + timestampValueExpression: 'Timestamp', + dateRange: [ + new Date('2025-04-15T00:00:00Z'), + new Date('2025-04-15T01:00:00Z'), + ], + granularity: '5 minute', + seriesLimit: 5, + }; + + const result = await hdxClient.queryChartConfig({ + config, + metadata, + querySettings: undefined, + }); + + // Without the cap this would be 50 distinct services. + const services = new Set( + (result.data as Array<{ ServiceName: string }>).map(r => r.ServiceName), + ); + expect(services.size).toBeLessThanOrEqual(5); + expect([...services].sort()).toEqual([ + 'svc-45', + 'svc-46', + 'svc-47', + 'svc-48', + 'svc-49', + ]); + } finally { + await client.command({ + query: `DROP TABLE IF EXISTS ${DATABASE}.${SERIES_TABLE}`, + }); + } + }); }); diff --git a/packages/common-utils/src/__tests__/renderChartConfig.test.ts b/packages/common-utils/src/__tests__/renderChartConfig.test.ts index 9816c0f935..632fe03a3c 100644 --- a/packages/common-utils/src/__tests__/renderChartConfig.test.ts +++ b/packages/common-utils/src/__tests__/renderChartConfig.test.ts @@ -469,6 +469,91 @@ describe('renderChartConfig', () => { expect(actual).not.toContain('TopGroups'); }); + describe('seriesLimit (group-by series cap)', () => { + const baseLogsConfig: ChartConfigWithOptDateRange = { + displayType: DisplayType.Line, + connection: 'test-connection', + from: { databaseName: 'default', tableName: 'logs' }, + select: [{ aggFn: 'count', aggCondition: '', valueExpression: '' }], + groupBy: [{ aggCondition: '', valueExpression: 'ServiceName' }], + where: '', + whereLanguage: 'sql', + timestampValueExpression: 'timestamp', + dateRange: [new Date('2025-02-12'), new Date('2025-02-13')], + granularity: '5 minute', + }; + + it('restricts to the top N group-by series via a CTE when seriesLimit is set', async () => { + const sql = parameterizedQueryToSql( + await renderChartConfig( + { ...baseLogsConfig, seriesLimit: 60 }, + mockMetadata, + querySettings, + ), + ); + // A ranking CTE keeps the top N groups by max value in any bucket. + expect(sql).toContain('__hdx_series_limit'); + expect(sql).toMatch(/ORDER\s+BY\s+max\(`__hdx_series_rank`\)\s+DESC/); + expect(sql).toContain('LIMIT 60'); + // The outer query is restricted to those groups via an IN subquery. + expect(sql).toMatch( + /tuple\(ServiceName\)\s+IN\s*\(\s*SELECT\s+`group`\s+FROM\s+`__hdx_series_limit`\)/, + ); + // NULL/empty groups are excluded from the ranking. + expect(sql).toMatch( + /ServiceName\s+IS\s+NOT\s+NULL\s+AND\s+toString\(ServiceName\)\s*!=\s*''/, + ); + }); + + it('does not emit a series-limit CTE when seriesLimit is unset (e.g. alert evaluation)', async () => { + const sql = parameterizedQueryToSql( + await renderChartConfig(baseLogsConfig, mockMetadata, querySettings), + ); + expect(sql).not.toContain('__hdx_series_limit'); + }); + + it('does not emit a series-limit CTE without a group-by', async () => { + const sql = parameterizedQueryToSql( + await renderChartConfig( + { ...baseLogsConfig, groupBy: undefined, seriesLimit: 60 }, + mockMetadata, + querySettings, + ), + ); + expect(sql).not.toContain('__hdx_series_limit'); + }); + + it('does not emit a series-limit CTE without granularity', async () => { + const sql = parameterizedQueryToSql( + await renderChartConfig( + { ...baseLogsConfig, granularity: undefined, seriesLimit: 60 }, + mockMetadata, + querySettings, + ), + ); + expect(sql).not.toContain('__hdx_series_limit'); + }); + + it('packs a multi-column group-by into a tuple for the series-limit CTE', async () => { + const sql = parameterizedQueryToSql( + await renderChartConfig( + { + ...baseLogsConfig, + groupBy: [ + { aggCondition: '', valueExpression: 'ServiceName' }, + { aggCondition: '', valueExpression: 'TraceId' }, + ], + seriesLimit: 60, + }, + mockMetadata, + querySettings, + ), + ); + expect(sql).toContain('__hdx_series_limit'); + expect(sql).toMatch(/tuple\(\s*ServiceName\s*,\s*TraceId\s*\)/); + }); + }); + it('should throw when aggFn=increase is used on a non-Sum metric', async () => { const config: ChartConfigWithOptDateRange = { displayType: DisplayType.Line, diff --git a/packages/common-utils/src/core/renderChartConfig.ts b/packages/common-utils/src/core/renderChartConfig.ts index 1ae4198c1b..61258a5d4f 100644 --- a/packages/common-utils/src/core/renderChartConfig.ts +++ b/packages/common-utils/src/core/renderChartConfig.ts @@ -1210,6 +1210,97 @@ async function renderGroupBy( ); } +// Cap the number of distinct group-by series for time-series charts. When +// `chartConfig.seriesLimit` is set, build a CTE that ranks groups by the +// chart's first aggregate (max over buckets, so a series that spikes in any +// single bucket still qualifies) and keeps only the top N, then return a +// predicate restricting the outer query to those groups. Returns undefined +// when the cap does not apply (no seriesLimit, no groupBy/granularity, a CTE or +// metric source, or a non-structured select), leaving the query unchanged. +// +// NULL/empty group values are excluded from the ranking so they neither +// dominate the chart as a single '-' series nor break the NULL-unsafe +// `tuple() IN (...)` comparison. Mirrors the aggFn='increase' TopGroups path. +async function renderSeriesLimitCte( + chartConfig: BuilderChartConfigWithOptDateRangeEx, + metadata: Metadata, + { + from, + where, + groupBy, + }: { from: ChSql; where: ChSql; groupBy: ChSql | undefined }, +): Promise<{ cte: ChSql; predicate: ChSql } | undefined> { + const { seriesLimit } = chartConfig; + if ( + seriesLimit == null || + !isUsingGroupBy(chartConfig) || + !isUsingGranularity(chartConfig) || + chartConfig.selectGroupBy === false || + // Only a real source table can be re-scanned for ranking; skip CTE/metric + // sources, whose `from` is a CTE name with no databaseName. + !chartConfig.from?.databaseName || + !chartConfig.from?.tableName || + // Ranking needs a structured aggregate to rank by. + !Array.isArray(chartConfig.select) || + chartConfig.select.length === 0 || + groupBy == null + ) { + return undefined; + } + + const groupByColsRendered = await renderSelectList( + chartConfig.groupBy, + chartConfig, + metadata, + ); + const groupByCols = Array.isArray(groupByColsRendered) + ? groupByColsRendered + : [groupByColsRendered]; + const groupByTuple = concatChSql(',', groupByCols); + + // Rank by the chart's first aggregate. Strip its alias so we can apply our + // own internal rank alias instead. + const firstSelect = chartConfig.select[0]; + const rankSelectList = + typeof firstSelect === 'string' + ? firstSelect + : [{ ...firstSelect, alias: undefined }]; + const rankRendered = await renderSelectList( + rankSelectList, + chartConfig, + metadata, + ); + const rankValue = Array.isArray(rankRendered) + ? rankRendered[0] + : rankRendered; + + const groupByEmptyFilter = concatChSql( + ' AND ', + groupByCols.map(g => chSql`(${g} IS NOT NULL AND toString(${g}) != '')`), + ); + const innerWhere = where.sql + ? concatChSql(' AND ', where, groupByEmptyFilter) + : groupByEmptyFilter; + + // Per-(group, bucket) aggregate, then max per group, keeping the top N. + const cte = chSql`\`__hdx_series_limit\` AS ( + SELECT \`group\` + FROM ( + SELECT tuple(${groupByTuple}) AS \`group\`, ${rankValue} AS \`__hdx_series_rank\` + FROM ${from} + WHERE ${innerWhere} + GROUP BY ${groupBy} + ) + GROUP BY \`group\` + ORDER BY max(\`__hdx_series_rank\`) DESC, \`group\` + LIMIT ${{ Int32: seriesLimit }} + )`; + + const predicate = chSql`tuple(${groupByTuple}) IN (SELECT \`group\` FROM \`__hdx_series_limit\`)`; + + return { cte, predicate }; +} + async function renderHaving( chartConfig: BuilderChartConfigWithOptDateRangeEx, metadata: Metadata, @@ -2065,7 +2156,11 @@ export async function renderChartConfig( : undefined), }; - const withClauses = await renderWith(chartConfig, metadata, querySettings); + const baseWithClauses = await renderWith( + chartConfig, + metadata, + querySettings, + ); const select = await renderSelect(chartConfig, metadata); const from = renderFrom(chartConfig); const where = await renderWhere(chartConfig, metadata); @@ -2076,11 +2171,31 @@ export async function renderChartConfig( const limit = renderLimit(chartConfig); const settings = renderSettings(chartConfig, querySettings); + // Opt-in: cap group-by series to the top N for time-series charts. The + // ranking CTE is merged into the rendered `withClauses` (not + // `chartConfig.with`) so the main SELECT keeps its materialized-column + // optimization, and the predicate is AND-ed into the outer WHERE. + const seriesLimit = await renderSeriesLimitCte(chartConfig, metadata, { + from, + where, + groupBy, + }); + const withClauses = seriesLimit + ? baseWithClauses + ? concatChSql(',', baseWithClauses, seriesLimit.cte) + : seriesLimit.cte + : baseWithClauses; + const finalWhere = seriesLimit + ? where.sql + ? concatChSql(' AND ', where, seriesLimit.predicate) + : seriesLimit.predicate + : where; + return concatChSql(' ', [ chSql`${withClauses?.sql ? chSql`WITH ${withClauses}` : ''}`, chSql`SELECT ${select}`, chSql`FROM ${from}`, - chSql`${where.sql ? chSql`WHERE ${where}` : ''}`, + chSql`${finalWhere.sql ? chSql`WHERE ${finalWhere}` : ''}`, chSql`${groupBy?.sql ? chSql`GROUP BY ${groupBy}` : ''}`, chSql`${having?.sql ? chSql`HAVING ${having}` : ''}`, chSql`${orderBy?.sql ? chSql`ORDER BY ${orderBy}` : ''}`, diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index 95c0f157df..60cc4e6579 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -275,6 +275,14 @@ export const SelectSQLStatementSchema = z.object({ havingLanguage: SearchConditionLanguageSchema.optional(), orderBy: SortSpecificationListSchema.optional(), limit: LimitSchema.optional(), + // Opt-in cap on the number of distinct group-by series returned for + // time-series (granularity) charts. When set, only the top-N series (ranked + // by max value in any bucket) are returned, via a TopGroups CTE in + // renderChartConfig. Only the chart-display path sets this; alerts and other + // consumers leave it unset so every series is evaluated. Prevents unbounded + // memory from high-cardinality group-bys (e.g. hundreds of thousands of + // series in one tile). + seriesLimit: z.number().int().positive().optional(), }); export type SQLInterval = z.infer; From 094729b7b61dd1c204c2adc68f9472f208f65962 Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Mon, 8 Jun 2026 19:15:48 -0700 Subject: [PATCH 02/10] feat(charts): make the time-chart series cap configurable per team MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a `seriesLimit` team setting alongside the existing ClickHouse client settings so teams can tune the top-N series cap that bounds time-chart memory. `convertToTimeChartConfig` now reads `me.team.seriesLimit`, falling back to the default (60) when unset and floored at 1. No upper bound — teams may intentionally fetch more series than the 60 rendered at once (the surplus is available in the series selector). The setting flows through the existing team-settings pipeline (Zod schema, Mongoose model, /me, PATCH /team/clickhouse-settings, ClickhouseSettingForm); no controller or endpoint changes were needed. --- packages/api/src/models/team.ts | 1 + packages/app/src/ChartUtils.tsx | 22 ++++++++----- packages/app/src/__tests__/ChartUtils.test.ts | 31 +++++++++++++++++++ packages/app/src/components/DBTimeChart.tsx | 9 ++++-- .../src/components/SearchTotalCountChart.tsx | 8 ++--- .../TeamSettings/TeamQueryConfigSection.tsx | 11 +++++++ packages/app/src/defaults.ts | 4 +++ packages/common-utils/src/types.ts | 6 ++++ 8 files changed, 78 insertions(+), 14 deletions(-) diff --git a/packages/api/src/models/team.ts b/packages/api/src/models/team.ts index 4d6b620162..15e92ec72c 100644 --- a/packages/api/src/models/team.ts +++ b/packages/api/src/models/team.ts @@ -42,6 +42,7 @@ export default mongoose.model( fieldMetadataDisabled: Boolean, parallelizeWhenPossible: Boolean, filterKeysFetchLimit: Number, + seriesLimit: Number, }, { timestamps: true, diff --git a/packages/app/src/ChartUtils.tsx b/packages/app/src/ChartUtils.tsx index d005da5ea2..6065d53827 100644 --- a/packages/app/src/ChartUtils.tsx +++ b/packages/app/src/ChartUtils.tsx @@ -37,6 +37,7 @@ import { notifications } from '@mantine/notifications'; import DateRangeIndicator from './components/charts/DateRangeIndicator'; import { MVOptimizationExplanationResult } from './hooks/useMVOptimizationExplanation'; +import { DEFAULT_SERIES_LIMIT } from './defaults'; import { getMetricNameSql } from './otelSemanticConventions'; import { AggFn, TableChartSeries, TimeChartSeries } from './types'; import { NumberFormat } from './types'; @@ -103,16 +104,23 @@ function getTimeChartDateRange( : getAlignedDateRange(dateRange, granularity); } -// Max number of series on a time chart. Used as both the render-time line cap -// (re-exported as `HARD_LINES_LIMIT`) and the query-time `seriesLimit` set in -// `convertToTimeChartConfig` below. Keep them equal so we never fetch series -// that can't be drawn — high-cardinality group-bys would otherwise return -// hundreds of thousands of series and OOM the tab. -export const MAX_TIME_CHART_SERIES = 60; +// Default max number of series on a time chart. Used as both the render-time +// line cap (re-exported as `HARD_LINES_LIMIT`) and the default query-time +// `seriesLimit` set in `convertToTimeChartConfig` below. Keep them equal so we +// never fetch series that can't be drawn — high-cardinality group-bys would +// otherwise return hundreds of thousands of series and OOM the tab. Teams can +// override the query-time cap via the `seriesLimit` team setting. +export const MAX_TIME_CHART_SERIES = DEFAULT_SERIES_LIMIT; export function convertToTimeChartConfig( config: ChartConfigWithDateRange, + // Optional per-team override for the query-time series cap. Falls back to the + // default when unset; floored at 1 so a stray non-positive value can't emit + // LIMIT 0. No upper bound — teams may intentionally render many series. + teamSeriesLimit?: number, ): ChartConfigWithDateRange { + const seriesLimit = Math.max(1, teamSeriesLimit ?? MAX_TIME_CHART_SERIES); + const granularity = getTimeChartGranularity( config.granularity, config.dateRange, @@ -144,7 +152,7 @@ export function convertToTimeChartConfig( // tile can't pull (and zero-fill) hundreds of thousands of series into // memory. Only applies to group-by + granularity queries; ignored // otherwise. See renderSeriesLimitCte in common-utils. - seriesLimit: MAX_TIME_CHART_SERIES, + seriesLimit, } : { ...config, diff --git a/packages/app/src/__tests__/ChartUtils.test.ts b/packages/app/src/__tests__/ChartUtils.test.ts index 2d3d5834ed..ab9acf58e7 100644 --- a/packages/app/src/__tests__/ChartUtils.test.ts +++ b/packages/app/src/__tests__/ChartUtils.test.ts @@ -11,6 +11,7 @@ import { formatResponseForPieChart, formatResponseForTimeChart, } from '@/ChartUtils'; +import { DEFAULT_SERIES_LIMIT } from '@/defaults'; import { COLORS } from '@/utils'; // Anchor info/error to concrete hexes rather than `getChartColorInfo()` / @@ -806,6 +807,36 @@ describe('ChartUtils', () => { expect(granularityFromFunction).toBe('5 minute'); }); + + const seriesLimitConfig = { + granularity: '5 minute', + dateRange: [ + new Date('2025-11-26T00:00:00Z'), + new Date('2025-11-27T00:00:00Z'), + ], + } as BuilderChartConfigWithDateRange; + + // seriesLimit lives on the builder member of the ChartConfigWithDateRange + // union, so narrow the result before reading it. + const seriesLimitOf = (teamSeriesLimit?: number) => + ( + convertToTimeChartConfig( + seriesLimitConfig, + teamSeriesLimit, + ) as BuilderChartConfigWithDateRange + ).seriesLimit; + + it('defaults seriesLimit to DEFAULT_SERIES_LIMIT when no team value is given', () => { + expect(seriesLimitOf()).toBe(DEFAULT_SERIES_LIMIT); + }); + + it('uses the team seriesLimit when provided', () => { + expect(seriesLimitOf(5)).toBe(5); + }); + + it('passes a large team seriesLimit through unbounded', () => { + expect(seriesLimitOf(100000)).toBe(100000); + }); }); describe('convertToNumberChartConfig', () => { diff --git a/packages/app/src/components/DBTimeChart.tsx b/packages/app/src/components/DBTimeChart.tsx index c0f2b26616..7944f4812d 100644 --- a/packages/app/src/components/DBTimeChart.tsx +++ b/packages/app/src/components/DBTimeChart.tsx @@ -287,9 +287,13 @@ function DBTimeChartComponent({ fillNulls, } = useTimeChartSettings(config); + // Read team settings before building the queried config so the per-team + // series cap can be threaded into convertToTimeChartConfig. + const { data: me, isLoading: isLoadingMe } = api.useMe(); + const queriedConfig = useMemo( - () => convertToTimeChartConfig(config), - [config], + () => convertToTimeChartConfig(config, me?.team?.seriesLimit), + [config, me?.team?.seriesLimit], ); // Determine whether the config can be optimized with an MV, to determine whether @@ -299,7 +303,6 @@ function DBTimeChartComponent({ const { data: mvOptimizationData } = useMVOptimizationExplanation(builderQueriedConfig); - const { data: me, isLoading: isLoadingMe } = api.useMe(); const { data, isLoading, isError, error, isPlaceholderData, isSuccess } = useQueriedChartConfig(queriedConfig, { placeholderData: (prev: any) => prev, diff --git a/packages/app/src/components/SearchTotalCountChart.tsx b/packages/app/src/components/SearchTotalCountChart.tsx index edceee9741..a6be03f89d 100644 --- a/packages/app/src/components/SearchTotalCountChart.tsx +++ b/packages/app/src/components/SearchTotalCountChart.tsx @@ -35,13 +35,13 @@ export function useSearchTotalCount( enableParallelQueries?: boolean; } = {}, ) { + const { data: me, isLoading: isLoadingMe } = api.useMe(); + // queriedConfig, queryKey, and enableQueryChunking match DBTimeChart so that react query can de-dupe these queries. const queriedConfig = useMemo( - () => convertToTimeChartConfig(config), - [config], + () => convertToTimeChartConfig(config, me?.team?.seriesLimit), + [config, me?.team?.seriesLimit], ); - - const { data: me, isLoading: isLoadingMe } = api.useMe(); const { data: totalCountData, isLoading, diff --git a/packages/app/src/components/TeamSettings/TeamQueryConfigSection.tsx b/packages/app/src/components/TeamSettings/TeamQueryConfigSection.tsx index de850d2d6f..36eeef04f8 100644 --- a/packages/app/src/components/TeamSettings/TeamQueryConfigSection.tsx +++ b/packages/app/src/components/TeamSettings/TeamQueryConfigSection.tsx @@ -24,6 +24,7 @@ import { DEFAULT_FILTER_KEYS_FETCH_LIMIT_WITH_MVS, DEFAULT_QUERY_TIMEOUT, DEFAULT_SEARCH_ROW_LIMIT, + DEFAULT_SERIES_LIMIT, } from '@/defaults'; import { useBrandDisplayName } from '@/theme/ThemeProvider'; @@ -343,6 +344,16 @@ export default function TeamQueryConfigSection() { type="boolean" displayValue={value => (value ? 'Enabled' : 'Disabled')} /> + diff --git a/packages/app/src/defaults.ts b/packages/app/src/defaults.ts index 55bf3dafc2..6c2fbac670 100644 --- a/packages/app/src/defaults.ts +++ b/packages/app/src/defaults.ts @@ -5,6 +5,10 @@ export const DEFAULT_SEARCH_ROW_LIMIT = 200; export const DEFAULT_QUERY_TIMEOUT = 60; // max_execution_time, seconds export const DEFAULT_FILTER_KEYS_FETCH_LIMIT = 20; export const DEFAULT_FILTER_KEYS_FETCH_LIMIT_WITH_MVS = 100; +// Max group-by series fetched per time chart (top-N query cap). Matches the +// render-time line cap (HARD_LINES_LIMIT) so we never fetch series that can't +// be drawn. Overridable per team via the `seriesLimit` setting. +export const DEFAULT_SERIES_LIMIT = 60; export function searchChartConfigDefaults( team: any | undefined | null, diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index 60cc4e6579..ac8df8309e 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -1530,6 +1530,11 @@ export const TeamClickHouseSettingsSchema = z.object({ metadataMaxRowsToRead: z.number().optional(), parallelizeWhenPossible: z.boolean().optional(), filterKeysFetchLimit: z.number().optional(), + // Max number of distinct group-by series fetched per time chart. Caps memory + // on high-cardinality group-bys (applied as a top-N query cap in + // renderChartConfig via the chart config's `seriesLimit`). Unset falls back to + // the client default. + seriesLimit: z.number().int().positive().optional(), }); /** Accepts null to unset (reset to default) a setting. */ @@ -1540,6 +1545,7 @@ export const TeamClickHouseSettingsUpdateSchema = z.object({ metadataMaxRowsToRead: z.number().nullish(), parallelizeWhenPossible: z.boolean().nullish(), filterKeysFetchLimit: z.number().nullish(), + seriesLimit: z.number().int().positive().nullish(), }); export type TeamClickHouseSettingsUpdate = z.infer< typeof TeamClickHouseSettingsUpdateSchema From b08ef0f034ebcf36f13f6d9e4b09425b6175023b Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Tue, 9 Jun 2026 08:09:57 -0700 Subject: [PATCH 03/10] chore: add changeset for time-chart series limit --- .changeset/time-chart-series-limit.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/time-chart-series-limit.md diff --git a/.changeset/time-chart-series-limit.md b/.changeset/time-chart-series-limit.md new file mode 100644 index 0000000000..1d1f428073 --- /dev/null +++ b/.changeset/time-chart-series-limit.md @@ -0,0 +1,7 @@ +--- +"@hyperdx/common-utils": patch +"@hyperdx/app": patch +"@hyperdx/api": patch +--- + +feat(charts): cap group-by time charts to a top-N series limit to prevent browser memory exhaustion on high-cardinality group-bys. The cap defaults to 60 (the number of series rendered) and is configurable per team via a new "Time Chart Series Limit" setting; series beyond the cap remain available in the series selector. From d6844427d35a40ff20a8b4a6a57011d60d094eab Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Tue, 9 Jun 2026 08:58:08 -0700 Subject: [PATCH 04/10] refactor(charts): simplify series-cap WITH/WHERE merge in renderChartConfig MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the `baseWithClauses` rename and two nested ternaries with a plain `let withClauses`/`let where` plus a single `if (seriesCap)` fold. Behaviour is unchanged — the ranking CTE is still appended to the already-rendered WITH clause (rather than chartConfig.with, which would disable the main SELECT's materialized-column optimization). Byte-identical SQL; all snapshots unchanged. --- .../src/core/renderChartConfig.ts | 39 ++++++++----------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/packages/common-utils/src/core/renderChartConfig.ts b/packages/common-utils/src/core/renderChartConfig.ts index 61258a5d4f..4548c047f2 100644 --- a/packages/common-utils/src/core/renderChartConfig.ts +++ b/packages/common-utils/src/core/renderChartConfig.ts @@ -2156,14 +2156,10 @@ export async function renderChartConfig( : undefined), }; - const baseWithClauses = await renderWith( - chartConfig, - metadata, - querySettings, - ); + let withClauses = await renderWith(chartConfig, metadata, querySettings); const select = await renderSelect(chartConfig, metadata); const from = renderFrom(chartConfig); - const where = await renderWhere(chartConfig, metadata); + let where = await renderWhere(chartConfig, metadata); const groupBy = await renderGroupBy(chartConfig, metadata); const having = await renderHaving(chartConfig, metadata); const orderBy = renderOrderBy(chartConfig); @@ -2171,31 +2167,30 @@ export async function renderChartConfig( const limit = renderLimit(chartConfig); const settings = renderSettings(chartConfig, querySettings); - // Opt-in: cap group-by series to the top N for time-series charts. The - // ranking CTE is merged into the rendered `withClauses` (not - // `chartConfig.with`) so the main SELECT keeps its materialized-column - // optimization, and the predicate is AND-ed into the outer WHERE. - const seriesLimit = await renderSeriesLimitCte(chartConfig, metadata, { + // Opt-in: cap group-by series to the top N for time-series charts. Fold the + // ranking CTE into the already-rendered `withClauses` (not `chartConfig.with`, + // which would disable the main SELECT's materialized-column optimization) and + // AND its predicate into the outer WHERE. renderSeriesLimitCte uses the + // pre-predicate `where` for its inner ranking query, so compute it first. + const seriesCap = await renderSeriesLimitCte(chartConfig, metadata, { from, where, groupBy, }); - const withClauses = seriesLimit - ? baseWithClauses - ? concatChSql(',', baseWithClauses, seriesLimit.cte) - : seriesLimit.cte - : baseWithClauses; - const finalWhere = seriesLimit - ? where.sql - ? concatChSql(' AND ', where, seriesLimit.predicate) - : seriesLimit.predicate - : where; + if (seriesCap) { + withClauses = withClauses + ? concatChSql(',', withClauses, seriesCap.cte) + : seriesCap.cte; + where = where.sql + ? concatChSql(' AND ', where, seriesCap.predicate) + : seriesCap.predicate; + } return concatChSql(' ', [ chSql`${withClauses?.sql ? chSql`WITH ${withClauses}` : ''}`, chSql`SELECT ${select}`, chSql`FROM ${from}`, - chSql`${finalWhere.sql ? chSql`WHERE ${finalWhere}` : ''}`, + chSql`${where.sql ? chSql`WHERE ${where}` : ''}`, chSql`${groupBy?.sql ? chSql`GROUP BY ${groupBy}` : ''}`, chSql`${having?.sql ? chSql`HAVING ${having}` : ''}`, chSql`${orderBy?.sql ? chSql`ORDER BY ${orderBy}` : ''}`, From ebd652a822fa941a5dd7893022f7de1c1e126d20 Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Tue, 9 Jun 2026 09:18:46 -0700 Subject: [PATCH 05/10] fix(charts): split comma-separated string group-by in series-cap CTE A multi-column string group-by (e.g. "LogAttributes['cap'],ServiceName") made the series-cap ranking CTE emit an invalid two-argument toString() and a malformed NULL check, because renderSelectList returns a comma-joined string as a single expression. Split it into per-column expressions with splitAndTrimWithBracket (which respects []/()/quotes) so the NULL/empty filter applies per column. Array and single-column-string group-bys are unchanged (snapshots unchanged); covered by a renderChartConfig unit test and a real-CH integration test mirroring the failing Map-access case. --- .../__tests__/queryChartConfig.int.test.ts | 79 +++++++++++++++++++ .../src/__tests__/renderChartConfig.test.ts | 27 +++++++ .../src/core/renderChartConfig.ts | 26 ++++-- 3 files changed, 124 insertions(+), 8 deletions(-) diff --git a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts index ffecc1ec7a..65504f45fc 100644 --- a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts +++ b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts @@ -247,4 +247,83 @@ describe('queryChartConfig Integration Tests', () => { }); } }); + + // Regression: a comma-separated *string* group-by (including a Map access with + // a comma inside the brackets) must split into per-column NULL/empty checks + // rather than emitting an invalid two-argument toString(). + it('handles a multi-column string group-by (with Map access) under seriesLimit', async () => { + const TABLE = 'logs_string_gb_int_test'; + await client.command({ + query: `CREATE OR REPLACE TABLE ${DATABASE}.${TABLE} ( + Timestamp DateTime CODEC(ZSTD(1)), + LogAttributes Map(String, String) CODEC(ZSTD(1)), + ServiceName String CODEC(ZSTD(1)) + ) ENGINE = MergeTree ORDER BY (ServiceName, Timestamp)`, + }); + + const ts = '2025-04-15 00:10:00'; + const rows = [ + // capA/svc1 (5 rows), capB/svc2 (3 rows) — the two non-empty series. + ...Array.from({ length: 5 }, () => ({ + Timestamp: ts, + LogAttributes: { 'agentToServer.capabilities': 'capA' }, + ServiceName: 'svc1', + })), + ...Array.from({ length: 3 }, () => ({ + Timestamp: ts, + LogAttributes: { 'agentToServer.capabilities': 'capB' }, + ServiceName: 'svc2', + })), + // Missing capability key (Map access -> '') for svc3 — largest by count, + // but must be excluded by the per-column empty filter. + ...Array.from({ length: 10 }, () => ({ + Timestamp: ts, + LogAttributes: {}, + ServiceName: 'svc3', + })), + ]; + await client.insert({ + table: `${DATABASE}.${TABLE}`, + values: rows, + format: 'JSONEachRow', + }); + + try { + const config: ChartConfigWithOptDateRange = { + displayType: DisplayType.Line, + connection: 'test-connection', + from: { databaseName: DATABASE, tableName: TABLE }, + select: [{ aggFn: 'count', aggCondition: '', valueExpression: '' }], + // Comma-separated string group-by — the shape that previously errored. + groupBy: "LogAttributes['agentToServer.capabilities'],ServiceName", + where: '', + whereLanguage: 'sql', + timestampValueExpression: 'Timestamp', + dateRange: [ + new Date('2025-04-15T00:00:00Z'), + new Date('2025-04-15T01:00:00Z'), + ], + granularity: '5 minute', + seriesLimit: 5, + }; + + // The query must execute without a ClickHouse error (the original bug). + const result = await hdxClient.queryChartConfig({ + config, + metadata, + querySettings: undefined, + }); + + const services = new Set( + (result.data as Array<{ ServiceName: string }>).map(r => r.ServiceName), + ); + // svc3 only appears with an empty capability, so it is filtered out of the + // ranking; only the two real series remain. + expect([...services].sort()).toEqual(['svc1', 'svc2']); + } finally { + await client.command({ + query: `DROP TABLE IF EXISTS ${DATABASE}.${TABLE}`, + }); + } + }); }); diff --git a/packages/common-utils/src/__tests__/renderChartConfig.test.ts b/packages/common-utils/src/__tests__/renderChartConfig.test.ts index 632fe03a3c..d3ef3f167f 100644 --- a/packages/common-utils/src/__tests__/renderChartConfig.test.ts +++ b/packages/common-utils/src/__tests__/renderChartConfig.test.ts @@ -552,6 +552,33 @@ describe('renderChartConfig', () => { expect(sql).toContain('__hdx_series_limit'); expect(sql).toMatch(/tuple\(\s*ServiceName\s*,\s*TraceId\s*\)/); }); + + it('splits a comma-separated string group-by into per-column null checks', async () => { + const sql = parameterizedQueryToSql( + await renderChartConfig( + { + ...baseLogsConfig, + groupBy: "LogAttributes['agentToServer.capabilities'],ServiceName", + seriesLimit: 60, + }, + mockMetadata, + querySettings, + ), + ); + expect(sql).toContain('__hdx_series_limit'); + // Each column gets its own NULL/empty check (split on the top-level comma, + // not the comma inside Map['...']). + expect(sql).toMatch( + /toString\(LogAttributes\[['"]agentToServer\.capabilities['"]\]\)\s*!=\s*''/, + ); + expect(sql).toMatch(/toString\(ServiceName\)\s*!=\s*''/); + // Regression: must NOT emit a two-argument toString of both columns. + expect(sql).not.toMatch(/toString\([^)]*,[^)]*ServiceName[^)]*\)/); + // Both columns are packed into the tuple for the IN predicate. + expect(sql).toMatch( + /tuple\(\s*LogAttributes\[['"]agentToServer\.capabilities['"]\]\s*,\s*ServiceName\s*\)\s+IN\s*\(\s*SELECT\s+`group`\s+FROM\s+`__hdx_series_limit`\)/, + ); + }); }); it('should throw when aggFn=increase is used on a non-Sum metric', async () => { diff --git a/packages/common-utils/src/core/renderChartConfig.ts b/packages/common-utils/src/core/renderChartConfig.ts index 4548c047f2..cd74d6146b 100644 --- a/packages/common-utils/src/core/renderChartConfig.ts +++ b/packages/common-utils/src/core/renderChartConfig.ts @@ -1248,14 +1248,24 @@ async function renderSeriesLimitCte( return undefined; } - const groupByColsRendered = await renderSelectList( - chartConfig.groupBy, - chartConfig, - metadata, - ); - const groupByCols = Array.isArray(groupByColsRendered) - ? groupByColsRendered - : [groupByColsRendered]; + // Render one ChSql per group-by column. `groupBy` may be an array of columns + // or a single comma-separated string; the per-column NULL/empty filter below + // needs the columns split out (else it would emit toString(col1, col2) and a + // malformed NULL check). splitAndTrimWithBracket respects []/()/quotes so it + // won't split inside Map['a,b'] or f(a, b). + let groupByCols: ChSql[]; + if (typeof chartConfig.groupBy === 'string') { + groupByCols = splitAndTrimWithBracket(chartConfig.groupBy).map( + col => chSql`${{ UNSAFE_RAW_SQL: col }}`, + ); + } else { + const rendered = await renderSelectList( + chartConfig.groupBy, + chartConfig, + metadata, + ); + groupByCols = Array.isArray(rendered) ? rendered : [rendered]; + } const groupByTuple = concatChSql(',', groupByCols); // Rank by the chart's first aggregate. Strip its alias so we can apply our From db29cf65a41be7c92d440d51e6d27c96602fa8ec Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Tue, 9 Jun 2026 09:33:53 -0700 Subject: [PATCH 06/10] fix(charts): only exclude NULL (not empty) group components from series cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The series-cap ranking CTE previously dropped both NULL and empty-string group values. Empty-string groups (e.g. a missing Map key like LogAttributes['x'] -> '') are real data, so silently hiding them was surprising. Now only NULL components are excluded — which is the genuine technical need, since the outer `tuple(...) IN (...)` is NULL-unsafe (transform_null_in=0) and would otherwise waste a top-N slot on a group it can't match. Empty-string groups now compete for a slot like any other value. --- .../__tests__/queryChartConfig.int.test.ts | 18 +++++++++------- .../src/__tests__/renderChartConfig.test.ts | 21 ++++++++++--------- .../src/core/renderChartConfig.ts | 18 +++++++++------- 3 files changed, 32 insertions(+), 25 deletions(-) diff --git a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts index 65504f45fc..c9a6d6554a 100644 --- a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts +++ b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts @@ -249,8 +249,9 @@ describe('queryChartConfig Integration Tests', () => { }); // Regression: a comma-separated *string* group-by (including a Map access with - // a comma inside the brackets) must split into per-column NULL/empty checks - // rather than emitting an invalid two-argument toString(). + // a comma inside the brackets) must split into per-column NULL checks rather + // than emitting an invalid two-argument toString(). Also verifies that an + // empty-string group (a missing Map key) is kept and competes for a top-N slot. it('handles a multi-column string group-by (with Map access) under seriesLimit', async () => { const TABLE = 'logs_string_gb_int_test'; await client.command({ @@ -274,8 +275,8 @@ describe('queryChartConfig Integration Tests', () => { LogAttributes: { 'agentToServer.capabilities': 'capB' }, ServiceName: 'svc2', })), - // Missing capability key (Map access -> '') for svc3 — largest by count, - // but must be excluded by the per-column empty filter. + // Missing capability key (Map access -> '') for svc3 — largest by count. + // Empty-string groups are kept, so this ranks #1 and survives the cap. ...Array.from({ length: 10 }, () => ({ Timestamp: ts, LogAttributes: {}, @@ -304,7 +305,9 @@ describe('queryChartConfig Integration Tests', () => { new Date('2025-04-15T01:00:00Z'), ], granularity: '5 minute', - seriesLimit: 5, + // Cap to 2 so the ranking is observable: by count svc3 (10) > svc1 (5) + // > svc2 (3), so the top 2 are svc3 and svc1; svc2 is dropped. + seriesLimit: 2, }; // The query must execute without a ClickHouse error (the original bug). @@ -317,9 +320,8 @@ describe('queryChartConfig Integration Tests', () => { const services = new Set( (result.data as Array<{ ServiceName: string }>).map(r => r.ServiceName), ); - // svc3 only appears with an empty capability, so it is filtered out of the - // ranking; only the two real series remain. - expect([...services].sort()).toEqual(['svc1', 'svc2']); + // svc3 (empty capability) is kept and ranks #1; the cap drops svc2. + expect([...services].sort()).toEqual(['svc1', 'svc3']); } finally { await client.command({ query: `DROP TABLE IF EXISTS ${DATABASE}.${TABLE}`, diff --git a/packages/common-utils/src/__tests__/renderChartConfig.test.ts b/packages/common-utils/src/__tests__/renderChartConfig.test.ts index d3ef3f167f..479ebb5ec2 100644 --- a/packages/common-utils/src/__tests__/renderChartConfig.test.ts +++ b/packages/common-utils/src/__tests__/renderChartConfig.test.ts @@ -499,10 +499,10 @@ describe('renderChartConfig', () => { expect(sql).toMatch( /tuple\(ServiceName\)\s+IN\s*\(\s*SELECT\s+`group`\s+FROM\s+`__hdx_series_limit`\)/, ); - // NULL/empty groups are excluded from the ranking. - expect(sql).toMatch( - /ServiceName\s+IS\s+NOT\s+NULL\s+AND\s+toString\(ServiceName\)\s*!=\s*''/, - ); + // Groups with a NULL component are excluded; empty-string groups are kept + // (no `!= ''` check). + expect(sql).toMatch(/ServiceName\s+IS\s+NOT\s+NULL/); + expect(sql).not.toMatch(/toString\(ServiceName\)\s*!=\s*''/); }); it('does not emit a series-limit CTE when seriesLimit is unset (e.g. alert evaluation)', async () => { @@ -566,14 +566,15 @@ describe('renderChartConfig', () => { ), ); expect(sql).toContain('__hdx_series_limit'); - // Each column gets its own NULL/empty check (split on the top-level comma, - // not the comma inside Map['...']). + // Each column gets its own NULL check, split on the top-level comma — not + // the comma inside Map['...']. expect(sql).toMatch( - /toString\(LogAttributes\[['"]agentToServer\.capabilities['"]\]\)\s*!=\s*''/, + /LogAttributes\[['"]agentToServer\.capabilities['"]\]\s+IS\s+NOT\s+NULL/, ); - expect(sql).toMatch(/toString\(ServiceName\)\s*!=\s*''/); - // Regression: must NOT emit a two-argument toString of both columns. - expect(sql).not.toMatch(/toString\([^)]*,[^)]*ServiceName[^)]*\)/); + expect(sql).toMatch(/ServiceName\s+IS\s+NOT\s+NULL/); + // Regression: must NOT emit a two-argument toString of both columns (the + // original bug that prompted the split). + expect(sql).not.toMatch(/toString\([^)]*,/); // Both columns are packed into the tuple for the IN predicate. expect(sql).toMatch( /tuple\(\s*LogAttributes\[['"]agentToServer\.capabilities['"]\]\s*,\s*ServiceName\s*\)\s+IN\s*\(\s*SELECT\s+`group`\s+FROM\s+`__hdx_series_limit`\)/, diff --git a/packages/common-utils/src/core/renderChartConfig.ts b/packages/common-utils/src/core/renderChartConfig.ts index cd74d6146b..8c2000902f 100644 --- a/packages/common-utils/src/core/renderChartConfig.ts +++ b/packages/common-utils/src/core/renderChartConfig.ts @@ -1218,9 +1218,11 @@ async function renderGroupBy( // when the cap does not apply (no seriesLimit, no groupBy/granularity, a CTE or // metric source, or a non-structured select), leaving the query unchanged. // -// NULL/empty group values are excluded from the ranking so they neither -// dominate the chart as a single '-' series nor break the NULL-unsafe -// `tuple() IN (...)` comparison. Mirrors the aggFn='increase' TopGroups path. +// Groups with a NULL component are excluded from the ranking because the outer +// `tuple() IN (...)` is NULL-unsafe (transform_null_in=0, so NULL never matches) +// — ranking such a group would waste a top-N slot on rows the outer query can't +// select. Empty-string groups (e.g. a missing Map key) are kept: they are real +// data and compete for a slot like any other value. async function renderSeriesLimitCte( chartConfig: BuilderChartConfigWithOptDateRangeEx, metadata: Metadata, @@ -1284,13 +1286,15 @@ async function renderSeriesLimitCte( ? rankRendered[0] : rankRendered; - const groupByEmptyFilter = concatChSql( + // Exclude only NULL group components (see the note above); a no-op on + // non-nullable columns, which is the common case. + const groupByNotNullFilter = concatChSql( ' AND ', - groupByCols.map(g => chSql`(${g} IS NOT NULL AND toString(${g}) != '')`), + groupByCols.map(g => chSql`${g} IS NOT NULL`), ); const innerWhere = where.sql - ? concatChSql(' AND ', where, groupByEmptyFilter) - : groupByEmptyFilter; + ? concatChSql(' AND ', where, groupByNotNullFilter) + : groupByNotNullFilter; // Per-(group, bucket) aggregate, then max per group, keeping the top N. const cte = chSql`\`__hdx_series_limit\` AS ( From cc1904aa1705be7fd47b49b641ff5da1d7aa760a Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Tue, 9 Jun 2026 09:39:44 -0700 Subject: [PATCH 07/10] fix(charts): strip group-by aliases inside the series-cap CTE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a group-by item carried an alias, renderSelectList appended ` AS "alias"`, which then leaked into tuple(...) and `(... IS NOT NULL)` in the series-cap CTE — both invalid SQL there (the outer GROUP BY tolerates aliases, these positions do not). Strip the alias before building the tuple and null filter, matching how the rank value is already rendered. No alias is set on group-bys today, so this is a defensive fix; covered by a new unit test. --- .../src/__tests__/renderChartConfig.test.ts | 28 +++++++++++++++++++ .../src/core/renderChartConfig.ts | 5 +++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/packages/common-utils/src/__tests__/renderChartConfig.test.ts b/packages/common-utils/src/__tests__/renderChartConfig.test.ts index 479ebb5ec2..52badd36b3 100644 --- a/packages/common-utils/src/__tests__/renderChartConfig.test.ts +++ b/packages/common-utils/src/__tests__/renderChartConfig.test.ts @@ -553,6 +553,34 @@ describe('renderChartConfig', () => { expect(sql).toMatch(/tuple\(\s*ServiceName\s*,\s*TraceId\s*\)/); }); + it('strips group-by aliases inside the series-limit CTE tuple and null filter', async () => { + const sql = parameterizedQueryToSql( + await renderChartConfig( + { + ...baseLogsConfig, + groupBy: [ + { + aggCondition: '', + valueExpression: 'ServiceName', + alias: 'svc', + }, + ], + seriesLimit: 60, + }, + mockMetadata, + querySettings, + ), + ); + expect(sql).toContain('__hdx_series_limit'); + // tuple() and `IS NOT NULL` must use the bare expression, not `ServiceName + // AS "svc"` (which would be invalid SQL there). + expect(sql).toMatch( + /tuple\(ServiceName\)\s+IN\s*\(\s*SELECT\s+`group`\s+FROM\s+`__hdx_series_limit`\)/, + ); + expect(sql).not.toContain('tuple(ServiceName AS'); + expect(sql).not.toMatch(/ServiceName\s+AS\s+"svc"\s+IS\s+NOT\s+NULL/); + }); + it('splits a comma-separated string group-by into per-column null checks', async () => { const sql = parameterizedQueryToSql( await renderChartConfig( diff --git a/packages/common-utils/src/core/renderChartConfig.ts b/packages/common-utils/src/core/renderChartConfig.ts index 8c2000902f..190c2ddbc2 100644 --- a/packages/common-utils/src/core/renderChartConfig.ts +++ b/packages/common-utils/src/core/renderChartConfig.ts @@ -1261,8 +1261,11 @@ async function renderSeriesLimitCte( col => chSql`${{ UNSAFE_RAW_SQL: col }}`, ); } else { + // Strip any alias: these columns go inside tuple(...) and `IS NOT NULL`, + // where an `AS "alias"` suffix would be a syntax error (unlike the outer + // GROUP BY, where renderSelectList's alias output is valid). const rendered = await renderSelectList( - chartConfig.groupBy, + chartConfig.groupBy.map(col => ({ ...col, alias: undefined })), chartConfig, metadata, ); From 9d8bb396517c8fff73803fe67d89fc17eafc43e1 Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Tue, 9 Jun 2026 09:47:19 -0700 Subject: [PATCH 08/10] test(charts): cover series-cap corner cases (NULL, alias, multi-col array, metric gating) Add integration tests against real ClickHouse for the corner cases the recent series-cap fixes target but unit tests can't fully prove: - NULL group component excluded (NULL-unsafe `tuple() IN (...)` would otherwise waste the only top-N slot and yield an empty chart); - multi-column *array* group-by with an alias (proves the 2-col tuple()/IN executes and the alias is stripped inside the CTE yet preserved in output). Add a unit test that a metric source does not emit the series-cap CTE (gating). --- .../__tests__/queryChartConfig.int.test.ts | 138 ++++++++++++++++++ .../src/__tests__/renderChartConfig.test.ts | 17 +++ 2 files changed, 155 insertions(+) diff --git a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts index c9a6d6554a..7686228129 100644 --- a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts +++ b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts @@ -328,4 +328,142 @@ describe('queryChartConfig Integration Tests', () => { }); } }); + + // The series cap filters NULL group components out of the ranking. Otherwise + // a NULL-keyed group could take a top-N slot it can never fill, because the + // outer `tuple(...) IN (...)` is NULL-unsafe (NULL never matches). + it('excludes NULL group components from the series cap', async () => { + const TABLE = 'logs_nullable_gb_int_test'; + await client.command({ + query: `CREATE OR REPLACE TABLE ${DATABASE}.${TABLE} ( + Timestamp DateTime CODEC(ZSTD(1)), + Region Nullable(String) CODEC(ZSTD(1)) + ) ENGINE = MergeTree ORDER BY (Timestamp)`, + }); + + const ts = '2025-04-15 00:10:00'; + const rows = [ + // 'us' has 5 rows; the NULL-region group has 10 (the largest by count). + ...Array.from({ length: 5 }, () => ({ Timestamp: ts, Region: 'us' })), + ...Array.from({ length: 10 }, () => ({ Timestamp: ts, Region: null })), + ]; + await client.insert({ + table: `${DATABASE}.${TABLE}`, + values: rows, + format: 'JSONEachRow', + }); + + try { + const config: ChartConfigWithOptDateRange = { + displayType: DisplayType.Line, + connection: 'test-connection', + from: { databaseName: DATABASE, tableName: TABLE }, + select: [{ aggFn: 'count', aggCondition: '', valueExpression: '' }], + groupBy: [{ aggCondition: '', valueExpression: 'Region' }], + where: '', + whereLanguage: 'sql', + timestampValueExpression: 'Timestamp', + dateRange: [ + new Date('2025-04-15T00:00:00Z'), + new Date('2025-04-15T01:00:00Z'), + ], + granularity: '5 minute', + // Only one slot: without the NULL filter the (larger) NULL group would + // claim it and then match nothing, yielding an empty chart. + seriesLimit: 1, + }; + + const result = await hdxClient.queryChartConfig({ + config, + metadata, + querySettings: undefined, + }); + + const regions = (result.data as Array<{ Region: string | null }>).map( + r => r.Region, + ); + // 'us' (top non-null) takes the slot; the NULL group is excluded. + expect(new Set(regions)).toEqual(new Set(['us'])); + expect(regions).not.toContain(null); + } finally { + await client.command({ + query: `DROP TABLE IF EXISTS ${DATABASE}.${TABLE}`, + }); + } + }); + + // Multi-column *array* group-by where one column carries an alias. Proves the + // 2-column tuple()/IN executes and that the alias is stripped inside the CTE + // (a leaked `Region AS "reg"` inside tuple()/IS NOT NULL is a CH syntax error) + // while still being preserved as the output column name. + it('handles a multi-column array group-by with an alias under seriesLimit', async () => { + const TABLE = 'logs_array_alias_gb_int_test'; + await client.command({ + query: `CREATE OR REPLACE TABLE ${DATABASE}.${TABLE} ( + Timestamp DateTime CODEC(ZSTD(1)), + Region String CODEC(ZSTD(1)), + ServiceName String CODEC(ZSTD(1)), + Value Float64 CODEC(ZSTD(1)) + ) ENGINE = MergeTree ORDER BY (Timestamp)`, + }); + + const ts = '2025-04-15 00:10:00'; + const rows = [ + { Timestamp: ts, Region: 'us', ServiceName: 'svc1', Value: 10 }, + { Timestamp: ts, Region: 'eu', ServiceName: 'svc2', Value: 5 }, + { Timestamp: ts, Region: 'ap', ServiceName: 'svc3', Value: 1 }, + ]; + await client.insert({ + table: `${DATABASE}.${TABLE}`, + values: rows, + format: 'JSONEachRow', + }); + + try { + const config: ChartConfigWithOptDateRange = { + displayType: DisplayType.Line, + connection: 'test-connection', + from: { databaseName: DATABASE, tableName: TABLE }, + select: [ + { + aggFn: 'max', + aggCondition: '', + aggConditionLanguage: 'sql', + valueExpression: 'Value', + }, + ], + groupBy: [ + { aggCondition: '', valueExpression: 'Region', alias: 'reg' }, + { aggCondition: '', valueExpression: 'ServiceName' }, + ], + where: '', + whereLanguage: 'sql', + timestampValueExpression: 'Timestamp', + dateRange: [ + new Date('2025-04-15T00:00:00Z'), + new Date('2025-04-15T01:00:00Z'), + ], + granularity: '5 minute', + // Top 2 by max(Value): (us,svc1)=10 and (eu,svc2)=5; (ap,svc3)=1 dropped. + seriesLimit: 2, + }; + + const result = await hdxClient.queryChartConfig({ + config, + metadata, + querySettings: undefined, + }); + + const services = new Set( + (result.data as Array<{ ServiceName: string }>).map(r => r.ServiceName), + ); + expect(services).toEqual(new Set(['svc1', 'svc2'])); + // The alias survives in the output even though it is stripped in the CTE. + expect(result.meta?.some(m => m.name === 'reg')).toBe(true); + } finally { + await client.command({ + query: `DROP TABLE IF EXISTS ${DATABASE}.${TABLE}`, + }); + } + }); }); diff --git a/packages/common-utils/src/__tests__/renderChartConfig.test.ts b/packages/common-utils/src/__tests__/renderChartConfig.test.ts index 52badd36b3..13a588900c 100644 --- a/packages/common-utils/src/__tests__/renderChartConfig.test.ts +++ b/packages/common-utils/src/__tests__/renderChartConfig.test.ts @@ -608,6 +608,23 @@ describe('renderChartConfig', () => { /tuple\(\s*LogAttributes\[['"]agentToServer\.capabilities['"]\]\s*,\s*ServiceName\s*\)\s+IN\s*\(\s*SELECT\s+`group`\s+FROM\s+`__hdx_series_limit`\)/, ); }); + + it('does not emit a series-limit CTE for a metric source', async () => { + // Metric configs are rewritten to query a Bucketed CTE (no real source + // table to re-scan), so the cap is gated off even with seriesLimit set. + const sql = parameterizedQueryToSql( + await renderChartConfig( + { + ...gaugeConfiguration, + groupBy: [{ aggCondition: '', valueExpression: 'ServiceName' }], + seriesLimit: 60, + }, + mockMetadata, + querySettings, + ), + ); + expect(sql).not.toContain('__hdx_series_limit'); + }); }); it('should throw when aggFn=increase is used on a non-Sum metric', async () => { From 31306280b852732600583c6db54b9321ed6749cd Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Tue, 9 Jun 2026 11:21:43 -0700 Subject: [PATCH 09/10] feat(charts): raise default time-chart series limit to 100 Bumps DEFAULT_SERIES_LIMIT 60 -> 100. Since MAX_TIME_CHART_SERIES and HARD_LINES_LIMIT derive from it, this raises both the default query-time cap and the rendered-line cap (and ServicesDashboard's MAX_NUM_SERIES) to 100. Teams can still override the query cap via the seriesLimit setting. --- .changeset/time-chart-series-limit.md | 2 +- packages/app/src/defaults.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/time-chart-series-limit.md b/.changeset/time-chart-series-limit.md index 1d1f428073..27aae22282 100644 --- a/.changeset/time-chart-series-limit.md +++ b/.changeset/time-chart-series-limit.md @@ -4,4 +4,4 @@ "@hyperdx/api": patch --- -feat(charts): cap group-by time charts to a top-N series limit to prevent browser memory exhaustion on high-cardinality group-bys. The cap defaults to 60 (the number of series rendered) and is configurable per team via a new "Time Chart Series Limit" setting; series beyond the cap remain available in the series selector. +feat(charts): cap group-by time charts to a top-N series limit to prevent browser memory exhaustion on high-cardinality group-bys. The cap defaults to 100 (the number of series rendered) and is configurable per team via a new "Time Chart Series Limit" setting; series beyond the cap remain available in the series selector. diff --git a/packages/app/src/defaults.ts b/packages/app/src/defaults.ts index 6c2fbac670..4bb6541dce 100644 --- a/packages/app/src/defaults.ts +++ b/packages/app/src/defaults.ts @@ -8,7 +8,7 @@ export const DEFAULT_FILTER_KEYS_FETCH_LIMIT_WITH_MVS = 100; // Max group-by series fetched per time chart (top-N query cap). Matches the // render-time line cap (HARD_LINES_LIMIT) so we never fetch series that can't // be drawn. Overridable per team via the `seriesLimit` setting. -export const DEFAULT_SERIES_LIMIT = 60; +export const DEFAULT_SERIES_LIMIT = 100; export function searchChartConfigDefaults( team: any | undefined | null, From 072d3ad05bab61594c9a5a10ebd0da7cd2c6a5e3 Mon Sep 17 00:00:00 2001 From: Warren Lee <5959690+wrn14897@users.noreply.github.com> Date: Tue, 9 Jun 2026 11:41:49 -0700 Subject: [PATCH 10/10] chore(charts): remove non-essential comments and shorten series-limit tooltip Drop the explanatory comments added across the series-cap changes (keeping only the ones inside renderSeriesLimitCte), and simplify the "Time Chart Series Limit" tooltip to "Maximum number of series fetched per time chart." --- packages/app/src/ChartUtils.tsx | 13 ------ packages/app/src/HDXMultiSeriesTimeChart.tsx | 2 - packages/app/src/components/DBTimeChart.tsx | 2 - .../TeamSettings/TeamQueryConfigSection.tsx | 2 +- packages/app/src/defaults.ts | 3 -- .../__tests__/queryChartConfig.int.test.ts | 23 ++++------- .../src/core/renderChartConfig.ts | 41 ++++--------------- packages/common-utils/src/types.ts | 11 ----- 8 files changed, 18 insertions(+), 79 deletions(-) diff --git a/packages/app/src/ChartUtils.tsx b/packages/app/src/ChartUtils.tsx index 6065d53827..b284670182 100644 --- a/packages/app/src/ChartUtils.tsx +++ b/packages/app/src/ChartUtils.tsx @@ -104,19 +104,10 @@ function getTimeChartDateRange( : getAlignedDateRange(dateRange, granularity); } -// Default max number of series on a time chart. Used as both the render-time -// line cap (re-exported as `HARD_LINES_LIMIT`) and the default query-time -// `seriesLimit` set in `convertToTimeChartConfig` below. Keep them equal so we -// never fetch series that can't be drawn — high-cardinality group-bys would -// otherwise return hundreds of thousands of series and OOM the tab. Teams can -// override the query-time cap via the `seriesLimit` team setting. export const MAX_TIME_CHART_SERIES = DEFAULT_SERIES_LIMIT; export function convertToTimeChartConfig( config: ChartConfigWithDateRange, - // Optional per-team override for the query-time series cap. Falls back to the - // default when unset; floored at 1 so a stray non-positive value can't emit - // LIMIT 0. No upper bound — teams may intentionally render many series. teamSeriesLimit?: number, ): ChartConfigWithDateRange { const seriesLimit = Math.max(1, teamSeriesLimit ?? MAX_TIME_CHART_SERIES); @@ -148,10 +139,6 @@ export function convertToTimeChartConfig( dateRangeEndInclusive, granularity, limit: { limit: 100000 }, - // Cap high-cardinality group-by series at the query level so a single - // tile can't pull (and zero-fill) hundreds of thousands of series into - // memory. Only applies to group-by + granularity queries; ignored - // otherwise. See renderSeriesLimitCte in common-utils. seriesLimit, } : { diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 2745da8aae..d97285eda4 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -321,8 +321,6 @@ const LegendRenderer = memo<{ ); }); -// Render-time cap on drawn series. Kept equal to the query-time series cap -// (see MAX_TIME_CHART_SERIES) so we never fetch series that can't be drawn. export const HARD_LINES_LIMIT = MAX_TIME_CHART_SERIES; const StackedBarWithOverlap = (props: BarProps) => { diff --git a/packages/app/src/components/DBTimeChart.tsx b/packages/app/src/components/DBTimeChart.tsx index 7944f4812d..9ae6f1a54c 100644 --- a/packages/app/src/components/DBTimeChart.tsx +++ b/packages/app/src/components/DBTimeChart.tsx @@ -287,8 +287,6 @@ function DBTimeChartComponent({ fillNulls, } = useTimeChartSettings(config); - // Read team settings before building the queried config so the per-team - // series cap can be threaded into convertToTimeChartConfig. const { data: me, isLoading: isLoadingMe } = api.useMe(); const queriedConfig = useMemo( diff --git a/packages/app/src/components/TeamSettings/TeamQueryConfigSection.tsx b/packages/app/src/components/TeamSettings/TeamQueryConfigSection.tsx index 36eeef04f8..2cfc66c9b4 100644 --- a/packages/app/src/components/TeamSettings/TeamQueryConfigSection.tsx +++ b/packages/app/src/components/TeamSettings/TeamQueryConfigSection.tsx @@ -347,7 +347,7 @@ export default function TeamQueryConfigSection() { { expect(metaNames.indexOf('__hdx_time_bucket')).toBeGreaterThanOrEqual(3); }); - // Validates the seriesLimit cap end-to-end against real ClickHouse: the - // generated TopGroups CTE must be valid SQL and must restrict a - // high-cardinality group-by to the top N series by max value in any bucket. + // End-to-end: the cap CTE is valid SQL and restricts a high-cardinality + // group-by to the top N by max value in any bucket. it('caps high-cardinality group-by series to the top N via seriesLimit', async () => { const SERIES_TABLE = 'logs_series_limit_int_test'; await client.command({ @@ -248,10 +247,8 @@ describe('queryChartConfig Integration Tests', () => { } }); - // Regression: a comma-separated *string* group-by (including a Map access with - // a comma inside the brackets) must split into per-column NULL checks rather - // than emitting an invalid two-argument toString(). Also verifies that an - // empty-string group (a missing Map key) is kept and competes for a top-N slot. + // 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 () => { const TABLE = 'logs_string_gb_int_test'; await client.command({ @@ -329,9 +326,8 @@ describe('queryChartConfig Integration Tests', () => { } }); - // The series cap filters NULL group components out of the ranking. Otherwise - // a NULL-keyed group could take a top-N slot it can never fill, because the - // outer `tuple(...) IN (...)` is NULL-unsafe (NULL never matches). + // NULL group components are dropped from the ranking; otherwise a NULL group + // could take a slot the NULL-unsafe outer `tuple() IN (...)` can never fill. it('excludes NULL group components from the series cap', async () => { const TABLE = 'logs_nullable_gb_int_test'; await client.command({ @@ -392,10 +388,9 @@ describe('queryChartConfig Integration Tests', () => { } }); - // Multi-column *array* group-by where one column carries an alias. Proves the - // 2-column tuple()/IN executes and that the alias is stripped inside the CTE - // (a leaked `Region AS "reg"` inside tuple()/IS NOT NULL is a CH syntax error) - // while still being preserved as the output column name. + // Multi-column array group-by with an alias: the 2-column tuple()/IN executes + // (alias stripped in the CTE — a leaked `AS "reg"` there is a syntax error) + // and the alias is preserved as the output column. it('handles a multi-column array group-by with an alias under seriesLimit', async () => { const TABLE = 'logs_array_alias_gb_int_test'; await client.command({ diff --git a/packages/common-utils/src/core/renderChartConfig.ts b/packages/common-utils/src/core/renderChartConfig.ts index 190c2ddbc2..c25e985890 100644 --- a/packages/common-utils/src/core/renderChartConfig.ts +++ b/packages/common-utils/src/core/renderChartConfig.ts @@ -1210,19 +1210,6 @@ async function renderGroupBy( ); } -// Cap the number of distinct group-by series for time-series charts. When -// `chartConfig.seriesLimit` is set, build a CTE that ranks groups by the -// chart's first aggregate (max over buckets, so a series that spikes in any -// single bucket still qualifies) and keeps only the top N, then return a -// predicate restricting the outer query to those groups. Returns undefined -// when the cap does not apply (no seriesLimit, no groupBy/granularity, a CTE or -// metric source, or a non-structured select), leaving the query unchanged. -// -// Groups with a NULL component are excluded from the ranking because the outer -// `tuple() IN (...)` is NULL-unsafe (transform_null_in=0, so NULL never matches) -// — ranking such a group would waste a top-N slot on rows the outer query can't -// select. Empty-string groups (e.g. a missing Map key) are kept: they are real -// data and compete for a slot like any other value. async function renderSeriesLimitCte( chartConfig: BuilderChartConfigWithOptDateRangeEx, metadata: Metadata, @@ -1238,11 +1225,9 @@ async function renderSeriesLimitCte( !isUsingGroupBy(chartConfig) || !isUsingGranularity(chartConfig) || chartConfig.selectGroupBy === false || - // Only a real source table can be re-scanned for ranking; skip CTE/metric - // sources, whose `from` is a CTE name with no databaseName. + // Skip CTE/metric sources (no real table to re-scan) and string selects. !chartConfig.from?.databaseName || !chartConfig.from?.tableName || - // Ranking needs a structured aggregate to rank by. !Array.isArray(chartConfig.select) || chartConfig.select.length === 0 || groupBy == null @@ -1250,20 +1235,17 @@ async function renderSeriesLimitCte( return undefined; } - // Render one ChSql per group-by column. `groupBy` may be an array of columns - // or a single comma-separated string; the per-column NULL/empty filter below - // needs the columns split out (else it would emit toString(col1, col2) and a - // malformed NULL check). splitAndTrimWithBracket respects []/()/quotes so it - // won't split inside Map['a,b'] or f(a, b). + // One ChSql per group-by column (groupBy may be an array or a comma-separated + // string). splitAndTrimWithBracket respects []/()/quotes so it won't split + // inside Map['a,b']; the per-column null filter below needs them separated. let groupByCols: ChSql[]; if (typeof chartConfig.groupBy === 'string') { groupByCols = splitAndTrimWithBracket(chartConfig.groupBy).map( col => chSql`${{ UNSAFE_RAW_SQL: col }}`, ); } else { - // Strip any alias: these columns go inside tuple(...) and `IS NOT NULL`, - // where an `AS "alias"` suffix would be a syntax error (unlike the outer - // GROUP BY, where renderSelectList's alias output is valid). + // Strip aliases: these go inside tuple(...)/`IS NOT NULL`, where an + // `AS "alias"` suffix is a syntax error (unlike the outer GROUP BY). const rendered = await renderSelectList( chartConfig.groupBy.map(col => ({ ...col, alias: undefined })), chartConfig, @@ -1273,8 +1255,7 @@ async function renderSeriesLimitCte( } const groupByTuple = concatChSql(',', groupByCols); - // Rank by the chart's first aggregate. Strip its alias so we can apply our - // own internal rank alias instead. + // Rank by the chart's first aggregate (alias stripped — we add our own). const firstSelect = chartConfig.select[0]; const rankSelectList = typeof firstSelect === 'string' @@ -1289,8 +1270,7 @@ async function renderSeriesLimitCte( ? rankRendered[0] : rankRendered; - // Exclude only NULL group components (see the note above); a no-op on - // non-nullable columns, which is the common case. + // Drop NULL components only (no-op on non-nullable columns). const groupByNotNullFilter = concatChSql( ' AND ', groupByCols.map(g => chSql`${g} IS NOT NULL`), @@ -2184,11 +2164,6 @@ export async function renderChartConfig( const limit = renderLimit(chartConfig); const settings = renderSettings(chartConfig, querySettings); - // Opt-in: cap group-by series to the top N for time-series charts. Fold the - // ranking CTE into the already-rendered `withClauses` (not `chartConfig.with`, - // which would disable the main SELECT's materialized-column optimization) and - // AND its predicate into the outer WHERE. renderSeriesLimitCte uses the - // pre-predicate `where` for its inner ranking query, so compute it first. const seriesCap = await renderSeriesLimitCte(chartConfig, metadata, { from, where, diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index ac8df8309e..dc38e71b2f 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -275,13 +275,6 @@ export const SelectSQLStatementSchema = z.object({ havingLanguage: SearchConditionLanguageSchema.optional(), orderBy: SortSpecificationListSchema.optional(), limit: LimitSchema.optional(), - // Opt-in cap on the number of distinct group-by series returned for - // time-series (granularity) charts. When set, only the top-N series (ranked - // by max value in any bucket) are returned, via a TopGroups CTE in - // renderChartConfig. Only the chart-display path sets this; alerts and other - // consumers leave it unset so every series is evaluated. Prevents unbounded - // memory from high-cardinality group-bys (e.g. hundreds of thousands of - // series in one tile). seriesLimit: z.number().int().positive().optional(), }); @@ -1530,10 +1523,6 @@ export const TeamClickHouseSettingsSchema = z.object({ metadataMaxRowsToRead: z.number().optional(), parallelizeWhenPossible: z.boolean().optional(), filterKeysFetchLimit: z.number().optional(), - // Max number of distinct group-by series fetched per time chart. Caps memory - // on high-cardinality group-bys (applied as a top-N query cap in - // renderChartConfig via the chart config's `seriesLimit`). Unset falls back to - // the client default. seriesLimit: z.number().int().positive().optional(), });