From 742ce163b07cb63a06266fb3bd5a702f35966926 Mon Sep 17 00:00:00 2001 From: Aaron Knudtson <87577305+knudtty@users.noreply.github.com> Date: Tue, 9 Jun 2026 17:00:22 -0400 Subject: [PATCH 1/3] fix: use cloud versions for usable ALIAS columns --- .../src/__tests__/clickhouseVersion.test.ts | 81 +++++++++++++++++++ .../src/__tests__/metadata.test.ts | 34 ++++++++ .../src/__tests__/renderChartConfig.test.ts | 1 + .../src/core/clickhouseVersion.ts | 29 ++++++- packages/common-utils/src/core/metadata.ts | 37 +++++++++ packages/common-utils/src/queryParser.ts | 5 +- 6 files changed, 184 insertions(+), 3 deletions(-) diff --git a/packages/common-utils/src/__tests__/clickhouseVersion.test.ts b/packages/common-utils/src/__tests__/clickhouseVersion.test.ts index 3232cfdec6..e364335de4 100644 --- a/packages/common-utils/src/__tests__/clickhouseVersion.test.ts +++ b/packages/common-utils/src/__tests__/clickhouseVersion.test.ts @@ -173,4 +173,85 @@ describe('supportsDirectReadMap', () => { it('returns false when the version is undefined', () => { expect(supportsDirectReadMap(undefined)).toBe(false); }); + + describe('ClickHouse Cloud (isCloud=true)', () => { + describe('26.2 cloud backport branch (min 26.2.1.307)', () => { + it.each([ + [[26, 2, 1, 307], true], + [[26, 2, 1, 308], true], + [[26, 2, 2, 0], true], + [[26, 2, 99, 99], true], + [[26, 2, 1, 306], false], + [[26, 2, 1, 0], false], + [[26, 2, 0, 99], false], + [[26, 2, 0, 0], false], + ])('%j → %s', (version, expected) => { + expect(supportsDirectReadMap(version, true)).toBe(expected); + }); + }); + + describe('26.3 cloud branch (always supported)', () => { + it.each([ + [[26, 3, 0, 0], true], + [[26, 3, 0, 1], true], + [[26, 3, 1, 0], true], + [[26, 3, 12, 2], true], + [[26, 3, 99, 99], true], + ])('%j → %s', (version, expected) => { + expect(supportsDirectReadMap(version, true)).toBe(expected); + }); + }); + + describe('26.4 cloud backport branch (min 26.4.1.1740)', () => { + it.each([ + [[26, 4, 1, 1740], true], + [[26, 4, 1, 1741], true], + [[26, 4, 2, 0], true], + [[26, 4, 99, 99], true], + [[26, 4, 1, 1739], false], + [[26, 4, 1, 0], false], + [[26, 4, 0, 99], false], + [[26, 4, 0, 0], false], + ])('%j → %s', (version, expected) => { + expect(supportsDirectReadMap(version, true)).toBe(expected); + }); + }); + + describe('26.5+ cloud baseline (always supported)', () => { + it.each([ + [[26, 5, 0, 0], true], + [[26, 6, 0, 0], true], + [[27, 0, 0, 0], true], + ])('%j → %s', (version, expected) => { + expect(supportsDirectReadMap(version, true)).toBe(expected); + }); + }); + + describe('unsupported cloud branches', () => { + it.each([ + [[26, 1, 99, 99], false], + [[26, 0, 0, 0], false], + [[25, 12, 0, 0], false], + [[24, 0, 0, 0], false], + ])('%j → %s', (version, expected) => { + expect(supportsDirectReadMap(version, true)).toBe(expected); + }); + }); + + it('returns false when the version is undefined', () => { + expect(supportsDirectReadMap(undefined, true)).toBe(false); + }); + + it.each([ + [[26, 3, 0, 0]], + [[26, 2, 1, 307]], + [[26, 4, 1, 1740]], + ])( + 'cloud cutoffs differ from self-hosted on %j (supported on cloud, not self-hosted)', + version => { + expect(supportsDirectReadMap(version, false)).toBe(false); + expect(supportsDirectReadMap(version, true)).toBe(true); + }, + ); + }); }); diff --git a/packages/common-utils/src/__tests__/metadata.test.ts b/packages/common-utils/src/__tests__/metadata.test.ts index bbd40781ef..e0b4935135 100644 --- a/packages/common-utils/src/__tests__/metadata.test.ts +++ b/packages/common-utils/src/__tests__/metadata.test.ts @@ -309,6 +309,40 @@ describe('Metadata', () => { }); }); + describe('isClickHouseCloud', () => { + beforeEach(() => { + mockCache.getOrFetch.mockImplementation((key, queryFn) => queryFn()); + }); + + it('returns true when SharedMergeTree is registered in system.table_engines', async () => { + (mockClickhouseClient.query as jest.Mock).mockResolvedValue({ + json: jest.fn().mockResolvedValue({ + data: [{ '1': 1 }], + }), + }); + + const result = await metadata.isClickHouseCloud({ + connectionId: 'test_connection', + }); + + expect(result).toBe(true); + }); + + it('returns false when SharedMergeTree is absent from system.table_engines', async () => { + (mockClickhouseClient.query as jest.Mock).mockResolvedValue({ + json: jest.fn().mockResolvedValue({ + data: [], + }), + }); + + const result = await metadata.isClickHouseCloud({ + connectionId: 'test_connection', + }); + + expect(result).toBe(false); + }); + }); + describe('getSkipIndices', () => { beforeEach(() => { mockCache.getOrFetch.mockImplementation((key, queryFn) => queryFn()); diff --git a/packages/common-utils/src/__tests__/renderChartConfig.test.ts b/packages/common-utils/src/__tests__/renderChartConfig.test.ts index 9816c0f935..5205981f6f 100644 --- a/packages/common-utils/src/__tests__/renderChartConfig.test.ts +++ b/packages/common-utils/src/__tests__/renderChartConfig.test.ts @@ -48,6 +48,7 @@ describe('renderChartConfig', () => { .mockResolvedValue({ primary_key: 'timestamp' }), getSkipIndices: jest.fn().mockResolvedValue([]), getSetting: jest.fn().mockResolvedValue(undefined), + isClickHouseCloud: jest.fn().mockResolvedValue(false), } as unknown as jest.Mocked; }); diff --git a/packages/common-utils/src/core/clickhouseVersion.ts b/packages/common-utils/src/core/clickhouseVersion.ts index 0f781a2b10..7fe2fbb428 100644 --- a/packages/common-utils/src/core/clickhouseVersion.ts +++ b/packages/common-utils/src/core/clickhouseVersion.ts @@ -84,6 +84,25 @@ const DIRECT_READ_MAP_BACKPORT_MINS: ReadonlyArray = [ [26, 4, 3, 37], ]; +/** + * Per-branch minimum versions required for the direct_read map column + * optimization on ClickHouse Cloud. Cloud has its own release cadence and + * received the backport at different cutoffs than self-hosted: + * + * - 26.2 branch → first available at 26.2.1.307 + * - 26.3 branch → automatically supported (any 26.3.x on Cloud) + * - 26.4 branch → first available at 26.4.1.1740 + * - 26.5+ → always supported (inherits `DIRECT_READ_MAP_BASELINE`) + * + * The 26.3 entry sits at the floor of the branch so every 26.3.x.y version + * passes the per-branch comparison. + */ +const DIRECT_READ_MAP_CLOUD_BACKPORT_MINS: ReadonlyArray = [ + [26, 2, 1, 307], + [26, 3, 0, 0], + [26, 4, 1, 1740], +]; + /** * First release where direct_read map support shipped unconditionally. Any * server with major.minor at or above this is considered supported, even if @@ -95,9 +114,13 @@ const DIRECT_READ_MAP_BASELINE: ClickHouseVersion = [26, 5, 0, 0]; * Returns true when the connected ClickHouse server supports the direct_read * map column optimization. Returns false when the version is undefined or * predates every known backport. + * + * When `isCloud` is true, uses the ClickHouse Cloud-specific backport cutoffs + * (which differ from self-hosted because Cloud ships its own release stream). */ export function supportsDirectReadMap( version: ClickHouseVersion | undefined, + isCloud = false, ): boolean { if (!version) return false; @@ -105,8 +128,12 @@ export function supportsDirectReadMap( return true; } + const branchMins = isCloud + ? DIRECT_READ_MAP_CLOUD_BACKPORT_MINS + : DIRECT_READ_MAP_BACKPORT_MINS; + const [vMajor, vMinor] = version; - const branchMin = DIRECT_READ_MAP_BACKPORT_MINS.find( + const branchMin = branchMins.find( ([major, minor]) => major === vMajor && minor === vMinor, ); if (!branchMin) return false; diff --git a/packages/common-utils/src/core/metadata.ts b/packages/common-utils/src/core/metadata.ts index d6e2b10e34..86e79462ed 100644 --- a/packages/common-utils/src/core/metadata.ts +++ b/packages/common-utils/src/core/metadata.ts @@ -934,6 +934,43 @@ export class Metadata { }); } + /** + * Returns true when the connected server is ClickHouse Cloud, detected by + * checking whether `SharedMergeTree` is registered in `system.table_engines`. + * The SharedMergeTree engine is compiled into Cloud builds only, so its + * presence in the engine registry is a reliable Cloud signal that does not + * depend on any user table existing. + * + * Result is cached per connection — Cloud-ness is a server property. + */ + async isClickHouseCloud({ + connectionId, + }: { + connectionId: string; + }): Promise { + return this.cache.getOrFetch( + `${connectionId}.isClickHouseCloud`, + async () => { + try { + const sql = chSql`SELECT 1 FROM system.table_engines WHERE name = ${{ String: 'SharedMergeTree' }} LIMIT 1`; + const json = await this.clickhouseClient + .query<'JSON'>({ + connectionId, + query: sql.sql, + query_params: sql.params, + clickhouse_settings: this.getClickHouseSettings(), + shouldSkipApplySettings: true, + }) + .then(res => res.json<{ '1': number }>()); + return json.data.length > 0; + } catch (e) { + console.warn('Error detecting ClickHouse Cloud:', e); + return false; + } + }, + ); + } + /** * Returns the parsed ClickHouse server version (from `SELECT version()`). * Returns undefined when the query fails or the value cannot be parsed; the diff --git a/packages/common-utils/src/queryParser.ts b/packages/common-utils/src/queryParser.ts index 595a334dda..2a500c6b1c 100644 --- a/packages/common-utils/src/queryParser.ts +++ b/packages/common-utils/src/queryParser.ts @@ -1103,15 +1103,16 @@ export async function buildKvItemsLookup({ }): Promise { const lookup: KvItemsLookup = new Map(); try { - const [serverVersion, columns, skipIndices] = await Promise.all([ + const [serverVersion, columns, skipIndices, isCloud] = await Promise.all([ metadata.getServerVersion({ connectionId }), metadata.getColumns({ databaseName, tableName, connectionId }), metadata .getSkipIndices({ databaseName, tableName, connectionId }) .catch(() => [] as SkipIndexMetadata[]), + metadata.isClickHouseCloud({ connectionId }).catch(() => false), ]); - const directReadSupported = supportsDirectReadMap(serverVersion); + const directReadSupported = supportsDirectReadMap(serverVersion, isCloud); const kvItemsCandidates = columns.filter( c => From 1daebc16f1ec85fd6eed80c6f0c09494facf588d Mon Sep 17 00:00:00 2001 From: Aaron Knudtson <87577305+knudtty@users.noreply.github.com> Date: Tue, 9 Jun 2026 17:22:17 -0400 Subject: [PATCH 2/3] fix: cache miss --- .../src/__tests__/metadata.test.ts | 43 +++++++++++++++++++ packages/common-utils/src/core/metadata.ts | 5 ++- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/packages/common-utils/src/__tests__/metadata.test.ts b/packages/common-utils/src/__tests__/metadata.test.ts index e0b4935135..788d13ecd7 100644 --- a/packages/common-utils/src/__tests__/metadata.test.ts +++ b/packages/common-utils/src/__tests__/metadata.test.ts @@ -341,6 +341,49 @@ describe('Metadata', () => { expect(result).toBe(false); }); + + it('re-probes after a transient failure instead of caching false', async () => { + const realCache = new MetadataCache(); + const realMetadata = new Metadata(mockClickhouseClient, realCache); + + (mockClickhouseClient.query as jest.Mock) + .mockRejectedValueOnce(new Error('connection refused')) + .mockResolvedValueOnce({ + json: jest.fn().mockResolvedValue({ data: [{ '1': 1 }] }), + }); + + const first = await realMetadata.isClickHouseCloud({ + connectionId: 'test_connection', + }); + expect(first).toBe(false); + + const second = await realMetadata.isClickHouseCloud({ + connectionId: 'test_connection', + }); + expect(second).toBe(true); + + expect(mockClickhouseClient.query).toHaveBeenCalledTimes(2); + }); + + it('caches a successful negative result and does not re-query', async () => { + const realCache = new MetadataCache(); + const realMetadata = new Metadata(mockClickhouseClient, realCache); + + (mockClickhouseClient.query as jest.Mock).mockResolvedValue({ + json: jest.fn().mockResolvedValue({ data: [] }), + }); + + const first = await realMetadata.isClickHouseCloud({ + connectionId: 'test_connection', + }); + const second = await realMetadata.isClickHouseCloud({ + connectionId: 'test_connection', + }); + + expect(first).toBe(false); + expect(second).toBe(false); + expect(mockClickhouseClient.query).toHaveBeenCalledTimes(1); + }); }); describe('getSkipIndices', () => { diff --git a/packages/common-utils/src/core/metadata.ts b/packages/common-utils/src/core/metadata.ts index 86e79462ed..dd4388d954 100644 --- a/packages/common-utils/src/core/metadata.ts +++ b/packages/common-utils/src/core/metadata.ts @@ -948,7 +948,7 @@ export class Metadata { }: { connectionId: string; }): Promise { - return this.cache.getOrFetch( + const result = await this.cache.getOrFetch( `${connectionId}.isClickHouseCloud`, async () => { try { @@ -965,10 +965,11 @@ export class Metadata { return json.data.length > 0; } catch (e) { console.warn('Error detecting ClickHouse Cloud:', e); - return false; + return undefined; } }, ); + return result ?? false; } /** From 52ef5483d9ce1fd19b742eab830ec48eb0b73d4c Mon Sep 17 00:00:00 2001 From: Aaron Knudtson <87577305+knudtty@users.noreply.github.com> Date: Wed, 10 Jun 2026 09:50:28 -0400 Subject: [PATCH 3/3] fix: better query pattern --- packages/common-utils/src/__tests__/metadata.test.ts | 4 ++-- packages/common-utils/src/core/metadata.ts | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/common-utils/src/__tests__/metadata.test.ts b/packages/common-utils/src/__tests__/metadata.test.ts index 788d13ecd7..2006bf3131 100644 --- a/packages/common-utils/src/__tests__/metadata.test.ts +++ b/packages/common-utils/src/__tests__/metadata.test.ts @@ -317,7 +317,7 @@ describe('Metadata', () => { it('returns true when SharedMergeTree is registered in system.table_engines', async () => { (mockClickhouseClient.query as jest.Mock).mockResolvedValue({ json: jest.fn().mockResolvedValue({ - data: [{ '1': 1 }], + data: [{ is_cloud: true }], }), }); @@ -349,7 +349,7 @@ describe('Metadata', () => { (mockClickhouseClient.query as jest.Mock) .mockRejectedValueOnce(new Error('connection refused')) .mockResolvedValueOnce({ - json: jest.fn().mockResolvedValue({ data: [{ '1': 1 }] }), + json: jest.fn().mockResolvedValue({ data: [{ is_cloud: true }] }), }); const first = await realMetadata.isClickHouseCloud({ diff --git a/packages/common-utils/src/core/metadata.ts b/packages/common-utils/src/core/metadata.ts index dd4388d954..5e80ad7f4a 100644 --- a/packages/common-utils/src/core/metadata.ts +++ b/packages/common-utils/src/core/metadata.ts @@ -952,17 +952,17 @@ export class Metadata { `${connectionId}.isClickHouseCloud`, async () => { try { - const sql = chSql`SELECT 1 FROM system.table_engines WHERE name = ${{ String: 'SharedMergeTree' }} LIMIT 1`; + const query = + "SELECT count() > 0 AS is_cloud FROM system.table_engines WHERE name = 'SharedMergeTree'"; const json = await this.clickhouseClient .query<'JSON'>({ connectionId, - query: sql.sql, - query_params: sql.params, + query, clickhouse_settings: this.getClickHouseSettings(), shouldSkipApplySettings: true, }) - .then(res => res.json<{ '1': number }>()); - return json.data.length > 0; + .then(res => res.json<{ is_cloud: boolean }>()); + return json.data.length > 0 && json.data[0].is_cloud; } catch (e) { console.warn('Error detecting ClickHouse Cloud:', e); return undefined;