diff --git a/.changeset/dashboard-tile-result-row-cap.md b/.changeset/dashboard-tile-result-row-cap.md new file mode 100644 index 0000000000..f6fdca958f --- /dev/null +++ b/.changeset/dashboard-tile-result-row-cap.md @@ -0,0 +1,35 @@ +--- +'@hyperdx/app': minor +--- + +Cap the number of rows a raw SQL dashboard tile query returns to protect the +browser from pathological high-cardinality queries. Raw SQL tiles rendered as a +line, stacked-bar, pie, or bar chart now run capped at 5,000 (+1 row of +headroom) via an order-preserving result-row cap (`max_result_rows` + +`result_overflow_mode = 'break'`, which trims the tail after the query's own +ORDER BY/LIMIT). Because that cap is block-aligned and weak on its own (a result +fitting in one block is returned whole), queries with no outer `LIMIT` also get +a group-by cardinality cap (`max_rows_to_group_by` + `group_by_overflow_mode = +'break'`) that bounds unique GROUP BY keys — the setting that actually protects +server memory. The cardinality cap is deliberately **skipped** for raw SQL that +carries its own outer `LIMIT`: it stops accumulating keys during aggregation, +before an outer `ORDER BY … LIMIT N` runs, so applying it there would compute the +top-N over an arbitrary key subset and render silently wrong values. (Builder +tiles already bound cardinality via the per-tile series limit, and builder tables +page rows separately, so they are unaffected.) When a query hits the cap, an +inline warning below the chart header notes the chart may be missing data and +nudges the user to narrow the query. + +To avoid a false warning when a complete result happens to be exactly the cap +size, the query runs with one row of headroom (`cap + 1`): a result of ≤ cap +rows comes back whole and is not flagged, while a larger result trips the cap and +is detected via the returned row count. Only the returned row count is used for +detection (never `rows_before_limit_at_least`), so a tile whose own SQL ends in a +`LIMIT` is not falsely flagged just because the pre-`LIMIT` aggregation was +large. Both caps are block-aligned (soft), so the returned result can overshoot +the cap by up to one block; because detection can't prove the server actually +dropped rows, the banner says the chart "may be missing data" rather than +asserting truncation. The warning also clears while a narrowed query is +in flight (it no longer lingers on stale placeholder data), and when a chart's +result is both row-capped and series-capped the "hidden series" notice no longer +claims all series were loaded. diff --git a/packages/app/src/DBDashboardPage.tsx b/packages/app/src/DBDashboardPage.tsx index d82c11217d..ca0e425a4f 100644 --- a/packages/app/src/DBDashboardPage.tsx +++ b/packages/app/src/DBDashboardPage.tsx @@ -144,6 +144,7 @@ import { useDashboards, useDeleteDashboard, } from '@/dashboard'; +import { resolveTileMaxResultRows } from '@/defaults'; import { useAlertAnnotations } from '@/hooks/useAlertAnnotations'; import useDashboardContainers, { TabDeleteAction, @@ -1100,6 +1101,11 @@ const Tile = forwardRef( } : undefined; + // Only raw SQL tiles get a row cap (see resolveTileMaxResultRows). + const tileMaxResultRows = resolveTileMaxResultRows( + effectiveQueriedConfig, + ); + // Markdown charts may not have queriedConfig, if config.source is not set const effectiveMarkdownConfig = effectiveQueriedConfig ?? chart.config; @@ -1145,6 +1151,7 @@ const Tile = forwardRef( enabled={chartEnabled} config={effectiveQueriedConfig} annotations={alertAnnotations} + maxResultRows={tileMaxResultRows} onTimeRangeSelect={ readOnly ? undefined @@ -1209,6 +1216,7 @@ const Tile = forwardRef( toolbarSuffix={toolbarSuffixItems} enabled={chartEnabled} config={effectiveQueriedConfig} + maxResultRows={tileMaxResultRows} /> )} {effectiveQueriedConfig?.displayType === DisplayType.Bar && ( @@ -1219,6 +1227,7 @@ const Tile = forwardRef( toolbarSuffix={toolbarSuffixItems} enabled={chartEnabled} config={effectiveQueriedConfig} + maxResultRows={tileMaxResultRows} /> )} {effectiveQueriedConfig?.displayType === DisplayType.Heatmap && diff --git a/packages/app/src/__tests__/defaults.test.ts b/packages/app/src/__tests__/defaults.test.ts new file mode 100644 index 0000000000..b9dabe4639 --- /dev/null +++ b/packages/app/src/__tests__/defaults.test.ts @@ -0,0 +1,298 @@ +import type { ChartConfigWithOptDateRange } from '@hyperdx/common-utils/dist/types'; + +import { + DEFAULT_MAX_TILE_RESULT_ROWS, + didResultOverflow, + hasOuterLimit, + resolveDidOverflow, + resolveMaxResultRowsValue, + resolveResultRowLimitSettings, + resolveTileMaxResultRows, +} from '@/defaults'; + +describe('resolveMaxResultRowsValue', () => { + it('requests one row of headroom above the cap', () => { + // cap + 1 lets a complete result of exactly `cap` rows come back whole, + // while anything larger trips the break. + expect(resolveMaxResultRowsValue(5000)).toBe(5001); + expect(resolveMaxResultRowsValue(1)).toBe(2); + }); + + it('floors non-integer caps before adding headroom', () => { + expect(resolveMaxResultRowsValue(99.9)).toBe(100); + }); + + it('returns undefined for a non-positive / absent cap (no limit applied)', () => { + expect(resolveMaxResultRowsValue(undefined)).toBeUndefined(); + expect(resolveMaxResultRowsValue(0)).toBeUndefined(); + expect(resolveMaxResultRowsValue(-10)).toBeUndefined(); + expect(resolveMaxResultRowsValue(Number.POSITIVE_INFINITY)).toBeUndefined(); + }); +}); + +describe('didResultOverflow', () => { + const cap = 5000; + + it('returns false when no cap is applied', () => { + expect(didResultOverflow({ rows: 999999, cap: undefined })).toBe(false); + expect(didResultOverflow({ rows: 999999, cap: 0 })).toBe(false); + expect( + didResultOverflow({ rows: 999999, cap: Number.POSITIVE_INFINITY }), + ).toBe(false); + }); + + it('does NOT flag a complete result of exactly the cap (no truncation)', () => { + // With cap + 1 headroom, a real result of exactly `cap` rows comes back + // whole — nothing was dropped, so no banner. + expect(didResultOverflow({ rows: cap, cap })).toBe(false); + }); + + it('detects overflow when rows exceed the cap', () => { + // break returns cap + 1 (block-aligned, possibly more) once the underlying + // result is larger than the cap. + expect(didResultOverflow({ rows: cap + 1, cap })).toBe(true); + expect(didResultOverflow({ rows: cap + 500, cap })).toBe(true); + }); + + it('does NOT flag when the query has its own LIMIT that trims a large aggregation', () => { + // Regression: a raw SQL tile ending in `... LIMIT 50` over a 6,250-group + // aggregation returns exactly 50 rows. ClickHouse reports + // rows_before_limit_at_least = 6250, but the tile received only 50 rows — + // nothing was truncated by our cap, so there must be no banner. We rely on + // the returned `rows` only, so this is inherently safe. + expect(didResultOverflow({ rows: 50, cap })).toBe(false); + }); + + it('returns false for a normal under-cap result', () => { + expect(didResultOverflow({ rows: 42, cap })).toBe(false); + expect(didResultOverflow({ rows: 0, cap })).toBe(false); + }); + + it('handles a missing row count gracefully', () => { + expect(didResultOverflow({ rows: undefined, cap })).toBe(false); + }); + + it('does not flag a NaN row count', () => { + // NaN > cap is false, so this is correct today; asserted to lock it in. + expect(didResultOverflow({ rows: Number.NaN, cap })).toBe(false); + }); + + it('exposes a sane default cap', () => { + expect(DEFAULT_MAX_TILE_RESULT_ROWS).toBe(5000); + }); +}); + +describe('resolveTileMaxResultRows', () => { + const rawSqlConfig = { + configType: 'sql', + sqlTemplate: 'SELECT 1', + connection: 'c', + } as unknown as ChartConfigWithOptDateRange; + const builderConfig = { + connection: 'c', + from: { databaseName: 'default', tableName: 't' }, + select: [{ aggFn: 'count', valueExpression: '' }], + where: '', + groupBy: 'k', + } as unknown as ChartConfigWithOptDateRange; + + it('caps raw SQL tiles at the default', () => { + expect(resolveTileMaxResultRows(rawSqlConfig)).toBe( + DEFAULT_MAX_TILE_RESULT_ROWS, + ); + }); + + it('does NOT cap builder tiles (they bound cardinality elsewhere)', () => { + expect(resolveTileMaxResultRows(builderConfig)).toBeUndefined(); + }); + + it('returns undefined for an absent config', () => { + expect(resolveTileMaxResultRows(undefined)).toBeUndefined(); + expect(resolveTileMaxResultRows(null)).toBeUndefined(); + }); +}); + +describe('hasOuterLimit', () => { + it('detects a trailing LIMIT (with optional offset / comma / semicolon / comment)', () => { + expect(hasOuterLimit('SELECT * FROM t LIMIT 50')).toBe(true); + expect(hasOuterLimit('SELECT * FROM t ORDER BY c DESC LIMIT 50')).toBe( + true, + ); + expect(hasOuterLimit('SELECT * FROM t limit 10')).toBe(true); // case-insensitive + expect(hasOuterLimit('SELECT * FROM t LIMIT 50;')).toBe(true); + expect(hasOuterLimit('SELECT * FROM t LIMIT 50 \n ')).toBe(true); + expect(hasOuterLimit('SELECT * FROM t LIMIT 10, 50')).toBe(true); // offset,count + expect(hasOuterLimit('SELECT * FROM t LIMIT 50 OFFSET 10')).toBe(true); + expect(hasOuterLimit('SELECT * FROM t LIMIT 50 -- trailing comment')).toBe( + true, + ); + }); + + it('returns false when there is no outer LIMIT', () => { + expect(hasOuterLimit('SELECT * FROM t')).toBe(false); + expect(hasOuterLimit('SELECT k, count() FROM t GROUP BY k')).toBe(false); + }); + + it('detects an outer LIMIT followed by trailing SETTINGS / FORMAT / WITH TIES', () => { + // ClickHouse allows these after LIMIT; missing them would wrongly enable the + // group-by cap and corrupt the top-N (greptile P1). + expect( + hasOuterLimit('SELECT * FROM t LIMIT 50 SETTINGS max_threads=4'), + ).toBe(true); + expect(hasOuterLimit('SELECT * FROM t LIMIT 50 SETTINGS a=1, b=2;')).toBe( + true, + ); + expect(hasOuterLimit('SELECT * FROM t LIMIT 50 FORMAT JSON')).toBe(true); + expect(hasOuterLimit('SELECT * FROM t ORDER BY c LIMIT 50 WITH TIES')).toBe( + true, + ); + expect( + hasOuterLimit( + 'SELECT * FROM t LIMIT 50 WITH TIES SETTINGS max_threads=4', + ), + ).toBe(true); + // SETTINGS wrapped onto multiple lines must still be consumed (greptile P1). + expect( + hasOuterLimit( + 'SELECT k, count() c FROM t GROUP BY k ORDER BY c DESC LIMIT 50 SETTINGS\n max_threads=4,\n max_memory_usage=1000', + ), + ).toBe(true); + }); + + it('detects an outer LIMIT followed by a trailing block comment', () => { + // /* ... */ after the LIMIT must not defeat detection (greptile P1). + expect( + hasOuterLimit( + 'SELECT k, count() c FROM t GROUP BY k ORDER BY c DESC LIMIT 50 /* dashboard note */', + ), + ).toBe(true); + expect(hasOuterLimit('SELECT * FROM t LIMIT 50 /* note */;')).toBe(true); + expect(hasOuterLimit('SELECT * FROM t LIMIT 50\n/* multi\nline */')).toBe( + true, + ); + }); + + it('detects an outer LIMIT … BY clause', () => { + expect(hasOuterLimit('SELECT * FROM t ORDER BY c LIMIT 10 BY host')).toBe( + true, + ); + expect(hasOuterLimit('SELECT * FROM t LIMIT 2 BY host, region;')).toBe( + true, + ); + // `LIMIT … BY … LIMIT m` is caught via the trailing regular LIMIT. + expect(hasOuterLimit('SELECT * FROM t LIMIT 1 BY host LIMIT 50')).toBe( + true, + ); + }); + + it('does not treat a LIMIT inside a subquery as an outer LIMIT', () => { + // Anchored to end-of-string: an inner LIMIT followed by more query is not + // matched, so the cardinality cap can still apply. + expect( + hasOuterLimit('SELECT * FROM (SELECT * FROM t LIMIT 10) GROUP BY k'), + ).toBe(false); + expect( + hasOuterLimit('SELECT * FROM (SELECT * FROM t LIMIT 5 BY h) GROUP BY k'), + ).toBe(false); + }); + + it('does not treat a trailing SETTINGS / FORMAT without a LIMIT as an outer LIMIT', () => { + expect( + hasOuterLimit( + 'SELECT k, count() FROM t GROUP BY k SETTINGS max_threads=4', + ), + ).toBe(false); + expect(hasOuterLimit('SELECT * FROM t FORMAT JSON')).toBe(false); + }); + + it('returns false for empty / non-string input', () => { + expect(hasOuterLimit('')).toBe(false); + expect(hasOuterLimit(undefined)).toBe(false); + }); +}); + +describe('resolveResultRowLimitSettings', () => { + it('returns undefined for a non-positive / absent cap', () => { + expect(resolveResultRowLimitSettings(undefined)).toBeUndefined(); + expect(resolveResultRowLimitSettings(0)).toBeUndefined(); + expect(resolveResultRowLimitSettings(-1)).toBeUndefined(); + }); + + it('applies both the result-row and cardinality caps when there is no outer LIMIT', () => { + const res = resolveResultRowLimitSettings(5000, { hasOuterLimit: false }); + expect(res).toEqual({ + cardinalityCapApplied: true, + settings: { + max_result_rows: '5001', + result_overflow_mode: 'break', + max_rows_to_group_by: '5001', + // 'break' (deterministic stop), never 'any' (folds keys → corrupts top-N) + group_by_overflow_mode: 'break', + }, + }); + }); + + it('applies only the order-preserving result-row cap when there is an outer LIMIT', () => { + const res = resolveResultRowLimitSettings(5000, { hasOuterLimit: true }); + expect(res).toEqual({ + cardinalityCapApplied: false, + settings: { + max_result_rows: '5001', + result_overflow_mode: 'break', + }, + }); + }); + + it('defaults to applying the cardinality cap when the option is omitted', () => { + expect(resolveResultRowLimitSettings(10)?.cardinalityCapApplied).toBe(true); + }); +}); + +describe('resolveDidOverflow', () => { + it('is false until the result is complete (no mid-stream flap)', () => { + expect( + resolveDidOverflow({ + isPlaceholderData: false, + isComplete: false, + didOverflow: true, + }), + ).toBe(false); + }); + + it('is false while showing stale placeholder data (no lingering banner)', () => { + expect( + resolveDidOverflow({ + isPlaceholderData: true, + isComplete: true, + didOverflow: true, + }), + ).toBe(false); + }); + + it('reflects the result once complete and fresh', () => { + expect( + resolveDidOverflow({ + isPlaceholderData: false, + isComplete: true, + didOverflow: true, + }), + ).toBe(true); + expect( + resolveDidOverflow({ + isPlaceholderData: false, + isComplete: true, + didOverflow: false, + }), + ).toBe(false); + }); + + it('treats an undefined didOverflow as false', () => { + expect( + resolveDidOverflow({ + isPlaceholderData: false, + isComplete: true, + didOverflow: undefined, + }), + ).toBe(false); + }); +}); diff --git a/packages/app/src/components/DBBarChart.tsx b/packages/app/src/components/DBBarChart.tsx index c95bdd5064..260799bf2e 100644 --- a/packages/app/src/components/DBBarChart.tsx +++ b/packages/app/src/components/DBBarChart.tsx @@ -20,6 +20,7 @@ import { import ChartContainer from './charts/ChartContainer'; import ChartErrorState from './charts/ChartErrorState'; import { ChartTooltipContainer, ChartTooltipItem } from './charts/ChartTooltip'; +import ResultOverflowBanner from './charts/ResultOverflowBanner'; const MAX_BAR_LABEL_LENGTH = 14; const BAR_LABEL_AXIS_HEIGHT = 80; // increased height to accommodate rotated + truncated labels @@ -68,10 +69,27 @@ export const DBBarChart = (props: CategoricalChartProps) => { error, chartData, responseFormatError, + didOverflow, + maxResultRows, + rows, + series, } = useCategoricalChart(props); return ( - + + ) : undefined + } + > {isLoading && !data ? (
Loading Chart Data... diff --git a/packages/app/src/components/DBPieChart.tsx b/packages/app/src/components/DBPieChart.tsx index a0eba392a0..ada6921335 100644 --- a/packages/app/src/components/DBPieChart.tsx +++ b/packages/app/src/components/DBPieChart.tsx @@ -12,6 +12,7 @@ import { import ChartContainer from './charts/ChartContainer'; import ChartErrorState from './charts/ChartErrorState'; import { ChartTooltipContainer, ChartTooltipItem } from './charts/ChartTooltip'; +import ResultOverflowBanner from './charts/ResultOverflowBanner'; const PieChartTooltip = memo( ({ @@ -98,10 +99,27 @@ export const DBPieChart = (props: CategoricalChartProps) => { error, chartData, responseFormatError, + didOverflow, + maxResultRows, + rows, + series, } = useCategoricalChart(props); return ( - + + ) : undefined + } + > {isLoading && !data ? (
Loading Chart Data... diff --git a/packages/app/src/components/DBTimeChart.tsx b/packages/app/src/components/DBTimeChart.tsx index 99cc456b75..4437b95e14 100644 --- a/packages/app/src/components/DBTimeChart.tsx +++ b/packages/app/src/components/DBTimeChart.tsx @@ -41,6 +41,7 @@ import { ChartSeriesTooltip } from '@/components/charts/ChartSeriesTooltip'; import { useChartTooltipZIndex } from '@/components/charts/ChartTooltip'; import { MAX_LOADABLE_TIME_CHART_SERIES, + resolveDidOverflow, resolveRenderedSeriesCap, } from '@/defaults'; import { type ActiveClickPayload, MemoChart } from '@/HDXMultiSeriesTimeChart'; @@ -56,6 +57,7 @@ import ChartErrorState, { import DateRangeIndicator from './charts/DateRangeIndicator'; import DisplaySwitcher from './charts/DisplaySwitcher'; import HiddenSeriesIndicator from './charts/HiddenSeriesIndicator'; +import ResultOverflowBanner from './charts/ResultOverflowBanner'; import MVOptimizationIndicator from './MaterializedViews/MVOptimizationIndicator'; /** A single group column / value pair decoded from a chart series key. */ @@ -323,6 +325,8 @@ type DBTimeChartComponentProps = { * behavior), which is all a standalone chart can do. */ onFocusSeries?: (filters: SeriesGroupFilter[]) => void; + /** Row cap for the query (see useQueriedChartConfig); overflow shows a banner. */ + maxResultRows?: number; }; function DBTimeChartComponent({ @@ -348,6 +352,7 @@ function DBTimeChartComponent({ showDateRangeIndicator = true, errorVariant, onFocusSeries, + maxResultRows, }: DBTimeChartComponentProps) { const [selectedSeriesSet, setSelectedSeriesSet] = useState>( new Set(), @@ -486,8 +491,17 @@ function DBTimeChartComponent({ enableQueryChunking: !disableQueryChunking, enableParallelQueries: enableParallelQueries && me?.team?.parallelizeWhenPossible, + maxResultRows, }); + // Whether the query exceeded the row cap. See resolveDidOverflow for the + // completion + freshness gating (shared with CategoricalChart). + const didOverflow = resolveDidOverflow({ + isPlaceholderData, + isComplete: data?.isComplete, + didOverflow: data?.didOverflow, + }); + const previousPeriodChartConfig: ChartConfigWithDateRange = useMemo(() => { const previousPeriodDateRange = queriedConfig.alignDateRangeToGranularity === false @@ -522,6 +536,10 @@ function DBTimeChartComponent({ queryKey: [queryKeyPrefix, previousPeriodChartConfig, 'chunked'], enabled: !!(enabled && config.compareToPreviousPeriod), enableQueryChunking: true, + // Cap the comparison series with the same bound as the primary query so + // the previous-period overlay can't bypass the guard. Banner still keys + // off the primary query only. + maxResultRows, }); const isLoadingOrPlaceholder = @@ -911,9 +929,10 @@ function DBTimeChartComponent({ key="db-time-chart-hidden-series-indicator" hiddenSeriesCount={hiddenSeriesCount} renderedSeriesCount={renderedSeriesCount} - // Offered only while still capped AND when load-all would actually - // raise the cap (loadAllHandler is undefined otherwise), so the - // notice never advertises a no-op click. + rowCount={data?.rows} + // When capped, the series counts describe only the subset — don't let + // the indicator claim "all series were loaded". + resultWasCapped={didOverflow} onLoadAll={loadAllHandler} />, ); @@ -939,11 +958,26 @@ function DBTimeChartComponent({ queriedConfig, hiddenSeriesCount, renderedSeriesCount, + data?.rows, + didOverflow, loadAllHandler, ]); return ( - + + ) : undefined + } + > {isLoading && !data ? (
Loading Chart Data... diff --git a/packages/app/src/components/charts/CategoricalChart.tsx b/packages/app/src/components/charts/CategoricalChart.tsx index 936af713cd..de7b6e80aa 100644 --- a/packages/app/src/components/charts/CategoricalChart.tsx +++ b/packages/app/src/components/charts/CategoricalChart.tsx @@ -9,6 +9,7 @@ import { formatResponseForCategoricalChart, } from '@/ChartUtils'; import MVOptimizationIndicator from '@/components/MaterializedViews/MVOptimizationIndicator'; +import { resolveDidOverflow } from '@/defaults'; import { useQueriedChartConfig } from '@/hooks/useChartConfig'; import { useMVOptimizationExplanation } from '@/hooks/useMVOptimizationExplanation'; import { useSingleSeriesNumberFormat, useSource } from '@/source'; @@ -26,6 +27,8 @@ export interface CategoricalChartProps { toolbarPrefix?: React.ReactNode[]; toolbarSuffix?: React.ReactNode[]; errorVariant?: ChartErrorStateVariant; + /** Row cap for the query (see useQueriedChartConfig); overflow shows a banner. */ + maxResultRows?: number; } /** @@ -39,6 +42,7 @@ export function useCategoricalChart({ showMVOptimizationIndicator = true, toolbarPrefix, toolbarSuffix, + maxResultRows, }: CategoricalChartProps) { const { data: source } = useSource({ id: config.source }); @@ -56,14 +60,21 @@ export function useCategoricalChart({ const { data: mvOptimizationData } = useMVOptimizationExplanation(builderQueriedConfig); - const { data, isLoading, isError, error } = useQueriedChartConfig( - queriedConfig, - { + const { data, isLoading, isError, error, isPlaceholderData } = + useQueriedChartConfig(queriedConfig, { placeholderData: (prev: any) => prev, queryKey: [queryKeyPrefix, queriedConfig], enabled, - }, - ); + maxResultRows, + }); + + // Whether the query exceeded the row cap. See resolveDidOverflow for the + // completion + freshness gating (shared with DBTimeChart). + const didOverflow = resolveDidOverflow({ + isPlaceholderData, + isComplete: data?.isComplete, + didOverflow: data?.didOverflow, + }); const toolbarItems = useMemo(() => { const allToolbarItems: React.ReactNode[] = []; @@ -134,5 +145,10 @@ export function useCategoricalChart({ error, chartData, responseFormatError, + didOverflow, + maxResultRows, + // Row + series (slice/bar) counts of the capped result, for the banner. + rows: data?.rows, + series: chartData.length, }; } diff --git a/packages/app/src/components/charts/ChartContainer.tsx b/packages/app/src/components/charts/ChartContainer.tsx index d92ad1c168..f90dc8169f 100644 --- a/packages/app/src/components/charts/ChartContainer.tsx +++ b/packages/app/src/components/charts/ChartContainer.tsx @@ -9,6 +9,8 @@ interface ChartContainerProps { toolbarItems?: React.ReactNode[]; children: React.ReactNode; disableReactiveContainer?: boolean; + /** Content between the header and chart body — e.g. a per-tile warning banner. */ + belowHeader?: React.ReactNode; } // When true, ChartContainer renders a "card" style header: an inline header @@ -71,6 +73,7 @@ function ChartContainer({ toolbarItems, children, disableReactiveContainer, + belowHeader, }: ChartContainerProps) { const cardHeader = use(ChartContainerCardHeaderContext); const collapsedToolbar = use(CollapsedToolbarContext); @@ -185,6 +188,7 @@ function ChartContainer({ )} )} + {belowHeader} {disableReactiveContainer ? ( children ) : ( @@ -193,7 +197,11 @@ function ChartContainer({ style={{ position: 'relative', width: '100%', - height: '100%', + // Flex-fill remaining height (vs rigid 100%) so a belowHeader + // banner squishes the chart instead of clipping it; minHeight:0 + // lets the absolute inner chart shrink. + flex: 1, + minHeight: 0, }} >
void; } /** - * Warns that the chart returned more series than the client renders. The - * transform caps series to protect memory; this surfaces the dropped ones and, - * when `onLoadAll` is provided, lets the user render all of them anyway. + * Notice that the chart drew only the top-N series (by peak value) to stay + * readable; `onLoadAll` lets the user draw all of them. Contrast + * ResultOverflowBanner (row cap — data the query may not have fetched). */ export default function HiddenSeriesIndicator({ hiddenSeriesCount, renderedSeriesCount, + rowCount, + resultWasCapped, onLoadAll, }: HiddenSeriesIndicatorProps) { if (hiddenSeriesCount <= 0) { @@ -23,16 +32,31 @@ export default function HiddenSeriesIndicator({ } const total = renderedSeriesCount + hiddenSeriesCount; + const rowsDetail = + typeof rowCount === 'number' + ? ` (from ${rowCount.toLocaleString()} rows)` + : ''; + // When capped, don't claim "all series were loaded" (the result is a subset). + const completenessClause = resultWasCapped + ? `Of the ${total.toLocaleString()} series in the loaded (capped) result, ` + + `only the ${renderedSeriesCount.toLocaleString()} largest (by peak value) ` + + `are drawn` + : `All ${total.toLocaleString()} series were loaded${rowsDetail}, but only ` + + `the ${renderedSeriesCount.toLocaleString()} largest (by peak value) are drawn`; const label = - `This query returned ${total.toLocaleString()} series. ` + - `${hiddenSeriesCount.toLocaleString()} low-value series were hidden to keep the page responsive; ` + - `showing the top ${renderedSeriesCount.toLocaleString()} by peak value. ` + + `Showing top ${renderedSeriesCount.toLocaleString()} of ` + + `${total.toLocaleString()} series. ${completenessClause} to keep the chart ` + + `readable — ${hiddenSeriesCount.toLocaleString()} smaller series are hidden. ` + (onLoadAll - ? `Click to load all ${total.toLocaleString()} (may be slow).` - : 'Add a stricter GROUP BY, a WHERE filter, or a series limit to reduce cardinality.'); + ? `Click to draw all ${total.toLocaleString()} (may be slow).` + : 'Increase the series limit, or narrow the query with a stricter GROUP BY or WHERE filter, to see more.'); const icon = ( - + ); return ( diff --git a/packages/app/src/components/charts/ResultOverflowBanner.tsx b/packages/app/src/components/charts/ResultOverflowBanner.tsx new file mode 100644 index 0000000000..51ef89ae70 --- /dev/null +++ b/packages/app/src/components/charts/ResultOverflowBanner.tsx @@ -0,0 +1,72 @@ +import { Alert, Text, Tooltip } from '@mantine/core'; +import { IconAlertTriangle } from '@tabler/icons-react'; + +interface ResultOverflowBannerProps { + /** Whether the query hit the row cap. When false, nothing renders. */ + didOverflow: boolean | undefined; + /** The row cap that was applied, shown in the message. */ + cap: number; + /** Rows returned by the capped query (≈cap since the cap is block-aligned). */ + rows?: number; + /** Distinct series (groups) in the capped result. Omit if unknown. */ + series?: number; +} + +/** + * Row-cap banner shown below a chart's header when the tile's query hit the cap + * (see DEFAULT_MAX_TILE_RESULT_ROWS). Detection is `rows > cap`, which can't + * prove rows were dropped, so the copy says the chart *may be* missing data. + * Contrast HiddenSeriesIndicator (limits only how many series are drawn). + */ +export default function ResultOverflowBanner({ + didOverflow, + cap, + rows, + series, +}: ResultOverflowBannerProps) { + if (!didOverflow) { + return null; + } + + // Approximate (block-aligned cap), so prefix with "~". + const rowsShown = (rows ?? cap).toLocaleString(); + const seriesText = + typeof series === 'number' ? ` (~${series.toLocaleString()} series)` : ''; + + return ( + + } + py={4} + px="xs" + styles={{ + wrapper: { alignItems: 'center' }, + body: { minWidth: 0 }, + message: { margin: 0 }, + icon: { marginRight: 8, width: 14, height: 14 }, + }} + // Stop propagation so a click doesn't start a react-grid-layout drag. + onMouseDown={e => e.stopPropagation()} + > + + Hit the ~{rowsShown}-row cap{seriesText} — chart may be missing data. + + + + ); +} diff --git a/packages/app/src/components/charts/__tests__/HiddenSeriesIndicator.test.tsx b/packages/app/src/components/charts/__tests__/HiddenSeriesIndicator.test.tsx index 672fe13828..234e859d59 100644 --- a/packages/app/src/components/charts/__tests__/HiddenSeriesIndicator.test.tsx +++ b/packages/app/src/components/charts/__tests__/HiddenSeriesIndicator.test.tsx @@ -42,4 +42,37 @@ describe('HiddenSeriesIndicator', () => { await userEvent.click(button); expect(onLoadAll).toHaveBeenCalledTimes(1); }); + + it('states all series were loaded when the result was NOT capped', async () => { + renderWithMantine( + , + ); + // Mantine renders the tooltip label into the DOM on hover. + await userEvent.hover(screen.getByTestId('hidden-series-indicator-icon')); + expect( + await screen.findByText(/All 1,000 series were loaded/i), + ).toBeInTheDocument(); + // Row count is surfaced as secondary detail. + expect(screen.getByText(/from 1,234 rows/i)).toBeInTheDocument(); + }); + + it('does NOT claim all series were loaded when the result WAS capped', async () => { + renderWithMantine( + , + ); + await userEvent.hover(screen.getByTestId('hidden-series-indicator-icon')); + const tip = await screen.findByText(/loaded \(capped\) result/i); + expect(tip).toBeInTheDocument(); + // The contradicting "all ... were loaded" claim must be absent. + expect(tip.textContent).not.toMatch(/series were loaded/i); + }); }); diff --git a/packages/app/src/components/charts/__tests__/ResultOverflowBanner.test.tsx b/packages/app/src/components/charts/__tests__/ResultOverflowBanner.test.tsx new file mode 100644 index 0000000000..f8f68b67ba --- /dev/null +++ b/packages/app/src/components/charts/__tests__/ResultOverflowBanner.test.tsx @@ -0,0 +1,45 @@ +import { screen } from '@testing-library/react'; + +import ResultOverflowBanner from '@/components/charts/ResultOverflowBanner'; + +describe('ResultOverflowBanner', () => { + it('renders nothing when didOverflow is false', () => { + renderWithMantine(); + expect(screen.queryByText(/row cap/)).not.toBeInTheDocument(); + }); + + it('renders nothing when didOverflow is undefined', () => { + renderWithMantine( + , + ); + expect(screen.queryByText(/row cap/)).not.toBeInTheDocument(); + }); + + it('renders the cap warning with the returned row count when overflowed', () => { + renderWithMantine( + , + ); + // Prefers the concrete returned-row count over the cap. + expect(screen.getByText(/6,250-row cap/)).toBeInTheDocument(); + expect(screen.getByText(/may be missing data/)).toBeInTheDocument(); + }); + + it('falls back to the cap when rows is omitted', () => { + renderWithMantine(); + expect(screen.getByText(/5,000-row cap/)).toBeInTheDocument(); + }); + + it('includes the series count when provided', () => { + renderWithMantine( + , + ); + expect(screen.getByText(/~1,250 series/)).toBeInTheDocument(); + }); + + it('omits the series suffix when series is not provided', () => { + renderWithMantine( + , + ); + expect(screen.queryByText(/series/)).not.toBeInTheDocument(); + }); +}); diff --git a/packages/app/src/defaults.ts b/packages/app/src/defaults.ts index 5894d8b78d..219a60b729 100644 --- a/packages/app/src/defaults.ts +++ b/packages/app/src/defaults.ts @@ -1,4 +1,8 @@ -import type { BuilderChartConfigWithDateRange } from '@hyperdx/common-utils/dist/types'; +import { isRawSqlChartConfig } from '@hyperdx/common-utils/dist/guards'; +import type { + BuilderChartConfigWithDateRange, + ChartConfigWithOptDateRange, +} from '@hyperdx/common-utils/dist/types'; // Limit defaults export const DEFAULT_SEARCH_ROW_LIMIT = 200; @@ -34,6 +38,160 @@ export const MAX_EXPANDED_TOOLTIP_ROWS = 500; // (HARD_LINES_LIMIT); drawn lines remain bounded by HARD_LINES_LIMIT regardless. export const MAX_LOADABLE_TIME_CHART_SERIES = 5000; +// Soft ceiling on rows a single dashboard-tile query returns — a last-line +// guard against a high-cardinality query streaming hundreds of thousands of +// rows into the browser. Bounds what is *transferred*, unlike the series/draw +// caps which bound what is *rendered*. Enforced by the ClickHouse settings in +// resolveResultRowLimitSettings; overflow is surfaced via didResultOverflow. +export const DEFAULT_MAX_TILE_RESULT_ROWS = 5000; + +export interface ResultRowLimitSettings { + /** `clickhouse_settings` to merge into the query request. */ + settings: Record; + /** True when the cardinality/memory guard (`max_rows_to_group_by`) applied. */ + cardinalityCapApplied: boolean; +} + +// Clauses ClickHouse allows AFTER an outer `LIMIT` (SETTINGS/FORMAT), plus a +// trailing `;`, single-line comment, and block comment. Stripped from the end +// (repeatedly) before the LIMIT test so a query like +// `... LIMIT 50 SETTINGS max_threads=4` or `... LIMIT 50 /* note */` still +// counts as having an outer LIMIT (missing it would wrongly enable the group-by +// cap and corrupt the top-N). SETTINGS uses `[\s\S]+` (not `.+`) so a clause +// wrapped onto multiple lines is still fully consumed. +const TRAILING_CLAUSE_RE = + /(?:\s+settings\s+[\s\S]+|\s+format\s+\w+|\s*;|\s*--[^\n]*|\s*\/\*[\s\S]*?\*\/)+\s*$/i; +// Outer `LIMIT` at the end (after trailing clauses are stripped): +// `LIMIT n [,m] [OFFSET n] [WITH TIES]` or `LIMIT n BY col…`. Anchored to +// end-of-string, so a LIMIT inside a subquery/CTE is not matched. Conservative: +// a miss only falls back to the safe result-row cap. +const OUTER_LIMIT_RE = + /\blimit\s+\d+(?:\s*,\s*\d+)?(?:\s+offset\s+\d+)?(?:\s+with\s+ties)?\s*$/i; +// `LIMIT n BY ` (ClickHouse LIMIT BY) at the end — still an outer LIMIT +// that runs after ORDER BY. Anchored to end-of-string; the column list may not +// contain a `)` (which would indicate a subquery's LIMIT BY, not an outer one). +// A `LIMIT … BY … LIMIT m` shape is already caught by OUTER_LIMIT_RE via its +// trailing `LIMIT m`. +const OUTER_LIMIT_BY_RE = + /\blimit\s+\d+(?:\s*,\s*\d+)?\s+by\s+[\w.,\s'"[\]]+$/i; + +/** Whether a raw SQL string ends in an outer `LIMIT` (incl. `LIMIT … BY`). */ +export function hasOuterLimit(sql: string | undefined): boolean { + if (typeof sql !== 'string' || sql.length === 0) { + return false; + } + const withoutTrailing = sql.trimEnd().replace(TRAILING_CLAUSE_RE, ''); + return ( + OUTER_LIMIT_RE.test(withoutTrailing) || + OUTER_LIMIT_BY_RE.test(withoutTrailing) + ); +} + +/** + * ClickHouse settings that enforce the row cap. Always applies the + * order-preserving result-row cap (`max_result_rows` + `result_overflow_mode`). + * Adds the cardinality/memory cap (`max_rows_to_group_by`) ONLY when the query + * has no outer LIMIT — that cap truncates the GROUP BY before an outer + * `ORDER BY … LIMIT`, which would silently corrupt the top-N. Uses `cap + 1` + * headroom so a complete result of exactly `cap` rows isn't flagged. Returns + * undefined for a non-positive/absent cap. + */ +export function resolveResultRowLimitSettings( + cap: number | undefined, + { + hasOuterLimit: queryHasOuterLimit = false, + }: { hasOuterLimit?: boolean } = {}, +): ResultRowLimitSettings | undefined { + const limit = resolveMaxResultRowsValue(cap); + if (limit == null) { + return undefined; + } + const settings: Record = { + max_result_rows: String(limit), + result_overflow_mode: 'break', + }; + const cardinalityCapApplied = !queryHasOuterLimit; + if (cardinalityCapApplied) { + settings.max_rows_to_group_by = String(limit); + settings.group_by_overflow_mode = 'break'; + } + return { settings, cardinalityCapApplied }; +} + +/** + * Translate a logical row cap into the `max_result_rows` value to send to + * ClickHouse. We request `cap + 1` so a result of exactly `cap` rows comes back + * whole (and is NOT flagged as overflow), while anything larger trips the break + * and returns at least `cap + 1` rows. Returns undefined for a non-positive / + * absent cap (no limit applied). + */ +export function resolveMaxResultRowsValue( + cap: number | undefined, +): number | undefined { + if (cap == null || !Number.isFinite(cap) || cap <= 0) { + return undefined; + } + return Math.floor(cap) + 1; +} + +/** + * Whether a tile query exceeded the row cap. Since the query runs with `cap + 1` + * headroom, `rows > cap` is the reliable signal (a complete result of ≤ cap rows + * comes back whole and is not flagged). We deliberately ignore + * `rows_before_limit_at_least`: it reports the count before the query's own + * LIMIT, so a tile ending in `... LIMIT 50` over a big aggregation would falsely + * trip. A non-positive/absent cap disables detection. + */ +export function didResultOverflow({ + rows, + cap, +}: { + rows: number | undefined; + cap: number | undefined; +}): boolean { + if (cap == null || !Number.isFinite(cap) || cap <= 0) { + return false; + } + if (typeof rows === 'number' && rows > cap) { + return true; + } + return false; +} + +/** + * The row cap a dashboard tile should apply, or undefined for none. Only raw SQL + * tiles are capped — builder tiles already bound cardinality via the series + * limit, and builder tables page rows through useOffsetPaginatedQuery. + */ +export function resolveTileMaxResultRows( + config: ChartConfigWithOptDateRange | undefined | null, +): number | undefined { + return config && isRawSqlChartConfig(config) + ? DEFAULT_MAX_TILE_RESULT_ROWS + : undefined; +} + +/** + * Whether a chart's result overflowed the row cap, gated so the banner is + * stable and never stale (shared by DBTimeChart and CategoricalChart): + * `isComplete` avoids flapping mid-stream, `!isPlaceholderData` avoids a stale + * banner lingering while a narrowed query is in flight. + */ +export function resolveDidOverflow({ + isPlaceholderData, + isComplete, + didOverflow, +}: { + isPlaceholderData: boolean | undefined; + isComplete: boolean | undefined; + didOverflow: boolean | undefined; +}): boolean { + if (isPlaceholderData || !isComplete) { + return false; + } + return didOverflow ?? false; +} + /** * Resolve the effective client-side render cap from a tile's `seriesLimit` * (see SharedChartSettingsSchema): null/undefined → the default cap, exactly 0 → diff --git a/packages/app/src/hooks/__tests__/useChartConfig.test.tsx b/packages/app/src/hooks/__tests__/useChartConfig.test.tsx index 7b9eab0b7c..38326a0893 100644 --- a/packages/app/src/hooks/__tests__/useChartConfig.test.tsx +++ b/packages/app/src/hooks/__tests__/useChartConfig.test.tsx @@ -415,6 +415,7 @@ describe('useChartConfig', () => { metadata: expect.any(Object), opts: { abort_signal: expect.any(AbortSignal), + clickhouse_settings: {}, }, }); expect(result.current.data).toEqual({ @@ -427,6 +428,165 @@ describe('useChartConfig', () => { expect(result.current.isPending).toBe(false); }); + it('applies max_result_rows + max_rows_to_group_by (cap + 1 headroom) and flags overflow', async () => { + const config = createMockChartConfig({ + dateRange: undefined, + granularity: '1 minute', + }); + + // 2 rows returned against a cap of 1 → rows (2) > cap (1) => overflow. + const mockResponse = createMockQueryResponse([ + { + 'count()': '71', + SeverityText: 'info', + __hdx_time_bucket: '2025-10-01T00:00:00Z', + }, + { + 'count()': '73', + SeverityText: 'warn', + __hdx_time_bucket: '2025-10-01T00:00:00Z', + }, + ]); + mockClickhouseClient.queryChartConfig.mockResolvedValue(mockResponse); + + const { result } = renderHook( + () => useQueriedChartConfig(config, { maxResultRows: 1 }), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + // A builder config has no outer LIMIT of its own, so both the result-row + // cap and the group-by cardinality cap are sent, each with one row of + // headroom (cap + 1). The group-by mode is 'break' (a deterministic stop), + // NOT 'any' (which folds late keys and would corrupt an ordered top-N). + expect(mockClickhouseClient.queryChartConfig).toHaveBeenCalledWith({ + config, + metadata: expect.any(Object), + opts: { + abort_signal: expect.any(AbortSignal), + clickhouse_settings: { + max_result_rows: '2', + result_overflow_mode: 'break', + max_rows_to_group_by: '2', + group_by_overflow_mode: 'break', + }, + }, + }); + expect(result.current.data?.didOverflow).toBe(true); + }); + + it('omits the group-by cardinality cap for raw SQL with an outer LIMIT (avoids corrupting its top-N)', async () => { + // Raw SQL whose own `ORDER BY … LIMIT N` would run AFTER a group-by cap, + // so applying the cardinality cap could silently fold away keys before the + // top-N is computed. Only the order-preserving result-row cap is applied. + const config = createMockChartConfig({ + dateRange: undefined, + granularity: undefined, + configType: 'sql', + sqlTemplate: + 'SELECT k, count() c FROM t GROUP BY k ORDER BY c DESC LIMIT 50', + }); + + const mockResponse = createMockQueryResponse([ + { 'count()': '71', SeverityText: 'info' }, + ]); + mockClickhouseClient.queryChartConfig.mockResolvedValue(mockResponse); + + const { result } = renderHook( + () => useQueriedChartConfig(config, { maxResultRows: 1 }), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + expect(mockClickhouseClient.queryChartConfig).toHaveBeenCalledWith({ + config, + metadata: expect.any(Object), + opts: { + abort_signal: expect.any(AbortSignal), + clickhouse_settings: { + readonly: '2', + max_result_rows: '2', + result_overflow_mode: 'break', + // NO max_rows_to_group_by / group_by_overflow_mode here. + }, + }, + }); + }); + + it('applies the group-by cardinality cap for raw SQL with no outer LIMIT', async () => { + const config = createMockChartConfig({ + dateRange: undefined, + granularity: undefined, + configType: 'sql', + sqlTemplate: 'SELECT k, count() c FROM t GROUP BY k', + }); + + const mockResponse = createMockQueryResponse([ + { 'count()': '71', SeverityText: 'info' }, + ]); + mockClickhouseClient.queryChartConfig.mockResolvedValue(mockResponse); + + const { result } = renderHook( + () => useQueriedChartConfig(config, { maxResultRows: 1 }), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + expect(mockClickhouseClient.queryChartConfig).toHaveBeenCalledWith({ + config, + metadata: expect.any(Object), + opts: { + abort_signal: expect.any(AbortSignal), + clickhouse_settings: { + readonly: '2', + max_result_rows: '2', + result_overflow_mode: 'break', + max_rows_to_group_by: '2', + group_by_overflow_mode: 'break', + }, + }, + }); + }); + + it('does not cap or flag overflow when maxResultRows is not set', async () => { + const config = createMockChartConfig({ + dateRange: undefined, + granularity: '1 minute', + }); + + const mockResponse = createMockQueryResponse([ + { + 'count()': '71', + SeverityText: 'info', + __hdx_time_bucket: '2025-10-01T00:00:00Z', + }, + ]); + mockClickhouseClient.queryChartConfig.mockResolvedValue(mockResponse); + + const { result } = renderHook(() => useQueriedChartConfig(config), { + wrapper, + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + expect(mockClickhouseClient.queryChartConfig).toHaveBeenCalledWith({ + config, + metadata: expect.any(Object), + opts: { + abort_signal: expect.any(AbortSignal), + clickhouse_settings: {}, + }, + }); + expect(result.current.data?.didOverflow).toBeUndefined(); + }); + it('fetches data without chunking when no granularity is provided', async () => { const config = createMockChartConfig({ dateRange: [new Date('2025-10-01'), new Date('2025-10-02')], @@ -464,6 +624,7 @@ describe('useChartConfig', () => { metadata: expect.any(Object), opts: { abort_signal: expect.any(AbortSignal), + clickhouse_settings: {}, }, }); expect(result.current.data).toEqual({ @@ -518,6 +679,7 @@ describe('useChartConfig', () => { metadata: expect.any(Object), opts: { abort_signal: expect.any(AbortSignal), + clickhouse_settings: {}, }, }); expect(result.current.data).toEqual({ @@ -597,6 +759,7 @@ describe('useChartConfig', () => { metadata: expect.any(Object), opts: { abort_signal: expect.any(AbortSignal), + clickhouse_settings: {}, }, }); expect(result.current.data).toEqual({ @@ -648,6 +811,7 @@ describe('useChartConfig', () => { metadata: expect.any(Object), opts: { abort_signal: expect.any(AbortSignal), + clickhouse_settings: {}, }, }); expect(result.current.data).toEqual({ @@ -696,6 +860,7 @@ describe('useChartConfig', () => { metadata: expect.any(Object), opts: { abort_signal: expect.any(AbortSignal), + clickhouse_settings: {}, }, }); expect(result.current.data).toEqual({ @@ -802,6 +967,101 @@ describe('useChartConfig', () => { expect(result.current.isPending).toBe(false); }); + it('flags overflow across chunks only when an individual chunk exceeds the cap', async () => { + // Chunked + capped: the cap is applied per chunk, so a single chunk that + // exceeds the cap is what signals a truncated result — even though other + // chunks are small. cap = 2 here. + const config = createMockChartConfig({ + dateRange: [ + new Date('2025-10-01 00:00:00Z'), + new Date('2025-10-02 00:00:00Z'), + ], + granularity: '3 hour', + }); + + // Newest window: 3 rows > cap(2) => this chunk overflowed. + const overflowingChunk = createMockQueryResponse([ + { 'count()': '1', __hdx_time_bucket: '2025-10-01T18:00:00Z' }, + { 'count()': '2', __hdx_time_bucket: '2025-10-01T19:00:00Z' }, + { 'count()': '3', __hdx_time_bucket: '2025-10-01T20:00:00Z' }, + ]); + // Older windows: each <= cap. + const smallChunkA = createMockQueryResponse([ + { 'count()': '4', __hdx_time_bucket: '2025-10-01T12:00:00Z' }, + ]); + const smallChunkB = createMockQueryResponse([ + { 'count()': '5', __hdx_time_bucket: '2025-10-01T01:00:00Z' }, + ]); + + mockClickhouseClient.queryChartConfig + .mockResolvedValueOnce(overflowingChunk) + .mockResolvedValueOnce(smallChunkA) + .mockResolvedValueOnce(smallChunkB); + + const { result } = renderHook( + () => + useQueriedChartConfig(config, { + enableQueryChunking: true, + maxResultRows: 2, + }), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + expect(result.current.data?.didOverflow).toBe(true); + expect(result.current.data?.isComplete).toBe(true); + }); + + it('does NOT flag overflow when the summed multi-chunk total exceeds the cap but no single chunk does', async () => { + // Regression guard: each chunk is individually within the cap, so nothing + // was truncated — the accumulated sum (which legitimately exceeds the cap) + // must NOT trip the banner. cap = 2, three chunks of <= 2 rows summing to 5. + const config = createMockChartConfig({ + dateRange: [ + new Date('2025-10-01 00:00:00Z'), + new Date('2025-10-02 00:00:00Z'), + ], + granularity: '3 hour', + }); + + const chunk1 = createMockQueryResponse([ + { 'count()': '1', __hdx_time_bucket: '2025-10-01T18:00:00Z' }, + { 'count()': '2', __hdx_time_bucket: '2025-10-01T19:00:00Z' }, + ]); + const chunk2 = createMockQueryResponse([ + { 'count()': '3', __hdx_time_bucket: '2025-10-01T12:00:00Z' }, + { 'count()': '4', __hdx_time_bucket: '2025-10-01T14:00:00Z' }, + ]); + const chunk3 = createMockQueryResponse([ + { 'count()': '5', __hdx_time_bucket: '2025-10-01T01:00:00Z' }, + ]); + + mockClickhouseClient.queryChartConfig + .mockResolvedValueOnce(chunk1) + .mockResolvedValueOnce(chunk2) + .mockResolvedValueOnce(chunk3); + + const { result } = renderHook( + () => + useQueriedChartConfig(config, { + enableQueryChunking: true, + maxResultRows: 2, + }), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + // Summed rows (5) exceed cap (2), but no chunk individually did, so this + // is a complete result — didOverflow must be false. + expect(result.current.data?.rows).toBe(5); + expect(result.current.data?.didOverflow).toBe(false); + expect(result.current.data?.isComplete).toBe(true); + }); + it('pins the series-limit ranking to the newest window on each chunk', async () => { const config = createMockChartConfig({ dateRange: [ @@ -1311,6 +1571,44 @@ describe('useChartConfig', () => { expect(result.current.isPending).toBe(false); }); + it('passes the row-cap clickhouse_settings on the parallel path', async () => { + const { config, mockResponse1, mockResponse2, mockResponse3 } = + setupParallelQueries(); + + mockClickhouseClient.queryChartConfig + .mockResolvedValueOnce(mockResponse1) + .mockResolvedValueOnce(mockResponse2) + .mockResolvedValueOnce(mockResponse3); + + const { result } = renderHook( + () => + useQueriedChartConfig(config, { + enableQueryChunking: true, + enableParallelQueries: true, + maxResultRows: 5000, + }), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isSuccess).toBe(true), { + timeout: 1000, + }); + await waitFor(() => expect(result.current.isFetching).toBe(false), { + timeout: 1000, + }); + + // Every parallel chunk query carries the cap settings (builder config has + // no outer LIMIT, so the cardinality cap applies too), each with cap + 1. + for (const call of mockClickhouseClient.queryChartConfig.mock.calls) { + expect(call[0].opts?.clickhouse_settings).toEqual({ + max_result_rows: '5001', + result_overflow_mode: 'break', + max_rows_to_group_by: '5001', + group_by_overflow_mode: 'break', + }); + } + }); + it('streams parallel query results in order', async () => { const { config, mockResponse1, mockResponse2, mockResponse3 } = setupParallelQueries(); diff --git a/packages/app/src/hooks/useChartConfig.tsx b/packages/app/src/hooks/useChartConfig.tsx index bdb5a39f3a..3c6de01785 100644 --- a/packages/app/src/hooks/useChartConfig.tsx +++ b/packages/app/src/hooks/useChartConfig.tsx @@ -37,6 +37,11 @@ import { prometheusApi } from '@/api'; import { toStartOfInterval } from '@/ChartUtils'; import { useClickhouseClient } from '@/clickhouse'; import { IS_MTVIEWS_ENABLED } from '@/config'; +import { + didResultOverflow, + hasOuterLimit, + resolveResultRowLimitSettings, +} from '@/defaults'; import { buildMTViewSelectQuery } from '@/hdxMTViews'; import { useMetadataWithSettings } from '@/hooks/useMetadata'; import { useSource } from '@/source'; @@ -54,6 +59,12 @@ interface AdditionalUseQueriedChartConfigOptions { */ enableQueryChunking?: boolean; enableParallelQueries?: boolean; + /** + * When positive, caps the rows the query may return (see + * resolveResultRowLimitSettings) and reports whether the cap was hit via + * `data.didOverflow`. Omit / 0 to leave uncapped. + */ + maxResultRows?: number; } type TimeWindow = { @@ -63,6 +74,11 @@ type TimeWindow = { type TQueryFnData = Pick, 'data' | 'meta' | 'rows'> & { isComplete: boolean; + /** + * True when the query exceeded the `maxResultRows` cap and the returned rows + * are a truncated slice of a larger result. Undefined when no cap was applied. + */ + didOverflow?: boolean; }; type TChunk = { @@ -142,6 +158,7 @@ async function* fetchDataInChunks({ enableParallelQueries = false, metadata, querySettings, + maxResultRows, }: { config: ChartConfigWithOptDateRange; clickhouseClient: ClickhouseClient; @@ -150,6 +167,7 @@ async function* fetchDataInChunks({ enableParallelQueries?: boolean; metadata: Metadata; querySettings: QuerySettings | undefined; + maxResultRows?: number; }) { const windows = enableQueryChunking && shouldUseChunking(config) @@ -189,10 +207,22 @@ async function* fetchDataInChunks({ } // Readonly = 2 means the query is readonly but can still specify query settings. - const clickHouseSettings = isRawSqlChartConfig(config) + const clickHouseSettings: Record = isRawSqlChartConfig(config) ? { readonly: '2' } : {}; + // Cap the rows any single (possibly chunked) query returns. The chosen + // settings depend on whether the raw SQL has its own outer LIMIT (which would + // be corrupted by the group-by cardinality cap) — see + // resolveResultRowLimitSettings. Builder configs have no sqlTemplate. + const rawSql = isRawSqlChartConfig(config) ? config.sqlTemplate : undefined; + const resultRowLimitSettings = resolveResultRowLimitSettings(maxResultRows, { + hasOuterLimit: hasOuterLimit(rawSql), + }); + if (resultRowLimitSettings != null) { + Object.assign(clickHouseSettings, resultRowLimitSettings.settings); + } + if (enableParallelQueries) { // fetch in parallel const promises = windows.map(async (w, index) => { @@ -242,6 +272,7 @@ async function* fetchDataInChunks({ metadata, opts: { abort_signal: signal, + clickhouse_settings: clickHouseSettings, }, querySettings, }); @@ -317,6 +348,7 @@ export function useQueriedChartConfig( config, options?.enableQueryChunking ?? false, options?.enableParallelQueries ?? false, + options?.maxResultRows ?? 0, ], // TODO: Replace this with `streamedQuery` when it is no longer experimental. Use 'replace' refetch mode. // https://tanstack.com/query/latest/docs/reference/streamedQuery @@ -430,15 +462,28 @@ export function useQueriedChartConfig( enableParallelQueries: options?.enableParallelQueries, metadata, querySettings: source?.querySettings, + maxResultRows: options?.maxResultRows, }); + // The cap applies per chunk, so `didOverflow` is the OR of the per-chunk + // `rows > cap` check (raw SQL never chunks, so its one chunk is the whole + // result). + const cap = options?.maxResultRows; + let anyChunkOverflowed = false; + let accumulatedChunks: TQueryFnData = emptyValue; for await (const chunk of chunks) { if (context.signal.aborted) { break; } + if (didResultOverflow({ rows: chunk.chunk.rows, cap })) { + anyChunkOverflowed = true; + } + accumulatedChunks = appendChunk(accumulatedChunks, chunk); + accumulatedChunks.didOverflow = + cap == null ? undefined : anyChunkOverflowed; // When refetching, the cache is not updated until all chunks are fetched. if (!isRefetch) {