From 9bf2e8c81c274a2855ccb344a2355f638f439255 Mon Sep 17 00:00:00 2001 From: Bilal Atique Date: Sat, 26 Sep 2026 13:06:24 +0500 Subject: [PATCH 01/12] fix(app): show MATERIALIZED and ALIAS columns in the row details panel The row details panel fetches the row with SELECT *, and ClickHouse leaves MATERIALIZED and ALIAS columns out of * by default, so the panel never showed them. useQueriedChartConfig takes an additionalQuerySettings option, added after the source's query settings (a setting the source already defines keeps its value). useRowData uses it to enable asterisk_include_materialized_columns and asterisk_include_alias_columns when it selects *. Sources with a Known Columns List are unchanged. Closes #3206 --- .../row-panel-materialized-alias-columns.md | 12 ++++ .../app/src/components/DBRowDataPanel.tsx | 14 ++++ .../__tests__/DBRowDataPanel.test.ts | 28 ++++++++ .../hooks/__tests__/useChartConfig.test.tsx | 65 +++++++++++++++++++ packages/app/src/hooks/useChartConfig.tsx | 35 +++++++++- 5 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 .changeset/row-panel-materialized-alias-columns.md diff --git a/.changeset/row-panel-materialized-alias-columns.md b/.changeset/row-panel-materialized-alias-columns.md new file mode 100644 index 0000000000..54270bb33e --- /dev/null +++ b/.changeset/row-panel-materialized-alias-columns.md @@ -0,0 +1,12 @@ +--- +'@hyperdx/app': patch +--- + +fix: show MATERIALIZED and ALIAS columns in the row details panel + +The row details panel fetched the row with `SELECT *`, which ClickHouse runs +without MATERIALIZED and ALIAS columns, so those columns never appeared. The row +query now enables `asterisk_include_materialized_columns` and +`asterisk_include_alias_columns`, except for sources that use a Known Columns +List. A value set for either setting in the source's query settings still takes +precedence. diff --git a/packages/app/src/components/DBRowDataPanel.tsx b/packages/app/src/components/DBRowDataPanel.tsx index d57c6c1a20..6d6879afc6 100644 --- a/packages/app/src/components/DBRowDataPanel.tsx +++ b/packages/app/src/components/DBRowDataPanel.tsx @@ -4,6 +4,7 @@ import type { ResponseJSON } from '@hyperdx/common-utils/dist/clickhouse'; import { isLogSource, isTraceSource, + QuerySettings, SourceKind, TSource, } from '@hyperdx/common-utils/dist/types'; @@ -40,6 +41,13 @@ export enum ROW_DATA_ALIASES { SPAN_LINKS = '__hdx_span_links', } +// ClickHouse leaves MATERIALIZED and ALIAS columns out of `SELECT *` unless +// these settings are on, so the row panel would hide those columns. +const SELECT_ALL_COLUMNS_QUERY_SETTINGS: QuerySettings = [ + { setting: 'asterisk_include_materialized_columns', value: '1' }, + { setting: 'asterisk_include_alias_columns', value: '1' }, +]; + export function useRowData({ source, rowId, @@ -209,6 +217,10 @@ export function useRowData({ ...(aliasWith && aliasWith.length > 0 ? { with: aliasWith } : {}), }; + const additionalQuerySettings = knownColumns + ? undefined + : SELECT_ALL_COLUMNS_QUERY_SETTINGS; + const baseQueryKey = ['row_side_panel', rowId, aliasWith, source]; // Both halves of the filter are needed for `renderChartConfig` to emit one, so // a source with no usable timestamp expression can't be bounded at all. @@ -224,6 +236,7 @@ export function useRowData({ { queryKey: [...baseQueryKey, dateRange], enabled: rowId != null && hasWindow, + additionalQuerySettings, }, ); @@ -244,6 +257,7 @@ export function useRowData({ const fallbackResult = useQueriedChartConfig(baseConfig, { queryKey: [...baseQueryKey, undefined], enabled: rowId != null && isFallbackActive, + additionalQuerySettings, }); const queryResult = isFallbackActive ? fallbackResult : boundedResult; diff --git a/packages/app/src/components/__tests__/DBRowDataPanel.test.ts b/packages/app/src/components/__tests__/DBRowDataPanel.test.ts index 211a0f4a4c..eaa7b1d00a 100644 --- a/packages/app/src/components/__tests__/DBRowDataPanel.test.ts +++ b/packages/app/src/components/__tests__/DBRowDataPanel.test.ts @@ -77,6 +77,34 @@ describe('DBRowDataPanel', () => { expect(config.select).not.toContainEqual({ valueExpression: '*' }); }); + it('includes MATERIALIZED and ALIAS columns when selecting `*`', () => { + renderHook(() => useRowData({ source, rowId: "id='abc123'" })); + + const [bounded, fallback] = mockUseQueriedChartConfig.mock.calls; + for (const [, options] of [bounded, fallback]) { + expect(options.additionalQuerySettings).toEqual([ + { setting: 'asterisk_include_materialized_columns', value: '1' }, + { setting: 'asterisk_include_alias_columns', value: '1' }, + ]); + } + }); + + it('adds no query settings when the source has a Known Columns List', () => { + const sourceWithKnownColumns: TLogSource = { + ...source, + knownColumnsListExpression: 'Timestamp, Body, ServiceName', + }; + + renderHook(() => + useRowData({ source: sourceWithKnownColumns, rowId: "id='abc123'" }), + ); + + const [bounded, fallback] = mockUseQueriedChartConfig.mock.calls; + for (const [, options] of [bounded, fallback]) { + expect(options.additionalQuerySettings).toBeUndefined(); + } + }); + describe('time filtering', () => { // A row id synthesized from ids alone (e.g. "View Trace" builds // TraceId + SpanId) has no timestamp of its own, so without a dateRange the diff --git a/packages/app/src/hooks/__tests__/useChartConfig.test.tsx b/packages/app/src/hooks/__tests__/useChartConfig.test.tsx index 3aaf0f208e..54cde353a7 100644 --- a/packages/app/src/hooks/__tests__/useChartConfig.test.tsx +++ b/packages/app/src/hooks/__tests__/useChartConfig.test.tsx @@ -20,6 +20,7 @@ import { appendChunk, getGranularityAlignedTimeWindows, getMinGranularitySeconds, + mergeQuerySettings, useQueriedChartConfig, } from '@/hooks/useChartConfig'; import { useMVOptimizationExplanation } from '@/hooks/useMVOptimizationExplanation'; @@ -1111,6 +1112,39 @@ describe('useChartConfig', () => { expect(result.current.isPending).toBe(false); }); + it('passes additionalQuerySettings to the query', async () => { + const config = createMockChartConfig({ + dateRange: undefined, + granularity: undefined, + }); + const additionalQuerySettings = [ + { setting: 'asterisk_include_alias_columns', value: '1' }, + ]; + + mockClickhouseClient.queryChartConfig.mockResolvedValue( + createMockQueryResponse([]), + ); + + const { result } = renderHook( + () => useQueriedChartConfig(config, { additionalQuerySettings }), + { + wrapper, + }, + ); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + expect(mockClickhouseClient.queryChartConfig).toHaveBeenCalledTimes(1); + expect(mockClickhouseClient.queryChartConfig).toHaveBeenCalledWith({ + config, + metadata: expect.any(Object), + opts: { + abort_signal: expect.any(AbortSignal), + }, + querySettings: additionalQuerySettings, + }); + }); + 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')], @@ -2260,6 +2294,37 @@ describe('useChartConfig', () => { }); }); + describe('mergeQuerySettings', () => { + const sourceSettings = [{ setting: 'max_threads', value: '4' }]; + + it('returns the source settings unchanged when there is nothing to add', () => { + expect(mergeQuerySettings(sourceSettings, undefined)).toBe( + sourceSettings, + ); + expect(mergeQuerySettings(undefined, [])).toBeUndefined(); + }); + + it('adds the additional settings after the source settings', () => { + expect( + mergeQuerySettings(sourceSettings, [ + { setting: 'asterisk_include_alias_columns', value: '1' }, + ]), + ).toEqual([ + { setting: 'max_threads', value: '4' }, + { setting: 'asterisk_include_alias_columns', value: '1' }, + ]); + }); + + it('keeps the value of a setting that the source already defines', () => { + expect( + mergeQuerySettings( + [{ setting: 'asterisk_include_alias_columns', value: '0' }], + [{ setting: 'asterisk_include_alias_columns', value: '1' }], + ), + ).toEqual([{ setting: 'asterisk_include_alias_columns', value: '0' }]); + }); + }); + describe('appendChunk', () => { const empty = { data: [], meta: [], rows: 0, isComplete: false }; diff --git a/packages/app/src/hooks/useChartConfig.tsx b/packages/app/src/hooks/useChartConfig.tsx index f94c339de7..c77b69768a 100644 --- a/packages/app/src/hooks/useChartConfig.tsx +++ b/packages/app/src/hooks/useChartConfig.tsx @@ -73,6 +73,11 @@ interface AdditionalUseQueriedChartConfigOptions { * replaces the key the hook builds from the config. */ queryKeyPrefix?: string; + /** + * Query settings for this query only, added after the source's own query + * settings. A setting that the source already defines keeps its value. + */ + additionalQuerySettings?: QuerySettings; } type TimeWindow = { @@ -285,6 +290,28 @@ async function* fetchDataInChunks({ } } +/** + * Adds per-query settings after the source's query settings. A setting that the + * source already defines keeps the source's value. Exported for tests. + */ +export function mergeQuerySettings( + sourceSettings: QuerySettings | undefined, + additionalSettings: QuerySettings | undefined, +): QuerySettings | undefined { + if (!additionalSettings?.length) { + return sourceSettings; + } + const sourceSettingNames = new Set( + (sourceSettings ?? []).map(({ setting }) => setting), + ); + return [ + ...(sourceSettings ?? []), + ...additionalSettings.filter( + ({ setting }) => !sourceSettingNames.has(setting), + ), + ]; +} + /** Append the given chunk to the given accumulated result. Exported for tests. */ export function appendChunk( accumulated: TQueryFnData, @@ -373,6 +400,9 @@ export function useQueriedChartConfig( options?.enableQueryChunking ?? false, options?.enableParallelQueries ?? false, minGranularitySeconds, + ...(options?.additionalQuerySettings?.length + ? [options.additionalQuerySettings] + : []), ], // TODO: Replace this with `streamedQuery` when it is no longer experimental. Use 'replace' refetch mode. // https://tanstack.com/query/latest/docs/reference/streamedQuery @@ -405,7 +435,10 @@ export function useQueriedChartConfig( enableQueryChunking: options?.enableQueryChunking, enableParallelQueries: options?.enableParallelQueries, metadata, - querySettings: source?.querySettings, + querySettings: mergeQuerySettings( + source?.querySettings, + options?.additionalQuerySettings, + ), }); let accumulatedChunks: TQueryFnData = emptyValue; From fcd4774a6e2af91c1dd4ca9dd997578a0739ce97 Mon Sep 17 00:00:00 2001 From: Bilal Atique Date: Sat, 26 Sep 2026 13:17:31 +0500 Subject: [PATCH 02/12] fix(app): fetch the row without the settings when the connection rejects them A connection whose user has readonly = 1 rejects any query setting (Code 164, READONLY), and an ALIAS column can fail to evaluate. In both cases the row query with the asterisk_include_* settings fails, so the panel would show an error where it used to load the row. useRowData now remembers the connection that rejected the settings and fetches the row again without them. The query with the settings does not retry, and the hook reports loading, not an error, until the plain query settles. Add integration tests that run SELECT * through queryChartConfig against a table with MATERIALIZED and ALIAS columns, with and without the settings. --- .../app/src/components/DBRowDataPanel.tsx | 44 +++++++++--- .../__tests__/DBRowDataPanel.test.ts | 39 ++++++++++- .../__tests__/queryChartConfig.int.test.ts | 68 +++++++++++++++++++ 3 files changed, 140 insertions(+), 11 deletions(-) diff --git a/packages/app/src/components/DBRowDataPanel.tsx b/packages/app/src/components/DBRowDataPanel.tsx index 6d6879afc6..5df29bfd21 100644 --- a/packages/app/src/components/DBRowDataPanel.tsx +++ b/packages/app/src/components/DBRowDataPanel.tsx @@ -1,4 +1,4 @@ -import { useMemo } from 'react'; +import { useEffect, useMemo, useState } from 'react'; import { flatten } from 'flat'; import type { ResponseJSON } from '@hyperdx/common-utils/dist/clickhouse'; import { @@ -217,11 +217,24 @@ export function useRowData({ ...(aliasWith && aliasWith.length > 0 ? { with: aliasWith } : {}), }; - const additionalQuerySettings = knownColumns - ? undefined - : SELECT_ALL_COLUMNS_QUERY_SETTINGS; - - const baseQueryKey = ['row_side_panel', rowId, aliasWith, source]; + // A connection can reject these settings (for example, a `readonly = 1` user), + // and an ALIAS column can fail to evaluate. Then fetch the row without them. + const [settingsRejectedBy, setSettingsRejectedBy] = useState(); + const additionalQuerySettings = + knownColumns || settingsRejectedBy === source.connection + ? undefined + : SELECT_ALL_COLUMNS_QUERY_SETTINGS; + const settingsQueryOptions = additionalQuerySettings + ? { additionalQuerySettings, retry: false } + : {}; + + const baseQueryKey = [ + 'row_side_panel', + rowId, + aliasWith, + source, + additionalQuerySettings, + ]; // Both halves of the filter are needed for `renderChartConfig` to emit one, so // a source with no usable timestamp expression can't be bounded at all. const hasWindow = dateRange != null && timestampValueExpr != null; @@ -236,7 +249,7 @@ export function useRowData({ { queryKey: [...baseQueryKey, dateRange], enabled: rowId != null && hasWindow, - additionalQuerySettings, + ...settingsQueryOptions, }, ); @@ -257,16 +270,26 @@ export function useRowData({ const fallbackResult = useQueriedChartConfig(baseConfig, { queryKey: [...baseQueryKey, undefined], enabled: rowId != null && isFallbackActive, - additionalQuerySettings, + ...settingsQueryOptions, }); const queryResult = isFallbackActive ? fallbackResult : boundedResult; + const isSettingsQueryFailed = + additionalQuerySettings != null && queryResult.isError; + useEffect(() => { + if (isSettingsQueryFailed) { + setSettingsRejectedBy(source.connection); + } + }, [isSettingsQueryFailed, source.connection]); + // The bounded result is known-empty by the time the retry is enabled, so // report loading until it settles rather than briefly claiming the row is - // absent. + // absent. The same applies while the row is fetched again without settings. const isLoading = - queryResult.isLoading || (isBoundedEmpty && queryResult.isPending); + queryResult.isLoading || + isSettingsQueryFailed || + (isBoundedEmpty && queryResult.isPending); // Normalize resource and event attributes to always use flat keys for both JSON and Map columns const normalizedData = useMemo(() => { @@ -297,6 +320,7 @@ export function useRowData({ return { ...queryResult, + ...(isSettingsQueryFailed ? { isError: false, error: null } : {}), data: normalizedData, isLoading, }; diff --git a/packages/app/src/components/__tests__/DBRowDataPanel.test.ts b/packages/app/src/components/__tests__/DBRowDataPanel.test.ts index eaa7b1d00a..a1a0ab6874 100644 --- a/packages/app/src/components/__tests__/DBRowDataPanel.test.ts +++ b/packages/app/src/components/__tests__/DBRowDataPanel.test.ts @@ -1,5 +1,5 @@ import { SourceKind, TLogSource } from '@hyperdx/common-utils/dist/types'; -import { renderHook } from '@testing-library/react'; +import { renderHook, waitFor } from '@testing-library/react'; import { getJSONColumnNames, @@ -105,6 +105,43 @@ describe('DBRowDataPanel', () => { } }); + it('fetches the row again without the settings when the connection rejects them', async () => { + const row = { Body: 'hello' }; + mockUseQueriedChartConfig.mockImplementation( + (_config: unknown, options?: { additionalQuerySettings?: unknown }) => + options?.additionalQuerySettings + ? { + data: undefined, + error: new Error( + "Cannot modify 'asterisk_include_materialized_columns' setting in readonly mode", + ), + isLoading: false, + isPending: false, + isError: true, + isSuccess: false, + } + : { + data: { data: [row], meta: [], rows: 1, isComplete: true }, + error: null, + isLoading: false, + isPending: false, + isError: false, + isSuccess: true, + }, + ); + + const { result } = renderHook(() => + useRowData({ source, rowId: "id='abc123'" }), + ); + + await waitFor(() => expect(result.current.data?.data).toEqual([row])); + expect(result.current.isError).toBe(false); + + const calls = mockUseQueriedChartConfig.mock.calls; + expect(calls[0][1]).toMatchObject({ retry: false }); + expect(calls[calls.length - 1][1].additionalQuerySettings).toBeUndefined(); + }); + describe('time filtering', () => { // A row id synthesized from ids alone (e.g. "View Trace" builds // TraceId + SpanId) has no timestamp of its own, so without a dateRange the diff --git a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts index 892b24a59a..427f7f6f76 100644 --- a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts +++ b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts @@ -2856,4 +2856,72 @@ describe('queryChartConfig Integration Tests', () => { expect(Number(col(rows[0], 'Error rate'))).toBeCloseTo(40, 5); }); }); + + // The row details panel selects `*` with these settings (see useRowData). + describe('SELECT * with MATERIALIZED and ALIAS columns', () => { + const ROW_TABLE = 'select_all_columns_int_test'; + const config: ChartConfigWithOptDateRange = { + connection: 'test-connection', + from: { databaseName: DATABASE, tableName: ROW_TABLE }, + select: [{ valueExpression: '*' }], + where: 'Id = 1', + limit: { limit: 1 }, + }; + + beforeAll(async () => { + await client.command({ + query: `CREATE OR REPLACE TABLE ${DATABASE}.${ROW_TABLE} ( + Id UInt32, + StatusCode UInt16, + StatusClass String MATERIALIZED if(StatusCode >= 500, '5xx', 'other'), + IsServerError Bool ALIAS StatusCode >= 500 + ) + ENGINE = MergeTree + ORDER BY Id`, + }); + await client.insert({ + table: `${DATABASE}.${ROW_TABLE}`, + values: [{ Id: 1, StatusCode: 503 }], + format: 'JSONEachRow', + }); + }); + + afterAll(async () => { + await client.command({ + query: `DROP TABLE IF EXISTS ${DATABASE}.${ROW_TABLE}`, + }); + }); + + it('leaves MATERIALIZED and ALIAS columns out of `*` by default', async () => { + const result = await hdxClient.queryChartConfig({ + config, + metadata, + querySettings: undefined, + }); + + expect(result.meta?.map(m => m.name)).toEqual(['Id', 'StatusCode']); + }); + + it('returns MATERIALIZED and ALIAS columns with the asterisk_include_* settings', async () => { + const result = await hdxClient.queryChartConfig({ + config, + metadata, + querySettings: [ + { setting: 'asterisk_include_materialized_columns', value: '1' }, + { setting: 'asterisk_include_alias_columns', value: '1' }, + ], + }); + + expect(result.meta?.map(m => m.name)).toEqual([ + 'Id', + 'StatusCode', + 'StatusClass', + 'IsServerError', + ]); + expect(result.data[0]).toMatchObject({ + StatusClass: '5xx', + IsServerError: true, + }); + }); + }); }); From fa630d79d7f6758a487864bb284a8724f9edab16 Mon Sep 17 00:00:00 2001 From: Bilal Atique Date: Sat, 26 Sep 2026 13:19:35 +0500 Subject: [PATCH 03/12] fix(app): honor the source's value for the row query settings The row query has no config.source, so useQueriedChartConfig never sees the source's query settings, and a source that set asterisk_include_alias_columns or asterisk_include_materialized_columns to 0 was ignored. useRowData now uses the source's own value for these two settings. Also document that callers passing their own queryKey must include additionalQuerySettings in it, and test the partial overlap in mergeQuerySettings. --- packages/app/src/components/DBRowDataPanel.tsx | 9 ++++++++- .../components/__tests__/DBRowDataPanel.test.ts | 17 +++++++++++++++++ .../src/hooks/__tests__/useChartConfig.test.tsx | 15 +++++++++++++++ packages/app/src/hooks/useChartConfig.tsx | 2 ++ 4 files changed, 42 insertions(+), 1 deletion(-) diff --git a/packages/app/src/components/DBRowDataPanel.tsx b/packages/app/src/components/DBRowDataPanel.tsx index 5df29bfd21..88a0465c67 100644 --- a/packages/app/src/components/DBRowDataPanel.tsx +++ b/packages/app/src/components/DBRowDataPanel.tsx @@ -220,10 +220,17 @@ export function useRowData({ // A connection can reject these settings (for example, a `readonly = 1` user), // and an ALIAS column can fail to evaluate. Then fetch the row without them. const [settingsRejectedBy, setSettingsRejectedBy] = useState(); + // The row query has no `config.source`, so the source's query settings do not + // reach it. Use the source's own value for these two settings here. const additionalQuerySettings = knownColumns || settingsRejectedBy === source.connection ? undefined - : SELECT_ALL_COLUMNS_QUERY_SETTINGS; + : SELECT_ALL_COLUMNS_QUERY_SETTINGS.map( + defaultSetting => + source.querySettings?.find( + ({ setting }) => setting === defaultSetting.setting, + ) ?? defaultSetting, + ); const settingsQueryOptions = additionalQuerySettings ? { additionalQuerySettings, retry: false } : {}; diff --git a/packages/app/src/components/__tests__/DBRowDataPanel.test.ts b/packages/app/src/components/__tests__/DBRowDataPanel.test.ts index a1a0ab6874..fe5c5b46d7 100644 --- a/packages/app/src/components/__tests__/DBRowDataPanel.test.ts +++ b/packages/app/src/components/__tests__/DBRowDataPanel.test.ts @@ -89,6 +89,23 @@ describe('DBRowDataPanel', () => { } }); + it("uses the source's own value for these settings", () => { + const sourceWithSetting: TLogSource = { + ...source, + querySettings: [{ setting: 'asterisk_include_alias_columns', value: '0' }], + }; + + renderHook(() => + useRowData({ source: sourceWithSetting, rowId: "id='abc123'" }), + ); + + const [[, options]] = mockUseQueriedChartConfig.mock.calls; + expect(options.additionalQuerySettings).toEqual([ + { setting: 'asterisk_include_materialized_columns', value: '1' }, + { setting: 'asterisk_include_alias_columns', value: '0' }, + ]); + }); + it('adds no query settings when the source has a Known Columns List', () => { const sourceWithKnownColumns: TLogSource = { ...source, diff --git a/packages/app/src/hooks/__tests__/useChartConfig.test.tsx b/packages/app/src/hooks/__tests__/useChartConfig.test.tsx index 54cde353a7..ff1fadeb11 100644 --- a/packages/app/src/hooks/__tests__/useChartConfig.test.tsx +++ b/packages/app/src/hooks/__tests__/useChartConfig.test.tsx @@ -2315,6 +2315,21 @@ describe('useChartConfig', () => { ]); }); + it('adds only the settings that the source does not define', () => { + expect( + mergeQuerySettings( + [{ setting: 'asterisk_include_alias_columns', value: '0' }], + [ + { setting: 'asterisk_include_materialized_columns', value: '1' }, + { setting: 'asterisk_include_alias_columns', value: '1' }, + ], + ), + ).toEqual([ + { setting: 'asterisk_include_alias_columns', value: '0' }, + { setting: 'asterisk_include_materialized_columns', value: '1' }, + ]); + }); + it('keeps the value of a setting that the source already defines', () => { expect( mergeQuerySettings( diff --git a/packages/app/src/hooks/useChartConfig.tsx b/packages/app/src/hooks/useChartConfig.tsx index c77b69768a..83640d38a4 100644 --- a/packages/app/src/hooks/useChartConfig.tsx +++ b/packages/app/src/hooks/useChartConfig.tsx @@ -76,6 +76,8 @@ interface AdditionalUseQueriedChartConfigOptions { /** * Query settings for this query only, added after the source's own query * settings. A setting that the source already defines keeps its value. + * They are part of the default query key; a caller that passes its own + * `queryKey` must include them in it. */ additionalQuerySettings?: QuerySettings; } From 274cd95acbd78e4c3183faa1d6c6a0cd206f676c Mon Sep 17 00:00:00 2001 From: Bilal Atique Date: Sat, 26 Sep 2026 18:05:20 +0500 Subject: [PATCH 04/12] fix(app): fall back only when the connection rejects the row query settings The row query fell back to a plain SELECT * on any error and turned off the normal retry, so a timeout or a bad row id dropped the MATERIALIZED and ALIAS columns for the rest of the panel's life, and a failing ALIAS column on one row disabled them for the whole connection. Now useRowData falls back only when ClickHouse rejects one of the two settings (the error names the setting: READONLY, UNKNOWN_SETTING, SETTING_CONSTRAINT_VIOLATION). Other errors keep the settings, the normal retry and the error state. The rejection is kept per connection in a store shared by every row lookup and read with useSyncExternalStore, which the React Compiler does not memoize. A source can still turn either setting off in its query settings. useQueriedChartConfig now adds additionalQuerySettings to a caller-supplied queryKey as well, and useRowData uses mergeQuerySettings for the source's own values. --- .../app/src/components/DBRowDataPanel.tsx | 90 ++++++++++----- .../__tests__/DBRowDataPanel.test.ts | 109 +++++++++++++----- .../hooks/__tests__/useChartConfig.test.tsx | 34 ++++++ packages/app/src/hooks/useChartConfig.tsx | 8 +- 4 files changed, 180 insertions(+), 61 deletions(-) diff --git a/packages/app/src/components/DBRowDataPanel.tsx b/packages/app/src/components/DBRowDataPanel.tsx index 88a0465c67..8753abee41 100644 --- a/packages/app/src/components/DBRowDataPanel.tsx +++ b/packages/app/src/components/DBRowDataPanel.tsx @@ -1,4 +1,4 @@ -import { useEffect, useMemo, useState } from 'react'; +import { useEffect, useMemo, useSyncExternalStore } from 'react'; import { flatten } from 'flat'; import type { ResponseJSON } from '@hyperdx/common-utils/dist/clickhouse'; import { @@ -10,7 +10,10 @@ import { } from '@hyperdx/common-utils/dist/types'; import { Box } from '@mantine/core'; -import { useQueriedChartConfig } from '@/hooks/useChartConfig'; +import { + mergeQuerySettings, + useQueriedChartConfig, +} from '@/hooks/useChartConfig'; import { WithClause } from '@/hooks/useRowWhere'; import { getDisplayedTimestampValueExpression, @@ -48,6 +51,35 @@ const SELECT_ALL_COLUMNS_QUERY_SETTINGS: QuerySettings = [ { setting: 'asterisk_include_alias_columns', value: '1' }, ]; +// Connections whose user cannot change these settings (for example, a +// `readonly = 1` user). Shared by every row lookup, so each one fails once. +// Read it through useSyncExternalStore: the React Compiler memoizes plain reads. +const connectionsRejectingSelectAllSettings = new Set(); +const rejectedConnectionListeners = new Set<() => void>(); + +function subscribeToRejectedConnections(listener: () => void) { + rejectedConnectionListeners.add(listener); + return () => { + rejectedConnectionListeners.delete(listener); + }; +} + +function markConnectionRejectingSettings(connection: string) { + if (!connectionsRejectingSelectAllSettings.has(connection)) { + connectionsRejectingSelectAllSettings.add(connection); + rejectedConnectionListeners.forEach(listener => listener()); + } +} + +// ClickHouse names the setting in each rejection: READONLY (164), +// UNKNOWN_SETTING (115) and SETTING_CONSTRAINT_VIOLATION (452). +function isSelectAllSettingRejected(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error ?? ''); + return SELECT_ALL_COLUMNS_QUERY_SETTINGS.some(({ setting }) => + message.includes(setting), + ); +} + export function useRowData({ source, rowId, @@ -217,31 +249,35 @@ export function useRowData({ ...(aliasWith && aliasWith.length > 0 ? { with: aliasWith } : {}), }; - // A connection can reject these settings (for example, a `readonly = 1` user), - // and an ALIAS column can fail to evaluate. Then fetch the row without them. - const [settingsRejectedBy, setSettingsRejectedBy] = useState(); + const connection = source.connection; + const rejectsSettings = useSyncExternalStore( + subscribeToRejectedConnections, + () => connectionsRejectingSelectAllSettings.has(connection), + () => false, + ); // The row query has no `config.source`, so the source's query settings do not // reach it. Use the source's own value for these two settings here. const additionalQuerySettings = - knownColumns || settingsRejectedBy === source.connection + knownColumns || rejectsSettings ? undefined - : SELECT_ALL_COLUMNS_QUERY_SETTINGS.map( - defaultSetting => - source.querySettings?.find( - ({ setting }) => setting === defaultSetting.setting, - ) ?? defaultSetting, + : mergeQuerySettings( + source.querySettings?.filter(({ setting }) => + SELECT_ALL_COLUMNS_QUERY_SETTINGS.some( + defaultSetting => defaultSetting.setting === setting, + ), + ), + SELECT_ALL_COLUMNS_QUERY_SETTINGS, ); + // A rejected setting will fail again, so only other errors keep the retry. const settingsQueryOptions = additionalQuerySettings - ? { additionalQuerySettings, retry: false } + ? { + additionalQuerySettings, + retry: (failureCount: number, error: Error) => + failureCount < 1 && !isSelectAllSettingRejected(error), + } : {}; - const baseQueryKey = [ - 'row_side_panel', - rowId, - aliasWith, - source, - additionalQuerySettings, - ]; + const baseQueryKey = ['row_side_panel', rowId, aliasWith, source]; // Both halves of the filter are needed for `renderChartConfig` to emit one, so // a source with no usable timestamp expression can't be bounded at all. const hasWindow = dateRange != null && timestampValueExpr != null; @@ -282,20 +318,22 @@ export function useRowData({ const queryResult = isFallbackActive ? fallbackResult : boundedResult; - const isSettingsQueryFailed = - additionalQuerySettings != null && queryResult.isError; + const isSettingsRejected = + additionalQuerySettings != null && + queryResult.isError && + isSelectAllSettingRejected(queryResult.error); useEffect(() => { - if (isSettingsQueryFailed) { - setSettingsRejectedBy(source.connection); + if (isSettingsRejected) { + markConnectionRejectingSettings(connection); } - }, [isSettingsQueryFailed, source.connection]); + }, [isSettingsRejected, connection]); // The bounded result is known-empty by the time the retry is enabled, so // report loading until it settles rather than briefly claiming the row is // absent. The same applies while the row is fetched again without settings. const isLoading = queryResult.isLoading || - isSettingsQueryFailed || + isSettingsRejected || (isBoundedEmpty && queryResult.isPending); // Normalize resource and event attributes to always use flat keys for both JSON and Map columns @@ -327,7 +365,7 @@ export function useRowData({ return { ...queryResult, - ...(isSettingsQueryFailed ? { isError: false, error: null } : {}), + ...(isSettingsRejected ? { isError: false, error: null } : {}), data: normalizedData, isLoading, }; diff --git a/packages/app/src/components/__tests__/DBRowDataPanel.test.ts b/packages/app/src/components/__tests__/DBRowDataPanel.test.ts index fe5c5b46d7..c9b63e25d8 100644 --- a/packages/app/src/components/__tests__/DBRowDataPanel.test.ts +++ b/packages/app/src/components/__tests__/DBRowDataPanel.test.ts @@ -9,6 +9,8 @@ import { import { useQueriedChartConfig } from '@/hooks/useChartConfig'; jest.mock('@/hooks/useChartConfig', () => ({ + mergeQuerySettings: jest.requireActual('@/hooks/useChartConfig') + .mergeQuerySettings, useQueriedChartConfig: jest.fn(), })); @@ -101,8 +103,8 @@ describe('DBRowDataPanel', () => { const [[, options]] = mockUseQueriedChartConfig.mock.calls; expect(options.additionalQuerySettings).toEqual([ - { setting: 'asterisk_include_materialized_columns', value: '1' }, { setting: 'asterisk_include_alias_columns', value: '0' }, + { setting: 'asterisk_include_materialized_columns', value: '1' }, ]); }); @@ -122,41 +124,84 @@ describe('DBRowDataPanel', () => { } }); - it('fetches the row again without the settings when the connection rejects them', async () => { + describe('when the query with the settings fails', () => { + const READONLY_ERROR = + "Code: 164. DB::Exception: Cannot modify 'asterisk_include_materialized_columns' setting in readonly mode. (READONLY)"; const row = { Body: 'hello' }; - mockUseQueriedChartConfig.mockImplementation( - (_config: unknown, options?: { additionalQuerySettings?: unknown }) => - options?.additionalQuerySettings - ? { - data: undefined, - error: new Error( - "Cannot modify 'asterisk_include_materialized_columns' setting in readonly mode", - ), - isLoading: false, - isPending: false, - isError: true, - isSuccess: false, - } - : { - data: { data: [row], meta: [], rows: 1, isComplete: true }, - error: null, - isLoading: false, - isPending: false, - isError: false, - isSuccess: true, - }, - ); - const { result } = renderHook(() => - useRowData({ source, rowId: "id='abc123'" }), - ); + // Fails every query that carries the settings with `error`. + const mockSettingsQueryError = (error: Error) => + mockUseQueriedChartConfig.mockImplementation( + (_config: unknown, options?: { additionalQuerySettings?: unknown }) => + options?.additionalQuerySettings + ? { + data: undefined, + error, + isLoading: false, + isPending: false, + isError: true, + isSuccess: false, + } + : { + data: { data: [row], meta: [], rows: 1, isComplete: true }, + error: null, + isLoading: false, + isPending: false, + isError: false, + isSuccess: true, + }, + ); - await waitFor(() => expect(result.current.data?.data).toEqual([row])); - expect(result.current.isError).toBe(false); + it('fetches the row again without them when the connection rejects them', async () => { + const readonlySource = { ...source, connection: 'readonly-conn' }; + mockSettingsQueryError(new Error(READONLY_ERROR)); - const calls = mockUseQueriedChartConfig.mock.calls; - expect(calls[0][1]).toMatchObject({ retry: false }); - expect(calls[calls.length - 1][1].additionalQuerySettings).toBeUndefined(); + const { result } = renderHook(() => + useRowData({ source: readonlySource, rowId: "id='abc123'" }), + ); + + await waitFor(() => expect(result.current.data?.data).toEqual([row])); + expect(result.current.isError).toBe(false); + const calls = mockUseQueriedChartConfig.mock.calls; + expect( + calls[calls.length - 1][1].additionalQuerySettings, + ).toBeUndefined(); + + // A later lookup on the same connection skips the settings from the start. + mockUseQueriedChartConfig.mockClear(); + renderHook(() => + useRowData({ source: readonlySource, rowId: "id='def456'" }), + ); + expect( + mockUseQueriedChartConfig.mock.calls[0][1].additionalQuerySettings, + ).toBeUndefined(); + }); + + it('keeps them and reports the error when the query fails for another reason', async () => { + const timeout = new Error( + 'Code: 159. DB::Exception: Timeout exceeded. (TIMEOUT_EXCEEDED)', + ); + mockSettingsQueryError(timeout); + + const { result } = renderHook(() => + useRowData({ source, rowId: "id='abc123'" }), + ); + + await waitFor(() => expect(result.current.isError).toBe(true)); + expect(result.current.error).toBe(timeout); + for (const [, options] of mockUseQueriedChartConfig.mock.calls) { + expect(options.additionalQuerySettings).toBeDefined(); + } + }); + + it('retries once, except when a setting is rejected', () => { + renderHook(() => useRowData({ source, rowId: "id='abc123'" })); + + const [[, { retry }]] = mockUseQueriedChartConfig.mock.calls; + expect(retry(0, new Error('Timeout exceeded'))).toBe(true); + expect(retry(1, new Error('Timeout exceeded'))).toBe(false); + expect(retry(0, new Error(READONLY_ERROR))).toBe(false); + }); }); describe('time filtering', () => { diff --git a/packages/app/src/hooks/__tests__/useChartConfig.test.tsx b/packages/app/src/hooks/__tests__/useChartConfig.test.tsx index ff1fadeb11..997da9b9b3 100644 --- a/packages/app/src/hooks/__tests__/useChartConfig.test.tsx +++ b/packages/app/src/hooks/__tests__/useChartConfig.test.tsx @@ -1145,6 +1145,40 @@ describe('useChartConfig', () => { }); }); + it('adds additionalQuerySettings to a queryKey that the caller passes', async () => { + const config = createMockChartConfig({ + dateRange: undefined, + granularity: undefined, + }); + const additionalQuerySettings = [ + { setting: 'asterisk_include_alias_columns', value: '1' }, + ]; + + mockClickhouseClient.queryChartConfig.mockResolvedValue( + createMockQueryResponse([]), + ); + + const { result } = renderHook( + () => + useQueriedChartConfig(config, { + queryKey: ['caller-key'], + additionalQuerySettings, + }), + { + wrapper, + }, + ); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + expect( + queryClient.getQueryCache().find({ + queryKey: ['caller-key', additionalQuerySettings], + exact: true, + }), + ).toBeDefined(); + }); + 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')], diff --git a/packages/app/src/hooks/useChartConfig.tsx b/packages/app/src/hooks/useChartConfig.tsx index 83640d38a4..6c1efd6dc5 100644 --- a/packages/app/src/hooks/useChartConfig.tsx +++ b/packages/app/src/hooks/useChartConfig.tsx @@ -76,8 +76,7 @@ interface AdditionalUseQueriedChartConfigOptions { /** * Query settings for this query only, added after the source's own query * settings. A setting that the source already defines keeps its value. - * They are part of the default query key; a caller that passes its own - * `queryKey` must include them in it. + * They are added to the query key, also when the caller passes `queryKey`. */ additionalQuerySettings?: QuerySettings; } @@ -294,7 +293,7 @@ async function* fetchDataInChunks({ /** * Adds per-query settings after the source's query settings. A setting that the - * source already defines keeps the source's value. Exported for tests. + * source already defines keeps the source's value. */ export function mergeQuerySettings( sourceSettings: QuerySettings | undefined, @@ -474,6 +473,9 @@ export function useQueriedChartConfig( retry: 1, refetchOnWindowFocus: false, ...options, + ...(options?.queryKey && options.additionalQuerySettings?.length + ? { queryKey: [...options.queryKey, options.additionalQuerySettings] } + : {}), enabled: enabled && !isLoadingMVOptimization && !isSourceLoading, }); From 43dcfc22eee3e6fcde48d14c80fa81bb5c0dd168 Mon Sep 17 00:00:00 2001 From: Bilal Atique Date: Sat, 26 Sep 2026 18:06:28 +0500 Subject: [PATCH 05/12] docs(changeset): note the row fallback when the settings are rejected --- .changeset/row-panel-materialized-alias-columns.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.changeset/row-panel-materialized-alias-columns.md b/.changeset/row-panel-materialized-alias-columns.md index 54270bb33e..0da2690d5a 100644 --- a/.changeset/row-panel-materialized-alias-columns.md +++ b/.changeset/row-panel-materialized-alias-columns.md @@ -9,4 +9,5 @@ without MATERIALIZED and ALIAS columns, so those columns never appeared. The row query now enables `asterisk_include_materialized_columns` and `asterisk_include_alias_columns`, except for sources that use a Known Columns List. A value set for either setting in the source's query settings still takes -precedence. +precedence. If the connection's user cannot change these settings (for example, +`readonly = 1`), the panel loads the row without them. From 80207e797c9ff5abf342a996f5f430ecf06c48f8 Mon Sep 17 00:00:00 2001 From: Bilal Atique Date: Sat, 26 Sep 2026 18:17:26 +0500 Subject: [PATCH 06/12] fix(app): reset the row settings rejection when a connection is edited A connection that rejected the asterisk_include_* settings stayed marked until the page reloaded, so after switching it to a user that can change settings, MATERIALIZED and ALIAS columns stayed hidden. useUpdateConnection now clears the entry for the edited connection. Move the settings, the rejection check and the shared store out of DBRowDataPanel into hooks/useSelectAllColumnsSettingsRejection, and test the render in which the rejection arrives: it reports loading, not the error. --- .../app/src/components/DBRowDataPanel.tsx | 57 ++++------------- .../__tests__/DBRowDataPanel.test.ts | 26 +++++++- packages/app/src/connection.ts | 4 +- ...eSelectAllColumnsSettingsRejection.test.ts | 53 ++++++++++++++++ .../useSelectAllColumnsSettingsRejection.ts | 62 +++++++++++++++++++ 5 files changed, 152 insertions(+), 50 deletions(-) create mode 100644 packages/app/src/hooks/__tests__/useSelectAllColumnsSettingsRejection.test.ts create mode 100644 packages/app/src/hooks/useSelectAllColumnsSettingsRejection.ts diff --git a/packages/app/src/components/DBRowDataPanel.tsx b/packages/app/src/components/DBRowDataPanel.tsx index 8753abee41..36f2957316 100644 --- a/packages/app/src/components/DBRowDataPanel.tsx +++ b/packages/app/src/components/DBRowDataPanel.tsx @@ -1,10 +1,9 @@ -import { useEffect, useMemo, useSyncExternalStore } from 'react'; +import { useEffect, useMemo } from 'react'; import { flatten } from 'flat'; import type { ResponseJSON } from '@hyperdx/common-utils/dist/clickhouse'; import { isLogSource, isTraceSource, - QuerySettings, SourceKind, TSource, } from '@hyperdx/common-utils/dist/types'; @@ -15,6 +14,12 @@ import { useQueriedChartConfig, } from '@/hooks/useChartConfig'; import { WithClause } from '@/hooks/useRowWhere'; +import { + isSelectAllColumnsSettingRejected, + markSelectAllColumnsSettingsRejected, + SELECT_ALL_COLUMNS_QUERY_SETTINGS, + useSelectAllColumnsSettingsRejected, +} from '@/hooks/useSelectAllColumnsSettingsRejection'; import { getDisplayedTimestampValueExpression, getDurationMsExpression, @@ -44,42 +49,6 @@ export enum ROW_DATA_ALIASES { SPAN_LINKS = '__hdx_span_links', } -// ClickHouse leaves MATERIALIZED and ALIAS columns out of `SELECT *` unless -// these settings are on, so the row panel would hide those columns. -const SELECT_ALL_COLUMNS_QUERY_SETTINGS: QuerySettings = [ - { setting: 'asterisk_include_materialized_columns', value: '1' }, - { setting: 'asterisk_include_alias_columns', value: '1' }, -]; - -// Connections whose user cannot change these settings (for example, a -// `readonly = 1` user). Shared by every row lookup, so each one fails once. -// Read it through useSyncExternalStore: the React Compiler memoizes plain reads. -const connectionsRejectingSelectAllSettings = new Set(); -const rejectedConnectionListeners = new Set<() => void>(); - -function subscribeToRejectedConnections(listener: () => void) { - rejectedConnectionListeners.add(listener); - return () => { - rejectedConnectionListeners.delete(listener); - }; -} - -function markConnectionRejectingSettings(connection: string) { - if (!connectionsRejectingSelectAllSettings.has(connection)) { - connectionsRejectingSelectAllSettings.add(connection); - rejectedConnectionListeners.forEach(listener => listener()); - } -} - -// ClickHouse names the setting in each rejection: READONLY (164), -// UNKNOWN_SETTING (115) and SETTING_CONSTRAINT_VIOLATION (452). -function isSelectAllSettingRejected(error: unknown): boolean { - const message = error instanceof Error ? error.message : String(error ?? ''); - return SELECT_ALL_COLUMNS_QUERY_SETTINGS.some(({ setting }) => - message.includes(setting), - ); -} - export function useRowData({ source, rowId, @@ -250,11 +219,7 @@ export function useRowData({ }; const connection = source.connection; - const rejectsSettings = useSyncExternalStore( - subscribeToRejectedConnections, - () => connectionsRejectingSelectAllSettings.has(connection), - () => false, - ); + const rejectsSettings = useSelectAllColumnsSettingsRejected(connection); // The row query has no `config.source`, so the source's query settings do not // reach it. Use the source's own value for these two settings here. const additionalQuerySettings = @@ -273,7 +238,7 @@ export function useRowData({ ? { additionalQuerySettings, retry: (failureCount: number, error: Error) => - failureCount < 1 && !isSelectAllSettingRejected(error), + failureCount < 1 && !isSelectAllColumnsSettingRejected(error), } : {}; @@ -321,10 +286,10 @@ export function useRowData({ const isSettingsRejected = additionalQuerySettings != null && queryResult.isError && - isSelectAllSettingRejected(queryResult.error); + isSelectAllColumnsSettingRejected(queryResult.error); useEffect(() => { if (isSettingsRejected) { - markConnectionRejectingSettings(connection); + markSelectAllColumnsSettingsRejected(connection); } }, [isSettingsRejected, connection]); diff --git a/packages/app/src/components/__tests__/DBRowDataPanel.test.ts b/packages/app/src/components/__tests__/DBRowDataPanel.test.ts index c9b63e25d8..1ba5bdcf96 100644 --- a/packages/app/src/components/__tests__/DBRowDataPanel.test.ts +++ b/packages/app/src/components/__tests__/DBRowDataPanel.test.ts @@ -156,10 +156,30 @@ describe('DBRowDataPanel', () => { const readonlySource = { ...source, connection: 'readonly-conn' }; mockSettingsQueryError(new Error(READONLY_ERROR)); - const { result } = renderHook(() => - useRowData({ source: readonlySource, rowId: "id='abc123'" }), - ); + const renders: Array<{ + isLoading: boolean; + isError: boolean; + error: unknown; + }> = []; + const { result } = renderHook(() => { + const rowData = useRowData({ + source: readonlySource, + rowId: "id='abc123'", + }); + renders.push({ + isLoading: rowData.isLoading, + isError: rowData.isError, + error: rowData.error, + }); + return rowData; + }); + // The render that sees the rejection reports loading, not the error. + expect(renders[0]).toEqual({ + isLoading: true, + isError: false, + error: null, + }); await waitFor(() => expect(result.current.data?.data).toEqual([row])); expect(result.current.isError).toBe(false); const calls = mockUseQueriedChartConfig.mock.calls; diff --git a/packages/app/src/connection.ts b/packages/app/src/connection.ts index 3ed38168b7..2cd5905d17 100644 --- a/packages/app/src/connection.ts +++ b/packages/app/src/connection.ts @@ -5,6 +5,7 @@ import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'; import { hdxServer } from '@/api'; import { HDX_LOCAL_DEFAULT_CONNECTIONS, IS_LOCAL_MODE } from '@/config'; +import { forgetSelectAllColumnsSettingsRejection } from '@/hooks/useSelectAllColumnsSettingsRejection'; import { parseJSON } from '@/utils'; export const LOCAL_STORE_CONNECTIONS_KEY = 'connections'; @@ -132,7 +133,8 @@ export function useUpdateConnection() { return; }, - onSuccess: () => { + onSuccess: (_data, { id }) => { + forgetSelectAllColumnsSettingsRejection(id); queryClient.invalidateQueries({ queryKey: ['connections'] }); }, }); diff --git a/packages/app/src/hooks/__tests__/useSelectAllColumnsSettingsRejection.test.ts b/packages/app/src/hooks/__tests__/useSelectAllColumnsSettingsRejection.test.ts new file mode 100644 index 0000000000..e6ddf19df1 --- /dev/null +++ b/packages/app/src/hooks/__tests__/useSelectAllColumnsSettingsRejection.test.ts @@ -0,0 +1,53 @@ +import { act, renderHook } from '@testing-library/react'; + +import { + forgetSelectAllColumnsSettingsRejection, + isSelectAllColumnsSettingRejected, + markSelectAllColumnsSettingsRejected, + useSelectAllColumnsSettingsRejected, +} from '@/hooks/useSelectAllColumnsSettingsRejection'; + +describe('isSelectAllColumnsSettingRejected', () => { + it.each([ + "Code: 164. DB::Exception: Cannot modify 'asterisk_include_materialized_columns' setting in readonly mode. (READONLY)", + 'Code: 452. DB::Exception: Setting asterisk_include_alias_columns should not be changed. (SETTING_CONSTRAINT_VIOLATION)', + "Code: 115. DB::Exception: Unknown setting 'asterisk_include_alias_columns'. (UNKNOWN_SETTING)", + ])('is true when ClickHouse rejects a setting: %s', message => { + expect(isSelectAllColumnsSettingRejected(new Error(message))).toBe(true); + }); + + it('is false for other errors', () => { + expect( + isSelectAllColumnsSettingRejected( + new Error( + 'Code: 159. DB::Exception: Timeout exceeded. (TIMEOUT_EXCEEDED)', + ), + ), + ).toBe(false); + expect(isSelectAllColumnsSettingRejected(undefined)).toBe(false); + }); +}); + +describe('useSelectAllColumnsSettingsRejected', () => { + it('follows a connection being marked and forgotten', () => { + const { result } = renderHook(() => + useSelectAllColumnsSettingsRejected('conn-a'), + ); + expect(result.current).toBe(false); + + act(() => markSelectAllColumnsSettingsRejected('conn-a')); + expect(result.current).toBe(true); + + act(() => forgetSelectAllColumnsSettingsRejection('conn-a')); + expect(result.current).toBe(false); + }); + + it('keeps connections apart', () => { + act(() => markSelectAllColumnsSettingsRejected('conn-b')); + + const { result } = renderHook(() => + useSelectAllColumnsSettingsRejected('conn-c'), + ); + expect(result.current).toBe(false); + }); +}); diff --git a/packages/app/src/hooks/useSelectAllColumnsSettingsRejection.ts b/packages/app/src/hooks/useSelectAllColumnsSettingsRejection.ts new file mode 100644 index 0000000000..e1334768a3 --- /dev/null +++ b/packages/app/src/hooks/useSelectAllColumnsSettingsRejection.ts @@ -0,0 +1,62 @@ +import { useSyncExternalStore } from 'react'; +import { QuerySettings } from '@hyperdx/common-utils/dist/types'; + +// ClickHouse leaves MATERIALIZED and ALIAS columns out of `SELECT *` unless +// these settings are on. +export const SELECT_ALL_COLUMNS_QUERY_SETTINGS: QuerySettings = [ + { setting: 'asterisk_include_materialized_columns', value: '1' }, + { setting: 'asterisk_include_alias_columns', value: '1' }, +]; + +// ClickHouse names the setting in each rejection: READONLY (164), +// UNKNOWN_SETTING (115) and SETTING_CONSTRAINT_VIOLATION (452). +export function isSelectAllColumnsSettingRejected(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error ?? ''); + return SELECT_ALL_COLUMNS_QUERY_SETTINGS.some(({ setting }) => + message.includes(setting), + ); +} + +// Connections whose user cannot change these settings (for example, a +// `readonly = 1` user), shared by every row lookup. Read it through +// useSyncExternalStore: the React Compiler memoizes plain reads. +const rejectingConnections = new Set(); +const listeners = new Set<() => void>(); + +function subscribe(listener: () => void) { + listeners.add(listener); + return () => { + listeners.delete(listener); + }; +} + +function setRejected(connection: string, rejected: boolean) { + if (rejectingConnections.has(connection) === rejected) { + return; + } + if (rejected) { + rejectingConnections.add(connection); + } else { + rejectingConnections.delete(connection); + } + listeners.forEach(listener => listener()); +} + +export function markSelectAllColumnsSettingsRejected(connection: string) { + setRejected(connection, true); +} + +// Called when a connection is edited, so its new user gets the settings again. +export function forgetSelectAllColumnsSettingsRejection(connection: string) { + setRejected(connection, false); +} + +export function useSelectAllColumnsSettingsRejected( + connection: string, +): boolean { + return useSyncExternalStore( + subscribe, + () => rejectingConnections.has(connection), + () => false, + ); +} From ddba0133a90207ec1b540fc0895d5171f256333b Mon Sep 17 00:00:00 2001 From: Bilal Atique Date: Tue, 29 Sep 2026 12:42:16 +0500 Subject: [PATCH 07/12] feat(app): make MATERIALIZED and ALIAS columns in the row panel opt-in The row query adds the two asterisk settings only when the new "Show materialized and alias columns" viewer option is on. It is off by default, because these columns usually repeat values that the panel already shows. Since #3234 the row query carries the source, so the source's query settings already win over the added ones in useQueriedChartConfig. The separate merge in useRowData is removed. --- .../row-panel-materialized-alias-columns.md | 17 +++---- .../app/src/components/DBRowDataPanel.tsx | 27 ++++------- .../src/components/DBRowJsonViewer.test.tsx | 36 +++++++++++++- .../app/src/components/DBRowJsonViewer.tsx | 25 ++++++++++ .../__tests__/DBRowDataPanel.test.ts | 47 +++++++++++-------- 5 files changed, 106 insertions(+), 46 deletions(-) diff --git a/.changeset/row-panel-materialized-alias-columns.md b/.changeset/row-panel-materialized-alias-columns.md index 0da2690d5a..781b43abfd 100644 --- a/.changeset/row-panel-materialized-alias-columns.md +++ b/.changeset/row-panel-materialized-alias-columns.md @@ -2,12 +2,13 @@ '@hyperdx/app': patch --- -fix: show MATERIALIZED and ALIAS columns in the row details panel +feat: add an option to show MATERIALIZED and ALIAS columns in the row details panel -The row details panel fetched the row with `SELECT *`, which ClickHouse runs -without MATERIALIZED and ALIAS columns, so those columns never appeared. The row -query now enables `asterisk_include_materialized_columns` and -`asterisk_include_alias_columns`, except for sources that use a Known Columns -List. A value set for either setting in the source's query settings still takes -precedence. If the connection's user cannot change these settings (for example, -`readonly = 1`), the panel loads the row without them. +ClickHouse leaves MATERIALIZED and ALIAS columns out of `SELECT *`, so the row +details panel never showed them. A new "Show materialized and alias columns" +item in the properties view options menu, off by default, adds +`asterisk_include_materialized_columns` and `asterisk_include_alias_columns` to +the row query. It has no effect on a source with a Known Columns List, and a +value that the source's query settings give for either setting wins. If the +connection's user cannot change these settings (for example, `readonly = 1`), +the panel loads the row without them. diff --git a/packages/app/src/components/DBRowDataPanel.tsx b/packages/app/src/components/DBRowDataPanel.tsx index bb1622a727..9a4c1fe0af 100644 --- a/packages/app/src/components/DBRowDataPanel.tsx +++ b/packages/app/src/components/DBRowDataPanel.tsx @@ -9,10 +9,7 @@ import { } from '@hyperdx/common-utils/dist/types'; import { Box } from '@mantine/core'; -import { - mergeQuerySettings, - useQueriedChartConfig, -} from '@/hooks/useChartConfig'; +import { useQueriedChartConfig } from '@/hooks/useChartConfig'; import { WithClause } from '@/hooks/useRowWhere'; import { isSelectAllColumnsSettingRejected, @@ -28,7 +25,10 @@ import { import { getSelectExpressionsForHighlightedAttributes } from '@/utils/highlightedAttributes'; import { getTimestampValueSelects } from '@/utils/rowTimestamps'; -import { DBRowJsonViewer } from './DBRowJsonViewer'; +import { + DBRowJsonViewer, + useShowMaterializedAliasColumns, +} from './DBRowJsonViewer'; import { getActiveInfraCorrelations } from './infraCorrelations'; // The source's own `timestampValueExpression` columns are projected too, under @@ -220,20 +220,13 @@ export function useRowData({ }; const connection = source.connection; + const showMaterializedAliasColumns = useShowMaterializedAliasColumns(); const rejectsSettings = useSelectAllColumnsSettingsRejected(connection); - // The row query has no `config.source`, so the source's query settings do not - // reach it. Use the source's own value for these two settings here. + // A value that the source's own query settings give for either setting wins. const additionalQuerySettings = - knownColumns || rejectsSettings - ? undefined - : mergeQuerySettings( - source.querySettings?.filter(({ setting }) => - SELECT_ALL_COLUMNS_QUERY_SETTINGS.some( - defaultSetting => defaultSetting.setting === setting, - ), - ), - SELECT_ALL_COLUMNS_QUERY_SETTINGS, - ); + showMaterializedAliasColumns && !knownColumns && !rejectsSettings + ? SELECT_ALL_COLUMNS_QUERY_SETTINGS + : undefined; // A rejected setting will fail again, so only other errors keep the retry. const settingsQueryOptions = additionalQuerySettings ? { diff --git a/packages/app/src/components/DBRowJsonViewer.test.tsx b/packages/app/src/components/DBRowJsonViewer.test.tsx index 32ef25dca2..cf17f5ef28 100644 --- a/packages/app/src/components/DBRowJsonViewer.test.tsx +++ b/packages/app/src/components/DBRowJsonViewer.test.tsx @@ -1,7 +1,11 @@ import React from 'react'; import { fireEvent, screen, within } from '@testing-library/react'; -import { buildJSONExtractQuery, DBRowJsonViewer } from './DBRowJsonViewer'; +import { + buildJSONExtractQuery, + DBRowJsonViewer, + useShowMaterializedAliasColumns, +} from './DBRowJsonViewer'; import { RowSidePanelContext } from './DBRowSidePanel'; // Mock Next.js router @@ -186,6 +190,36 @@ describe('DBRowJsonViewer', () => { expect(mockToggleColumn).toHaveBeenCalledWith("LogAttributes['field1']"); }); + it('turns the materialized and alias columns option on and off from the menu', async () => { + const OptionValue = () => ( + + {String(useShowMaterializedAliasColumns())} + + ); + const { container } = renderWithMantine( + + + + , + ); + const option = screen.getByTestId('materialized-alias-option'); + const toggle = async () => { + fireEvent.click(container.querySelector('[aria-haspopup="menu"]')!); + fireEvent.click( + await screen.findByTestId('json-viewer-materialized-alias-toggle'), + ); + }; + + expect(option).toHaveTextContent('false'); + await toggle(); + expect(option).toHaveTextContent('true'); + expect( + JSON.parse(localStorage.getItem('hdx_json_viewer_options')!), + ).toMatchObject({ showMaterializedAliasColumns: true }); + await toggle(); + expect(option).toHaveTextContent('false'); + }); + describe('timestamp fields', () => { it('displays Timestamp using the same formatter as the results table', () => { renderComponent({ diff --git a/packages/app/src/components/DBRowJsonViewer.tsx b/packages/app/src/components/DBRowJsonViewer.tsx index f8e2ec2eaf..1269df6335 100644 --- a/packages/app/src/components/DBRowJsonViewer.tsx +++ b/packages/app/src/components/DBRowJsonViewer.tsx @@ -132,6 +132,8 @@ type ViewerOptions = { whiteSpace?: 'pre' | 'pre-wrap'; tabulate: boolean; filterBlanks: boolean; + // Off when absent, as in options saved before it existed. + showMaterializedAliasColumns?: boolean; }; const VIEWER_OPTIONS_KEY = 'hdx_json_viewer_options'; @@ -227,6 +229,10 @@ const viewerOptionsAtom = atomWithStorage( viewerOptionsStorage, ); +export function useShowMaterializedAliasColumns(): boolean { + return useAtomValue(viewerOptionsAtom).showMaterializedAliasColumns === true; +} + function HyperJsonMenu({ rowData }: { rowData: any }) { const [jsonOptions, setJsonOptions] = useAtom(viewerOptionsAtom); const effectiveWhiteSpace = jsonOptions.whiteSpace ?? 'pre-wrap'; @@ -335,6 +341,25 @@ function HyperJsonMenu({ rowData }: { rowData: any }) { > Hide blank values + + ) : null + } + onClick={() => + setJsonOptions({ + ...jsonOptions, + showMaterializedAliasColumns: + !jsonOptions.showMaterializedAliasColumns, + }) + } + > + Show materialized and alias columns + diff --git a/packages/app/src/components/__tests__/DBRowDataPanel.test.ts b/packages/app/src/components/__tests__/DBRowDataPanel.test.ts index 10a0e2f61f..768b3afbe3 100644 --- a/packages/app/src/components/__tests__/DBRowDataPanel.test.ts +++ b/packages/app/src/components/__tests__/DBRowDataPanel.test.ts @@ -6,15 +6,22 @@ import { getMapColumnNames, useRowData, } from '@/components/DBRowDataPanel'; +import { useShowMaterializedAliasColumns } from '@/components/DBRowJsonViewer'; import { useQueriedChartConfig } from '@/hooks/useChartConfig'; jest.mock('@/hooks/useChartConfig', () => ({ - mergeQuerySettings: jest.requireActual('@/hooks/useChartConfig') - .mergeQuerySettings, useQueriedChartConfig: jest.fn(), })); +jest.mock('@/components/DBRowJsonViewer', () => ({ + DBRowJsonViewer: () => null, + useShowMaterializedAliasColumns: jest.fn(), +})); + const mockUseQueriedChartConfig = useQueriedChartConfig as jest.Mock; +const mockUseShowMaterializedAliasColumns = jest.mocked( + useShowMaterializedAliasColumns, +); describe('DBRowDataPanel', () => { const source: TLogSource = { @@ -30,6 +37,7 @@ describe('DBRowDataPanel', () => { beforeEach(() => { jest.clearAllMocks(); + mockUseShowMaterializedAliasColumns.mockReturnValue(false); mockUseQueriedChartConfig.mockReturnValue({ data: { data: [], @@ -95,7 +103,18 @@ describe('DBRowDataPanel', () => { expect(config.select).not.toContainEqual({ valueExpression: '*' }); }); - it('includes MATERIALIZED and ALIAS columns when selecting `*`', () => { + it('adds no query settings while the viewer option is off', () => { + renderHook(() => useRowData({ source, rowId: "id='abc123'" })); + + const [bounded, fallback] = mockUseQueriedChartConfig.mock.calls; + for (const [, options] of [bounded, fallback]) { + expect(options.additionalQuerySettings).toBeUndefined(); + expect(options.retry).toBeUndefined(); + } + }); + + it('includes MATERIALIZED and ALIAS columns when the viewer option is on', () => { + mockUseShowMaterializedAliasColumns.mockReturnValue(true); renderHook(() => useRowData({ source, rowId: "id='abc123'" })); const [bounded, fallback] = mockUseQueriedChartConfig.mock.calls; @@ -107,24 +126,8 @@ describe('DBRowDataPanel', () => { } }); - it("uses the source's own value for these settings", () => { - const sourceWithSetting: TLogSource = { - ...source, - querySettings: [{ setting: 'asterisk_include_alias_columns', value: '0' }], - }; - - renderHook(() => - useRowData({ source: sourceWithSetting, rowId: "id='abc123'" }), - ); - - const [[, options]] = mockUseQueriedChartConfig.mock.calls; - expect(options.additionalQuerySettings).toEqual([ - { setting: 'asterisk_include_alias_columns', value: '0' }, - { setting: 'asterisk_include_materialized_columns', value: '1' }, - ]); - }); - it('adds no query settings when the source has a Known Columns List', () => { + mockUseShowMaterializedAliasColumns.mockReturnValue(true); const sourceWithKnownColumns: TLogSource = { ...source, knownColumnsListExpression: 'Timestamp, Body, ServiceName', @@ -141,6 +144,10 @@ describe('DBRowDataPanel', () => { }); describe('when the query with the settings fails', () => { + beforeEach(() => { + mockUseShowMaterializedAliasColumns.mockReturnValue(true); + }); + const READONLY_ERROR = "Code: 164. DB::Exception: Cannot modify 'asterisk_include_materialized_columns' setting in readonly mode. (READONLY)"; const row = { Body: 'hello' }; From 04232a733393f1a5e658a6236327618719a6a4cc Mon Sep 17 00:00:00 2001 From: Bilal Atique Date: Tue, 29 Sep 2026 13:45:22 +0500 Subject: [PATCH 08/12] fix(app): count only a ClickHouse settings rejection as rejected A syntax error or an analyzer error quotes the query, SETTINGS clause included, so a message that names an asterisk setting is not enough. Require the parsed ClickHouse error type too (READONLY, UNKNOWN_SETTING, SETTING_CONSTRAINT_VIOLATION), read from the error or its cause, as the max_rows_to_read check in ClickhouseClient.query does. --- .../__tests__/DBRowDataPanel.test.ts | 18 +++-- ...eSelectAllColumnsSettingsRejection.test.ts | 66 ++++++++++++++++--- .../useSelectAllColumnsSettingsRejection.ts | 33 +++++++++- 3 files changed, 103 insertions(+), 14 deletions(-) diff --git a/packages/app/src/components/__tests__/DBRowDataPanel.test.ts b/packages/app/src/components/__tests__/DBRowDataPanel.test.ts index 768b3afbe3..202589de40 100644 --- a/packages/app/src/components/__tests__/DBRowDataPanel.test.ts +++ b/packages/app/src/components/__tests__/DBRowDataPanel.test.ts @@ -1,3 +1,4 @@ +import { ClickHouseQueryError } from '@hyperdx/common-utils/dist/clickhouse'; import { SourceKind, TLogSource } from '@hyperdx/common-utils/dist/types'; import { renderHook, waitFor } from '@testing-library/react'; @@ -148,8 +149,17 @@ describe('DBRowDataPanel', () => { mockUseShowMaterializedAliasColumns.mockReturnValue(true); }); - const READONLY_ERROR = - "Code: 164. DB::Exception: Cannot modify 'asterisk_include_materialized_columns' setting in readonly mode. (READONLY)"; + // The client's parsed error, wrapped the way the row query sees it. + const readonlyError = () => { + const message = + "Cannot modify 'asterisk_include_materialized_columns' setting in readonly mode. "; + const error = new ClickHouseQueryError(message, 'SELECT * LIMIT 1'); + error.cause = Object.assign(new Error(message), { + code: '164', + type: 'READONLY', + }); + return error; + }; const row = { Body: 'hello' }; // Fails every query that carries the settings with `error`. @@ -177,7 +187,7 @@ describe('DBRowDataPanel', () => { it('fetches the row again without them when the connection rejects them', async () => { const readonlySource = { ...source, connection: 'readonly-conn' }; - mockSettingsQueryError(new Error(READONLY_ERROR)); + mockSettingsQueryError(readonlyError()); const renders: Array<{ isLoading: boolean; @@ -243,7 +253,7 @@ describe('DBRowDataPanel', () => { const [[, { retry }]] = mockUseQueriedChartConfig.mock.calls; expect(retry(0, new Error('Timeout exceeded'))).toBe(true); expect(retry(1, new Error('Timeout exceeded'))).toBe(false); - expect(retry(0, new Error(READONLY_ERROR))).toBe(false); + expect(retry(0, readonlyError())).toBe(false); }); }); diff --git a/packages/app/src/hooks/__tests__/useSelectAllColumnsSettingsRejection.test.ts b/packages/app/src/hooks/__tests__/useSelectAllColumnsSettingsRejection.test.ts index e6ddf19df1..c881a4e03f 100644 --- a/packages/app/src/hooks/__tests__/useSelectAllColumnsSettingsRejection.test.ts +++ b/packages/app/src/hooks/__tests__/useSelectAllColumnsSettingsRejection.test.ts @@ -1,3 +1,4 @@ +import { ClickHouseQueryError } from '@hyperdx/common-utils/dist/clickhouse'; import { act, renderHook } from '@testing-library/react'; import { @@ -7,23 +8,72 @@ import { useSelectAllColumnsSettingsRejected, } from '@/hooks/useSelectAllColumnsSettingsRejection'; +const ROW_QUERY = + 'SELECT *, Timestamp AS __hdx_timestamp FROM default.otel_logs WHERE id = 1 LIMIT 1 SETTINGS asterisk_include_materialized_columns = 1, asterisk_include_alias_columns = 1'; + +// The shape the row query sees: the client's parsed error (message without the +// code, plus `code` and `type`) wrapped in a ClickHouseQueryError. +function queryError(message: string, code: string, type: string) { + const error = new ClickHouseQueryError(message, ROW_QUERY); + error.cause = Object.assign(new Error(message), { code, type }); + return error; +} + describe('isSelectAllColumnsSettingRejected', () => { it.each([ - "Code: 164. DB::Exception: Cannot modify 'asterisk_include_materialized_columns' setting in readonly mode. (READONLY)", - 'Code: 452. DB::Exception: Setting asterisk_include_alias_columns should not be changed. (SETTING_CONSTRAINT_VIOLATION)', - "Code: 115. DB::Exception: Unknown setting 'asterisk_include_alias_columns'. (UNKNOWN_SETTING)", - ])('is true when ClickHouse rejects a setting: %s', message => { - expect(isSelectAllColumnsSettingRejected(new Error(message))).toBe(true); + [ + "Cannot modify 'asterisk_include_materialized_columns' setting in readonly mode. ", + '164', + 'READONLY', + ], + [ + 'Setting asterisk_include_alias_columns should not be changed. ', + '452', + 'SETTING_CONSTRAINT_VIOLATION', + ], + [ + "Unknown setting 'asterisk_include_alias_columns'. ", + '115', + 'UNKNOWN_SETTING', + ], + ])('is true when ClickHouse rejects a setting: %s', (message, code, type) => { + expect( + isSelectAllColumnsSettingRejected(queryError(message, code, type)), + ).toBe(true); }); - it('is false for other errors', () => { + it('is false for an error that quotes the SETTINGS clause', () => { + const syntaxError = queryError( + `Syntax error: failed at position 88 ('LIMT') (line 1, col 88): LIMT 1 SETTINGS asterisk_include_materialized_columns = 1, asterisk_include_alias_columns = 1. Expected one of: LIMIT, OFFSET, SETTINGS. `, + '62', + 'SYNTAX_ERROR', + ); + expect(isSelectAllColumnsSettingRejected(syntaxError)).toBe(false); + }); + + it('is false when a different setting is rejected', () => { expect( isSelectAllColumnsSettingRejected( - new Error( - 'Code: 159. DB::Exception: Timeout exceeded. (TIMEOUT_EXCEEDED)', + queryError( + "Cannot modify 'max_rows_to_read' setting in readonly mode. ", + '164', + 'READONLY', ), ), ).toBe(false); + }); + + it('is false for other errors', () => { + expect( + isSelectAllColumnsSettingRejected( + queryError('Timeout exceeded. ', '159', 'TIMEOUT_EXCEEDED'), + ), + ).toBe(false); + expect( + isSelectAllColumnsSettingRejected( + new Error("Unknown setting 'asterisk_include_alias_columns'"), + ), + ).toBe(false); expect(isSelectAllColumnsSettingRejected(undefined)).toBe(false); }); }); diff --git a/packages/app/src/hooks/useSelectAllColumnsSettingsRejection.ts b/packages/app/src/hooks/useSelectAllColumnsSettingsRejection.ts index e1334768a3..00e07c1b2d 100644 --- a/packages/app/src/hooks/useSelectAllColumnsSettingsRejection.ts +++ b/packages/app/src/hooks/useSelectAllColumnsSettingsRejection.ts @@ -8,9 +8,38 @@ export const SELECT_ALL_COLUMNS_QUERY_SETTINGS: QuerySettings = [ { setting: 'asterisk_include_alias_columns', value: '1' }, ]; -// ClickHouse names the setting in each rejection: READONLY (164), -// UNKNOWN_SETTING (115) and SETTING_CONSTRAINT_VIOLATION (452). +const REJECTION_TYPES = new Set([ + 'READONLY', + 'UNKNOWN_SETTING', + 'SETTING_CONSTRAINT_VIOLATION', +]); + +// The ClickHouse client parses `type` out of the message, and +// ClickHouseQueryError keeps the parsed error as its `cause`. +function clickHouseErrorType(error: unknown): string | undefined { + for (const candidate of [ + error, + error instanceof Error ? error.cause : undefined, + ]) { + if ( + typeof candidate === 'object' && + candidate !== null && + 'type' in candidate && + typeof candidate.type === 'string' + ) { + return candidate.type; + } + } + return undefined; +} + +// A rejection names the setting. The type check is needed too: a syntax error +// quotes the rest of the query, SETTINGS clause included. export function isSelectAllColumnsSettingRejected(error: unknown): boolean { + const type = clickHouseErrorType(error); + if (type == null || !REJECTION_TYPES.has(type)) { + return false; + } const message = error instanceof Error ? error.message : String(error ?? ''); return SELECT_ALL_COLUMNS_QUERY_SETTINGS.some(({ setting }) => message.includes(setting), From 3f1487636a58a464d0d0b9bf4d709a0fdaaf66f6 Mon Sep 17 00:00:00 2001 From: Bilal Atique Date: Tue, 29 Sep 2026 14:30:53 +0500 Subject: [PATCH 09/12] fix(app): offer to turn the option off when a row fails to load The option lives in the Column Values menu, which the row error state replaces. When a column that the option adds fails to evaluate, the error state now offers "Hide materialized and alias columns". The option moves to hooks/useMaterializedAliasColumnsOption.ts (useLocalStorage, like the other panel preferences), so the menu, the row query and the error state share it without importing the viewer. --- .../row-panel-materialized-alias-columns.md | 3 +- .../app/src/components/DBRowDataPanel.tsx | 24 +++------ .../src/components/DBRowJsonViewer.test.tsx | 14 +++--- .../app/src/components/DBRowJsonViewer.tsx | 17 ++----- .../components/DBRowSidePanelErrorState.tsx | 35 +++++++++++++ .../__tests__/DBRowDataPanel.test.ts | 19 +++---- .../DBRowSidePanelErrorState.test.tsx | 50 ++++++++++++++++++- .../useMaterializedAliasColumnsOption.ts | 39 +++++++++++++++ 8 files changed, 149 insertions(+), 52 deletions(-) create mode 100644 packages/app/src/hooks/useMaterializedAliasColumnsOption.ts diff --git a/.changeset/row-panel-materialized-alias-columns.md b/.changeset/row-panel-materialized-alias-columns.md index 781b43abfd..4f6788a745 100644 --- a/.changeset/row-panel-materialized-alias-columns.md +++ b/.changeset/row-panel-materialized-alias-columns.md @@ -11,4 +11,5 @@ item in the properties view options menu, off by default, adds the row query. It has no effect on a source with a Known Columns List, and a value that the source's query settings give for either setting wins. If the connection's user cannot change these settings (for example, `readonly = 1`), -the panel loads the row without them. +the panel loads the row without them. If the row fails to load for another +reason while the option is on, the error state offers to turn the option off. diff --git a/packages/app/src/components/DBRowDataPanel.tsx b/packages/app/src/components/DBRowDataPanel.tsx index 9a4c1fe0af..df2907d427 100644 --- a/packages/app/src/components/DBRowDataPanel.tsx +++ b/packages/app/src/components/DBRowDataPanel.tsx @@ -10,12 +10,14 @@ import { import { Box } from '@mantine/core'; import { useQueriedChartConfig } from '@/hooks/useChartConfig'; +import { + getKnownColumnsList, + useSelectAllColumnsQuerySettings, +} from '@/hooks/useMaterializedAliasColumnsOption'; import { WithClause } from '@/hooks/useRowWhere'; import { isSelectAllColumnsSettingRejected, markSelectAllColumnsSettingsRejected, - SELECT_ALL_COLUMNS_QUERY_SETTINGS, - useSelectAllColumnsSettingsRejected, } from '@/hooks/useSelectAllColumnsSettingsRejection'; import { getDisplayedTimestampValueExpression, @@ -25,10 +27,7 @@ import { import { getSelectExpressionsForHighlightedAttributes } from '@/utils/highlightedAttributes'; import { getTimestampValueSelects } from '@/utils/rowTimestamps'; -import { - DBRowJsonViewer, - useShowMaterializedAliasColumns, -} from './DBRowJsonViewer'; +import { DBRowJsonViewer } from './DBRowJsonViewer'; import { getActiveInfraCorrelations } from './infraCorrelations'; // The source's own `timestampValueExpression` columns are projected too, under @@ -99,10 +98,7 @@ export function useRowData({ // target tables declare different column sets. When the source declares a // "known columns" list (columns known to exist across all target tables) we // select that instead of `*` when fetching full row data. - const knownColumns = - isLogSource(source) || isTraceSource(source) - ? source.knownColumnsListExpression?.trim() - : undefined; + const knownColumns = getKnownColumnsList(source); const baseConfig = { source: source.id, @@ -220,13 +216,7 @@ export function useRowData({ }; const connection = source.connection; - const showMaterializedAliasColumns = useShowMaterializedAliasColumns(); - const rejectsSettings = useSelectAllColumnsSettingsRejected(connection); - // A value that the source's own query settings give for either setting wins. - const additionalQuerySettings = - showMaterializedAliasColumns && !knownColumns && !rejectsSettings - ? SELECT_ALL_COLUMNS_QUERY_SETTINGS - : undefined; + const additionalQuerySettings = useSelectAllColumnsQuerySettings(source); // A rejected setting will fail again, so only other errors keep the retry. const settingsQueryOptions = additionalQuerySettings ? { diff --git a/packages/app/src/components/DBRowJsonViewer.test.tsx b/packages/app/src/components/DBRowJsonViewer.test.tsx index cf17f5ef28..4c0a3dc3f8 100644 --- a/packages/app/src/components/DBRowJsonViewer.test.tsx +++ b/packages/app/src/components/DBRowJsonViewer.test.tsx @@ -1,11 +1,9 @@ import React from 'react'; import { fireEvent, screen, within } from '@testing-library/react'; -import { - buildJSONExtractQuery, - DBRowJsonViewer, - useShowMaterializedAliasColumns, -} from './DBRowJsonViewer'; +import { useMaterializedAliasColumnsOption } from '@/hooks/useMaterializedAliasColumnsOption'; + +import { buildJSONExtractQuery, DBRowJsonViewer } from './DBRowJsonViewer'; import { RowSidePanelContext } from './DBRowSidePanel'; // Mock Next.js router @@ -193,7 +191,7 @@ describe('DBRowJsonViewer', () => { it('turns the materialized and alias columns option on and off from the menu', async () => { const OptionValue = () => ( - {String(useShowMaterializedAliasColumns())} + {String(useMaterializedAliasColumnsOption()[0])} ); const { container } = renderWithMantine( @@ -214,8 +212,8 @@ describe('DBRowJsonViewer', () => { await toggle(); expect(option).toHaveTextContent('true'); expect( - JSON.parse(localStorage.getItem('hdx_json_viewer_options')!), - ).toMatchObject({ showMaterializedAliasColumns: true }); + localStorage.getItem('hdx-row-show-materialized-alias-columns'), + ).toBe('true'); await toggle(); expect(option).toHaveTextContent('false'); }); diff --git a/packages/app/src/components/DBRowJsonViewer.tsx b/packages/app/src/components/DBRowJsonViewer.tsx index 1269df6335..fb5ec9dc97 100644 --- a/packages/app/src/components/DBRowJsonViewer.tsx +++ b/packages/app/src/components/DBRowJsonViewer.tsx @@ -32,6 +32,7 @@ import HyperJson, { GetLineActions, LineAction, } from '@/components/HyperJson'; +import { useMaterializedAliasColumnsOption } from '@/hooks/useMaterializedAliasColumnsOption'; import { useFormatTime } from '@/useFormatTime'; import { isColumnInSelect, mergePath } from '@/utils'; import { @@ -132,8 +133,6 @@ type ViewerOptions = { whiteSpace?: 'pre' | 'pre-wrap'; tabulate: boolean; filterBlanks: boolean; - // Off when absent, as in options saved before it existed. - showMaterializedAliasColumns?: boolean; }; const VIEWER_OPTIONS_KEY = 'hdx_json_viewer_options'; @@ -229,12 +228,10 @@ const viewerOptionsAtom = atomWithStorage( viewerOptionsStorage, ); -export function useShowMaterializedAliasColumns(): boolean { - return useAtomValue(viewerOptionsAtom).showMaterializedAliasColumns === true; -} - function HyperJsonMenu({ rowData }: { rowData: any }) { const [jsonOptions, setJsonOptions] = useAtom(viewerOptionsAtom); + const [showMaterializedAliasColumns, setShowMaterializedAliasColumns] = + useMaterializedAliasColumnsOption(); const effectiveWhiteSpace = jsonOptions.whiteSpace ?? 'pre-wrap'; return ( @@ -346,16 +343,12 @@ function HyperJsonMenu({ rowData }: { rowData: any }) { py={8} data-testid="json-viewer-materialized-alias-toggle" rightSection={ - jsonOptions.showMaterializedAliasColumns ? ( + showMaterializedAliasColumns ? ( ) : null } onClick={() => - setJsonOptions({ - ...jsonOptions, - showMaterializedAliasColumns: - !jsonOptions.showMaterializedAliasColumns, - }) + setShowMaterializedAliasColumns(!showMaterializedAliasColumns) } > Show materialized and alias columns diff --git a/packages/app/src/components/DBRowSidePanelErrorState.tsx b/packages/app/src/components/DBRowSidePanelErrorState.tsx index 19ed40599b..d8a9af6add 100644 --- a/packages/app/src/components/DBRowSidePanelErrorState.tsx +++ b/packages/app/src/components/DBRowSidePanelErrorState.tsx @@ -14,6 +14,10 @@ import { useDisclosure } from '@mantine/hooks'; import { IconAlertTriangle } from '@tabler/icons-react'; import { IS_LOCAL_MODE } from '@/config'; +import { + useMaterializedAliasColumnsOption, + useSelectAllColumnsQuerySettings, +} from '@/hooks/useMaterializedAliasColumnsOption'; import { useTableMetadata } from '@/hooks/useMetadata'; import { useBrandDisplayName } from '@/theme/ThemeProvider'; @@ -82,6 +86,28 @@ function KnownColumnsListHint({ ); } +// The option lives in the Column Values menu, which a failed row does not show. +function MaterializedAliasColumnsHint({ onHide }: { onHide: () => void }) { + return ( + } + title="Materialized and alias columns are on" + data-testid="materialized-alias-columns-hint" + > + + + The row query includes MATERIALIZED and ALIAS columns. If one of them + cannot be evaluated, the whole row fails to load. + + + + + ); +} + export function DBRowSidePanelErrorState({ error, source, @@ -94,6 +120,9 @@ export function DBRowSidePanelErrorState({ const showHint = isMissingColumnError(error) && !!tableMetadata?.isPointerTable; + const selectAllColumnsSettings = useSelectAllColumnsQuerySettings(source); + const [, setShowMaterializedAliasColumns] = + useMaterializedAliasColumnsOption(); return ( @@ -103,6 +132,12 @@ export function DBRowSidePanelErrorState({ )} + {selectAllColumnsSettings != null && ( + setShowMaterializedAliasColumns(false)} + /> + )} + Error Message: diff --git a/packages/app/src/components/__tests__/DBRowDataPanel.test.ts b/packages/app/src/components/__tests__/DBRowDataPanel.test.ts index 202589de40..66fff9744e 100644 --- a/packages/app/src/components/__tests__/DBRowDataPanel.test.ts +++ b/packages/app/src/components/__tests__/DBRowDataPanel.test.ts @@ -7,22 +7,15 @@ import { getMapColumnNames, useRowData, } from '@/components/DBRowDataPanel'; -import { useShowMaterializedAliasColumns } from '@/components/DBRowJsonViewer'; import { useQueriedChartConfig } from '@/hooks/useChartConfig'; jest.mock('@/hooks/useChartConfig', () => ({ useQueriedChartConfig: jest.fn(), })); -jest.mock('@/components/DBRowJsonViewer', () => ({ - DBRowJsonViewer: () => null, - useShowMaterializedAliasColumns: jest.fn(), -})); - const mockUseQueriedChartConfig = useQueriedChartConfig as jest.Mock; -const mockUseShowMaterializedAliasColumns = jest.mocked( - useShowMaterializedAliasColumns, -); +const showMaterializedAliasColumns = () => + localStorage.setItem('hdx-row-show-materialized-alias-columns', 'true'); describe('DBRowDataPanel', () => { const source: TLogSource = { @@ -38,7 +31,7 @@ describe('DBRowDataPanel', () => { beforeEach(() => { jest.clearAllMocks(); - mockUseShowMaterializedAliasColumns.mockReturnValue(false); + localStorage.clear(); mockUseQueriedChartConfig.mockReturnValue({ data: { data: [], @@ -115,7 +108,7 @@ describe('DBRowDataPanel', () => { }); it('includes MATERIALIZED and ALIAS columns when the viewer option is on', () => { - mockUseShowMaterializedAliasColumns.mockReturnValue(true); + showMaterializedAliasColumns(); renderHook(() => useRowData({ source, rowId: "id='abc123'" })); const [bounded, fallback] = mockUseQueriedChartConfig.mock.calls; @@ -128,7 +121,7 @@ describe('DBRowDataPanel', () => { }); it('adds no query settings when the source has a Known Columns List', () => { - mockUseShowMaterializedAliasColumns.mockReturnValue(true); + showMaterializedAliasColumns(); const sourceWithKnownColumns: TLogSource = { ...source, knownColumnsListExpression: 'Timestamp, Body, ServiceName', @@ -146,7 +139,7 @@ describe('DBRowDataPanel', () => { describe('when the query with the settings fails', () => { beforeEach(() => { - mockUseShowMaterializedAliasColumns.mockReturnValue(true); + showMaterializedAliasColumns(); }); // The client's parsed error, wrapped the way the row query sees it. diff --git a/packages/app/src/components/__tests__/DBRowSidePanelErrorState.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanelErrorState.test.tsx index f67795ff90..66a24e525e 100644 --- a/packages/app/src/components/__tests__/DBRowSidePanelErrorState.test.tsx +++ b/packages/app/src/components/__tests__/DBRowSidePanelErrorState.test.tsx @@ -2,7 +2,7 @@ import React from 'react'; import { ClickHouseQueryError } from '@hyperdx/common-utils/dist/clickhouse'; import { SourceKind, TLogSource } from '@hyperdx/common-utils/dist/types'; import { MantineProvider } from '@mantine/core'; -import { render, screen } from '@testing-library/react'; +import { fireEvent, render, screen } from '@testing-library/react'; import { DBRowSidePanelErrorState } from '@/components/DBRowSidePanelErrorState'; @@ -129,4 +129,52 @@ describe('DBRowSidePanelErrorState', () => { ), ).toBeNull(); }); + + describe('materialized and alias columns hint', () => { + const OPTION_KEY = 'hdx-row-show-materialized-alias-columns'; + const aliasError = new ClickHouseQueryError( + "Dictionary ('default.missing') not found", + 'SELECT * FROM logs LIMIT 1 SETTINGS asterisk_include_alias_columns = 1', + ); + + beforeEach(() => { + localStorage.clear(); + }); + + it('offers to hide the columns when the row query added them', () => { + localStorage.setItem(OPTION_KEY, 'true'); + renderErrorState(aliasError); + + fireEvent.click( + screen.getByRole('button', { + name: 'Hide materialized and alias columns', + }), + ); + + expect(localStorage.getItem(OPTION_KEY)).toBe('false'); + expect( + screen.queryByTestId('materialized-alias-columns-hint'), + ).toBeNull(); + }); + + it('is not shown while the option is off', () => { + renderErrorState(aliasError); + + expect( + screen.queryByTestId('materialized-alias-columns-hint'), + ).toBeNull(); + }); + + it('is not shown when a Known Columns List replaces SELECT *', () => { + localStorage.setItem(OPTION_KEY, 'true'); + renderErrorState(aliasError, { + ...source, + knownColumnsListExpression: 'Timestamp, Body', + }); + + expect( + screen.queryByTestId('materialized-alias-columns-hint'), + ).toBeNull(); + }); + }); }); diff --git a/packages/app/src/hooks/useMaterializedAliasColumnsOption.ts b/packages/app/src/hooks/useMaterializedAliasColumnsOption.ts new file mode 100644 index 0000000000..c2925e108c --- /dev/null +++ b/packages/app/src/hooks/useMaterializedAliasColumnsOption.ts @@ -0,0 +1,39 @@ +import { + isLogSource, + isTraceSource, + QuerySettings, + TSource, +} from '@hyperdx/common-utils/dist/types'; + +import { + SELECT_ALL_COLUMNS_QUERY_SETTINGS, + useSelectAllColumnsSettingsRejected, +} from '@/hooks/useSelectAllColumnsSettingsRejection'; +import { useLocalStorage } from '@/utils'; + +export function useMaterializedAliasColumnsOption() { + return useLocalStorage('hdx-row-show-materialized-alias-columns', false); +} + +// A Known Columns List replaces `SELECT *` in the row query. +export function getKnownColumnsList(source: TSource): string | undefined { + return isLogSource(source) || isTraceSource(source) + ? source.knownColumnsListExpression?.trim() || undefined + : undefined; +} + +// The settings that the row query adds for this source, if any. A value that +// the source's own query settings give for either setting wins. +export function useSelectAllColumnsQuerySettings( + source: TSource, +): QuerySettings | undefined { + const [showMaterializedAliasColumns] = useMaterializedAliasColumnsOption(); + const rejectsSettings = useSelectAllColumnsSettingsRejected( + source.connection, + ); + return showMaterializedAliasColumns && + !getKnownColumnsList(source) && + !rejectsSettings + ? SELECT_ALL_COLUMNS_QUERY_SETTINGS + : undefined; +} From 77c7851331e086317f9e7faaca273ede77bd8c36 Mon Sep 17 00:00:00 2001 From: Bilal Atique Date: Tue, 29 Sep 2026 14:38:39 +0500 Subject: [PATCH 10/12] fix(app): match the row hint to the settings the query adds useSelectAllColumnsQuerySettings now returns only the settings that the source's own query settings do not define, and none when the source defines both. The error-state hint then shows only when the option really changed the query. The new hint uses the warning Alert variant. --- .../components/DBRowSidePanelErrorState.tsx | 2 +- .../__tests__/DBRowDataPanel.test.ts | 37 +++++++++++++++++++ .../DBRowSidePanelErrorState.test.tsx | 15 ++++++++ .../useMaterializedAliasColumnsOption.ts | 15 ++++++-- 4 files changed, 64 insertions(+), 5 deletions(-) diff --git a/packages/app/src/components/DBRowSidePanelErrorState.tsx b/packages/app/src/components/DBRowSidePanelErrorState.tsx index d8a9af6add..c7645ddb52 100644 --- a/packages/app/src/components/DBRowSidePanelErrorState.tsx +++ b/packages/app/src/components/DBRowSidePanelErrorState.tsx @@ -90,7 +90,7 @@ function KnownColumnsListHint({ function MaterializedAliasColumnsHint({ onHide }: { onHide: () => void }) { return ( } title="Materialized and alias columns are on" data-testid="materialized-alias-columns-hint" diff --git a/packages/app/src/components/__tests__/DBRowDataPanel.test.ts b/packages/app/src/components/__tests__/DBRowDataPanel.test.ts index 66fff9744e..21c7707d1f 100644 --- a/packages/app/src/components/__tests__/DBRowDataPanel.test.ts +++ b/packages/app/src/components/__tests__/DBRowDataPanel.test.ts @@ -120,6 +120,43 @@ describe('DBRowDataPanel', () => { } }); + it('adds only the settings that the source does not set itself', () => { + showMaterializedAliasColumns(); + const sourceWithSetting: TLogSource = { + ...source, + querySettings: [ + { setting: 'asterisk_include_alias_columns', value: '0' }, + ], + }; + + renderHook(() => + useRowData({ source: sourceWithSetting, rowId: "id='abc123'" }), + ); + + const [[, options]] = mockUseQueriedChartConfig.mock.calls; + expect(options.additionalQuerySettings).toEqual([ + { setting: 'asterisk_include_materialized_columns', value: '1' }, + ]); + }); + + it('adds no query settings when the source sets both itself', () => { + showMaterializedAliasColumns(); + const sourceWithSettings: TLogSource = { + ...source, + querySettings: [ + { setting: 'asterisk_include_materialized_columns', value: '0' }, + { setting: 'asterisk_include_alias_columns', value: '0' }, + ], + }; + + renderHook(() => + useRowData({ source: sourceWithSettings, rowId: "id='abc123'" }), + ); + + const [[, options]] = mockUseQueriedChartConfig.mock.calls; + expect(options.additionalQuerySettings).toBeUndefined(); + }); + it('adds no query settings when the source has a Known Columns List', () => { showMaterializedAliasColumns(); const sourceWithKnownColumns: TLogSource = { diff --git a/packages/app/src/components/__tests__/DBRowSidePanelErrorState.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanelErrorState.test.tsx index 66a24e525e..43334bd8bb 100644 --- a/packages/app/src/components/__tests__/DBRowSidePanelErrorState.test.tsx +++ b/packages/app/src/components/__tests__/DBRowSidePanelErrorState.test.tsx @@ -165,6 +165,21 @@ describe('DBRowSidePanelErrorState', () => { ).toBeNull(); }); + it('is not shown when the source sets both settings itself', () => { + localStorage.setItem(OPTION_KEY, 'true'); + renderErrorState(aliasError, { + ...source, + querySettings: [ + { setting: 'asterisk_include_materialized_columns', value: '0' }, + { setting: 'asterisk_include_alias_columns', value: '0' }, + ], + }); + + expect( + screen.queryByTestId('materialized-alias-columns-hint'), + ).toBeNull(); + }); + it('is not shown when a Known Columns List replaces SELECT *', () => { localStorage.setItem(OPTION_KEY, 'true'); renderErrorState(aliasError, { diff --git a/packages/app/src/hooks/useMaterializedAliasColumnsOption.ts b/packages/app/src/hooks/useMaterializedAliasColumnsOption.ts index c2925e108c..0d78b8deb5 100644 --- a/packages/app/src/hooks/useMaterializedAliasColumnsOption.ts +++ b/packages/app/src/hooks/useMaterializedAliasColumnsOption.ts @@ -22,8 +22,8 @@ export function getKnownColumnsList(source: TSource): string | undefined { : undefined; } -// The settings that the row query adds for this source, if any. A value that -// the source's own query settings give for either setting wins. +// The settings that the row query adds for this source, if any. A setting that +// the source's own query settings define keeps the source's value. export function useSelectAllColumnsQuerySettings( source: TSource, ): QuerySettings | undefined { @@ -31,9 +31,16 @@ export function useSelectAllColumnsQuerySettings( const rejectsSettings = useSelectAllColumnsSettingsRejected( source.connection, ); + const added = SELECT_ALL_COLUMNS_QUERY_SETTINGS.filter( + ({ setting }) => + !source.querySettings?.some( + sourceSetting => sourceSetting.setting === setting, + ), + ); return showMaterializedAliasColumns && !getKnownColumnsList(source) && - !rejectsSettings - ? SELECT_ALL_COLUMNS_QUERY_SETTINGS + !rejectsSettings && + added.length > 0 + ? added : undefined; } From b858fc03245caa3f9de483769f0b7b32bfed97d0 Mon Sep 17 00:00:00 2001 From: Bilal Atique Date: Tue, 29 Sep 2026 23:21:22 +0500 Subject: [PATCH 11/12] fix(app): report a rejected row setting instead of retrying without it The option is opt-in, so a row query that the connection rejects (for example, a readonly user) now fails and shows the error state, which offers to turn the option off. This removes the per-connection rejection store, the retry without the settings, and the store reset on connection edit. --- .../row-panel-materialized-alias-columns.md | 8 +- .../app/src/components/DBRowDataPanel.tsx | 36 +---- .../components/DBRowSidePanelErrorState.tsx | 6 +- .../__tests__/DBRowDataPanel.test.ts | 132 ++++-------------- .../DBRowSidePanelErrorState.test.tsx | 11 +- packages/app/src/connection.ts | 4 +- ...eSelectAllColumnsSettingsRejection.test.ts | 103 -------------- .../useMaterializedAliasColumnsOption.ts | 15 +- .../useSelectAllColumnsSettingsRejection.ts | 91 ------------ 9 files changed, 55 insertions(+), 351 deletions(-) delete mode 100644 packages/app/src/hooks/__tests__/useSelectAllColumnsSettingsRejection.test.ts delete mode 100644 packages/app/src/hooks/useSelectAllColumnsSettingsRejection.ts diff --git a/.changeset/row-panel-materialized-alias-columns.md b/.changeset/row-panel-materialized-alias-columns.md index 4f6788a745..d14af7b81d 100644 --- a/.changeset/row-panel-materialized-alias-columns.md +++ b/.changeset/row-panel-materialized-alias-columns.md @@ -9,7 +9,7 @@ details panel never showed them. A new "Show materialized and alias columns" item in the properties view options menu, off by default, adds `asterisk_include_materialized_columns` and `asterisk_include_alias_columns` to the row query. It has no effect on a source with a Known Columns List, and a -value that the source's query settings give for either setting wins. If the -connection's user cannot change these settings (for example, `readonly = 1`), -the panel loads the row without them. If the row fails to load for another -reason while the option is on, the error state offers to turn the option off. +value that the source's query settings give for either setting wins. If the row +fails to load while the option is on (for example, because the connection's user +is `readonly = 1`, or an ALIAS column cannot be evaluated), the error state +offers to turn the option off. diff --git a/packages/app/src/components/DBRowDataPanel.tsx b/packages/app/src/components/DBRowDataPanel.tsx index df2907d427..d586019e71 100644 --- a/packages/app/src/components/DBRowDataPanel.tsx +++ b/packages/app/src/components/DBRowDataPanel.tsx @@ -1,4 +1,4 @@ -import { useEffect, useMemo } from 'react'; +import { useMemo } from 'react'; import { flatten } from 'flat'; import type { ResponseJSON } from '@hyperdx/common-utils/dist/clickhouse'; import { @@ -15,10 +15,6 @@ import { useSelectAllColumnsQuerySettings, } from '@/hooks/useMaterializedAliasColumnsOption'; import { WithClause } from '@/hooks/useRowWhere'; -import { - isSelectAllColumnsSettingRejected, - markSelectAllColumnsSettingsRejected, -} from '@/hooks/useSelectAllColumnsSettingsRejection'; import { getDisplayedTimestampValueExpression, getDurationMsExpression, @@ -215,16 +211,7 @@ export function useRowData({ ...(aliasWith && aliasWith.length > 0 ? { with: aliasWith } : {}), }; - const connection = source.connection; const additionalQuerySettings = useSelectAllColumnsQuerySettings(source); - // A rejected setting will fail again, so only other errors keep the retry. - const settingsQueryOptions = additionalQuerySettings - ? { - additionalQuerySettings, - retry: (failureCount: number, error: Error) => - failureCount < 1 && !isSelectAllColumnsSettingRejected(error), - } - : {}; const baseQueryKey = ['row_side_panel', rowId, aliasWith, source]; // Both halves of the filter are needed for `renderChartConfig` to emit one, so @@ -241,7 +228,7 @@ export function useRowData({ { queryKey: [...baseQueryKey, dateRange], enabled: rowId != null && hasWindow, - ...settingsQueryOptions, + additionalQuerySettings, }, ); @@ -262,28 +249,16 @@ export function useRowData({ const fallbackResult = useQueriedChartConfig(baseConfig, { queryKey: [...baseQueryKey, undefined], enabled: rowId != null && isFallbackActive, - ...settingsQueryOptions, + additionalQuerySettings, }); const queryResult = isFallbackActive ? fallbackResult : boundedResult; - const isSettingsRejected = - additionalQuerySettings != null && - queryResult.isError && - isSelectAllColumnsSettingRejected(queryResult.error); - useEffect(() => { - if (isSettingsRejected) { - markSelectAllColumnsSettingsRejected(connection); - } - }, [isSettingsRejected, connection]); - // The bounded result is known-empty by the time the retry is enabled, so // report loading until it settles rather than briefly claiming the row is - // absent. The same applies while the row is fetched again without settings. + // absent. const isLoading = - queryResult.isLoading || - isSettingsRejected || - (isBoundedEmpty && queryResult.isPending); + queryResult.isLoading || (isBoundedEmpty && queryResult.isPending); // Normalize resource and event attributes to always use flat keys for both JSON and Map columns const normalizedData = useMemo(() => { @@ -314,7 +289,6 @@ export function useRowData({ return { ...queryResult, - ...(isSettingsRejected ? { isError: false, error: null } : {}), data: normalizedData, isLoading, }; diff --git a/packages/app/src/components/DBRowSidePanelErrorState.tsx b/packages/app/src/components/DBRowSidePanelErrorState.tsx index c7645ddb52..673df7879e 100644 --- a/packages/app/src/components/DBRowSidePanelErrorState.tsx +++ b/packages/app/src/components/DBRowSidePanelErrorState.tsx @@ -97,8 +97,10 @@ function MaterializedAliasColumnsHint({ onHide }: { onHide: () => void }) { > - The row query includes MATERIALIZED and ALIAS columns. If one of them - cannot be evaluated, the whole row fails to load. + The row query includes MATERIALIZED and ALIAS columns. The whole row + fails to load if the connection's user cannot change query + settings (for example, a readonly user) or if one of these columns + cannot be evaluated. )}
- + diff --git a/packages/app/src/components/__tests__/DBRowSidePanelErrorState.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanelErrorState.test.tsx index 23d475f89b..9e06654849 100644 --- a/packages/app/src/components/__tests__/DBRowSidePanelErrorState.test.tsx +++ b/packages/app/src/components/__tests__/DBRowSidePanelErrorState.test.tsx @@ -132,23 +132,17 @@ describe('DBRowSidePanelErrorState', () => { describe('materialized and alias columns hint', () => { const OPTION_KEY = 'hdx-row-show-materialized-alias-columns'; - const aliasError = new ClickHouseQueryError( - "Dictionary ('default.missing') not found", - 'SELECT * FROM logs LIMIT 1 SETTINGS asterisk_include_alias_columns = 1', - ); - const readonlyError = new ClickHouseQueryError( - "Cannot modify 'asterisk_include_materialized_columns' setting in readonly mode. ", - 'SELECT * FROM logs LIMIT 1 SETTINGS asterisk_include_materialized_columns = 1', + // The hint does not depend on the error, so any error shows it. + const error = new ClickHouseQueryError( + 'Timeout exceeded: elapsed 30 seconds', + 'SELECT * FROM logs LIMIT 1', ); beforeEach(() => { localStorage.clear(); }); - it.each([ - ['an ALIAS column fails', aliasError], - ['the connection rejects the settings', readonlyError], - ])('offers to hide the columns when %s', (_case, error) => { + it('offers to hide the columns while the option is on', () => { localStorage.setItem(OPTION_KEY, 'true'); renderErrorState(error); @@ -165,34 +159,7 @@ describe('DBRowSidePanelErrorState', () => { }); it('is not shown while the option is off', () => { - renderErrorState(aliasError); - - expect( - screen.queryByTestId('materialized-alias-columns-hint'), - ).toBeNull(); - }); - - it('is not shown when the source sets both settings itself', () => { - localStorage.setItem(OPTION_KEY, 'true'); - renderErrorState(aliasError, { - ...source, - querySettings: [ - { setting: 'asterisk_include_materialized_columns', value: '0' }, - { setting: 'asterisk_include_alias_columns', value: '0' }, - ], - }); - - expect( - screen.queryByTestId('materialized-alias-columns-hint'), - ).toBeNull(); - }); - - it('is not shown when a Known Columns List replaces SELECT *', () => { - localStorage.setItem(OPTION_KEY, 'true'); - renderErrorState(aliasError, { - ...source, - knownColumnsListExpression: 'Timestamp, Body', - }); + renderErrorState(error); expect( screen.queryByTestId('materialized-alias-columns-hint'), diff --git a/packages/app/src/hooks/__tests__/useChartConfig.test.tsx b/packages/app/src/hooks/__tests__/useChartConfig.test.tsx index 3834085f3c..a94296ca30 100644 --- a/packages/app/src/hooks/__tests__/useChartConfig.test.tsx +++ b/packages/app/src/hooks/__tests__/useChartConfig.test.tsx @@ -2407,15 +2407,6 @@ describe('useChartConfig', () => { { setting: 'asterisk_include_materialized_columns', value: '1' }, ]); }); - - it('keeps the value of a setting that the source already defines', () => { - expect( - mergeQuerySettings( - [{ setting: 'asterisk_include_alias_columns', value: '0' }], - [{ setting: 'asterisk_include_alias_columns', value: '1' }], - ), - ).toEqual([{ setting: 'asterisk_include_alias_columns', value: '0' }]); - }); }); describe('appendChunk', () => { diff --git a/packages/app/src/hooks/useMaterializedAliasColumnsOption.ts b/packages/app/src/hooks/useMaterializedAliasColumnsOption.ts index 057af8628c..ba70435379 100644 --- a/packages/app/src/hooks/useMaterializedAliasColumnsOption.ts +++ b/packages/app/src/hooks/useMaterializedAliasColumnsOption.ts @@ -25,21 +25,29 @@ export function getKnownColumnsList(source: TSource): string | undefined { : undefined; } -// The settings that the row query adds for this source, if any. A setting that -// the source's own query settings define keeps the source's value. -export function useSelectAllColumnsQuerySettings( +// The settings that the row query adds for this source while the option is on, +// if any. A setting that the source's own query settings define keeps the +// source's value. +export function getSelectAllColumnsQuerySettings( source: TSource, ): QuerySettings | undefined { - const [showMaterializedAliasColumns] = useMaterializedAliasColumnsOption(); + if (getKnownColumnsList(source)) { + return undefined; + } const added = SELECT_ALL_COLUMNS_QUERY_SETTINGS.filter( ({ setting }) => !source.querySettings?.some( sourceSetting => sourceSetting.setting === setting, ), ); - return showMaterializedAliasColumns && - !getKnownColumnsList(source) && - added.length > 0 - ? added + return added.length > 0 ? added : undefined; +} + +export function useSelectAllColumnsQuerySettings( + source: TSource, +): QuerySettings | undefined { + const [showMaterializedAliasColumns] = useMaterializedAliasColumnsOption(); + return showMaterializedAliasColumns + ? getSelectAllColumnsQuerySettings(source) : undefined; }