Skip to content

[HDX-5348] fix(clickhouse): disable per-part subcolumn size calculation on CH 26.3+ #3102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kodiakhq merged 3 commits into from
Sep 10, 2026
Merged

[HDX-5348] fix(clickhouse): disable per-part subcolumn size calculation on CH 26.3+ #3102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
47c003f
fix(clickhouse): disable per-part subcolumn size calculation on CH 26.3+
wrn14897 Sep 9, 2026
b6c879e
test: drop redundant subcolumn-setting omission case
wrn14897 Sep 9, 2026
8094b9b
Merge branch 'main' into warren/hdx-5348-disable-subcolumn-size-calc
kodiakhq[bot] Sep 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .changeset/lazy-pugs-smoke.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
'@hyperdx/common-utils': patch
'@hyperdx/app': patch
---

fix: disable per-part subcolumn size calculation on ClickHouse 26.3+

ClickHouse 26.3 turned on
`allow_calculating_subcolumns_sizes_for_merge_tree_reading` by default, which
makes PREWHERE planning fetch per-part sizes for every map key a query
references. On SharedMergeTree that is one S3 GET per (key × active part), it
runs before any row is read, and `max_execution_time` does not interrupt it.
Queries referencing many attribute keys — the LLM dashboard reads ~64 — could
spend minutes in planning. Queries now send the setting as `0` when the server
supports it.
3 changes: 2 additions & 1 deletion packages/app/src/llm/dashboard/EfficiencyCharts.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ const PERCENT_NUMBER_FORMAT: NumberFormat = {
*/
export function EfficiencyCharts(props: LLMChartProps) {
const { source, expressions } = props;
const base = baseLLMChartConfig(props);
// Neither chart selects cost, so skip the ~70 KiB cost alias binding.
const base = baseLLMChartConfig({ ...props, withCostAlias: false });

@github-actions github-actions Bot Sep 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 minor — withCostAlias is an opt-out flag scoped to a shared base, so the unused ~70 KiB binding survives in mixed files and will recur

The flag defaults to IS_LLM_COST_ENABLED (chartConfig.ts:33), so every new chart ships the binding unless the author remembers to pass false — this diff is the second retro-fit round after ErrorsTab/LatencyTab/LLMEmptyStateBanner. It is also per-base, not per-chart: OverviewCharts.tsx:50 builds one base for six tiles and only the two at lines 99 and 118 reference LLM_COST_SQL_ALIAS, so with cost enabled four tiles still carry it (same in AttributionCharts.tsx:108, AgentToolCharts.tsx:210, SessionsTab.tsx:139). Derive it instead of declaring it — e.g. a small llmChartConfig(props, chartFields) wrapper that emits the with entry only when the chart's own select/groupBy/filters mention LLM_COST_SQL_ALIAS — which fixes the mixed files too and removes the flag from the call sites.


return (
<>
Expand Down
3 changes: 2 additions & 1 deletion packages/app/src/llm/dashboard/LatencyCharts.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ const CHART_HEIGHT = 320;
*/
export function LatencyCharts(props: LLMChartProps) {
const { source, expressions } = props;
const base = baseLLMChartConfig(props);
// Neither chart selects cost, so skip the ~70 KiB cost alias binding.
const base = baseLLMChartConfig({ ...props, withCostAlias: false });

return (
<>
Expand Down
8 changes: 8 additions & 0 deletions packages/common-utils/src/__tests__/clickhouse.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,10 @@ describe('processClickhouseSettings - optimization settings', () => {
// { name: 'use_top_k_dynamic_filtering', value: '1' },
{ name: 'use_skip_indexes_on_data_read', value: '1' },
{ name: 'use_skip_indexes_for_disjunctions', value: '1' },
{
name: 'allow_calculating_subcolumns_sizes_for_merge_tree_reading',
value: '1',
},
]);

await client.query({
Expand All @@ -440,9 +444,13 @@ describe('processClickhouseSettings - optimization settings', () => {
// use_top_k_dynamic_filtering: '1',
use_skip_indexes_on_data_read: '1',
use_skip_indexes_for_disjunctions: '1',
allow_calculating_subcolumns_sizes_for_merge_tree_reading: '0',
});
});

// The exact-equality assertion below is what keeps unavailable settings off
// the wire — e.g. a pre-26.3 server that would reject
// allow_calculating_subcolumns_sizes_for_merge_tree_reading as unknown.
it('should only apply available optimization settings', async () => {
setupMockQuery([
{ name: 'use_skip_indexes_for_top_k', value: '1' },
Expand Down
9 changes: 9 additions & 0 deletions packages/common-utils/src/clickhouse/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,15 @@ export abstract class BaseClickhouseClient {
// Enables full-text (inverted index) search.
applySettingIfAvailable('enable_full_text_index', '1');

// 26.3 turned this on by default. On SharedMergeTree it makes PREWHERE
// planning fetch per-part sizes for every map key referenced — one S3 GET
// each, not interruptible by max_execution_time. The sizes only reorder
// PREWHERE conditions, so the pre-26.3 approximation is fine.
applySettingIfAvailable(
'allow_calculating_subcolumns_sizes_for_merge_tree_reading',
Comment thread
wrn14897 marked this conversation as resolved.
Resolved
'0',
);

return {
...defaultSettings,
...clickhouse_settings,
Expand Down
Loading