From eeb05a58079eac91a8679035ae0d5a92e01df540 Mon Sep 17 00:00:00 2001 From: ramzi Date: Thu, 12 Nov 2026 15:10:36 -0500 Subject: [PATCH 1/2] fix(app): keep JSON-column filter keys in JSON form when filters are re-escaped Adding a second filter re-escaped every key in the filter state. toClickHouseKeyExpression treated a dotted key under a native JSON column as a Map key and rewrote it to col['key'], which ClickHouse rejects: 'First argument for function arrayElement must be array, got JSON'. Pass the table's JSON column names into escapeFilterStateKeys and toQuotedClickHouseKeyExpression, so keys under a JSON column are written as toString(`col`.`a`.`b`), the form the row panel already produces. Array subscripts in a path (`col`.`a`[1]) stay subscripts. Co-Authored-By: Claude Sonnet 5.5 --- .changeset/json-filter-keys-reescape.md | 9 +++ packages/app/src/DBSearchPage.tsx | 14 ++++ .../app/src/__tests__/searchFilters.test.ts | 56 +++++++++++++++ .../src/components/DBSearchPageFilters.tsx | 8 +++ .../DBSearchPageFilters/utils.test.ts | 71 +++++++++++++++++++ .../components/DBSearchPageFilters/utils.ts | 31 ++++++++ packages/app/src/searchFilters.tsx | 12 +++- 7 files changed, 200 insertions(+), 1 deletion(-) create mode 100644 .changeset/json-filter-keys-reescape.md diff --git a/.changeset/json-filter-keys-reescape.md b/.changeset/json-filter-keys-reescape.md new file mode 100644 index 0000000000..7984f43993 --- /dev/null +++ b/.changeset/json-filter-keys-reescape.md @@ -0,0 +1,9 @@ +--- +'@hyperdx/app': patch +--- + +fix: keep JSON-column filter keys in JSON form when another filter is added + +Adding a second filter no longer rewrites an earlier filter on a key inside a +native JSON column to `col['key']`, which ClickHouse rejects with +`Code 43: First argument for function 'arrayElement' must be array, got 'JSON'`. diff --git a/packages/app/src/DBSearchPage.tsx b/packages/app/src/DBSearchPage.tsx index 2a90cae9c6..9584b54399 100644 --- a/packages/app/src/DBSearchPage.tsx +++ b/packages/app/src/DBSearchPage.tsx @@ -1430,6 +1430,19 @@ function DBSearchPageContent() { : new Set(), [inputSourceColumns], ); + // Native JSON columns, so dotted filter keys under them are written as JSON + // access (`toString(`col`.`a`)`) and not as Map brackets (`col['a']`). + const jsonColumns = useMemo( + () => + inputSourceColumns + ? new Set( + inputSourceColumns + .filter(c => c.type === 'JSON' || c.type.startsWith('JSON(')) + .map(c => c.name), + ) + : new Set(), + [inputSourceColumns], + ); const watchedSource = useWatch({ control, @@ -1468,6 +1481,7 @@ function DBSearchPageContent() { onFilterChange: handleSetFilters, dateTimeColumns, knownColumns, + jsonColumns, }); useEffect(() => { diff --git a/packages/app/src/__tests__/searchFilters.test.ts b/packages/app/src/__tests__/searchFilters.test.ts index a6eec0c225..ce0a04ed9e 100644 --- a/packages/app/src/__tests__/searchFilters.test.ts +++ b/packages/app/src/__tests__/searchFilters.test.ts @@ -990,6 +990,62 @@ describe('searchFilters', () => { ]); }); + it('keeps an existing JSON-column filter in JSON form when another filter is added', () => { + onFilterChange.mockClear(); + const { result } = renderHook(() => + useSearchPageFilterState({ + // An earlier chip on a key inside the JSON column `message`, as + // persisted in the URL. + searchQuery: [ + { + type: 'sql', + condition: "toString(`message`.`email`) IN ('user@example.test')", + }, + ], + onFilterChange, + knownColumns: new Set(['message', 'source_name']), + jsonColumns: new Set(['message']), + }), + ); + + act(() => { + result.current.setFilterValue('source_name', 'EMAIL_SERVICE'); + }); + + const conditions = ( + onFilterChange.mock.lastCall![0] as ConditionFilter[] + ).map(f => f.condition); + // Re-persisting must not rewrite the existing key into Map bracket form + // (`message['email']`), which ClickHouse rejects for a JSON column. + expect(conditions).toContain( + "toString(`message`.`email`) IN ('user@example.test')", + ); + expect(conditions.some(c => c.includes("['"))).toBe(false); + }); + + it('keeps an existing JSON array-path filter intact when another filter is added', () => { + onFilterChange.mockClear(); + const arrayPathCondition = + "toString(`message`.`events`[1].`detail`.`parameters`[2].`value`) IN ('EMAIL_SERVICE')"; + const { result } = renderHook(() => + useSearchPageFilterState({ + searchQuery: [{ type: 'sql', condition: arrayPathCondition }], + onFilterChange, + knownColumns: new Set(['message', 'source_name']), + jsonColumns: new Set(['message']), + }), + ); + + act(() => { + result.current.setFilterValue('source_name', 'EMAIL_SERVICE'); + }); + + const conditions = ( + onFilterChange.mock.lastCall![0] as ConditionFilter[] + ).map(f => f.condition); + expect(conditions).toContain(arrayPathCondition); + }); + it('setOnlyFilters applies all filters in a single onFilterChange emit', () => { onFilterChange.mockClear(); const { result } = renderHook(() => diff --git a/packages/app/src/components/DBSearchPageFilters.tsx b/packages/app/src/components/DBSearchPageFilters.tsx index e370cda45e..249b352aeb 100644 --- a/packages/app/src/components/DBSearchPageFilters.tsx +++ b/packages/app/src/components/DBSearchPageFilters.tsx @@ -1259,6 +1259,11 @@ const DBSearchPageFiltersComponent = ({ () => (columns ? new Set(columns.map(c => c.name)) : new Set()), [columns], ); + // Same JSON columns as `jsonColumns` above, as a Set for key escaping. + const jsonColumnSet = useMemo( + () => new Set(jsonColumns ?? []), + [jsonColumns], + ); const [showMoreFields, setShowMoreFields] = useState(false); const { @@ -1581,6 +1586,7 @@ const DBSearchPageFiltersComponent = ({ sqlKey: toQuotedClickHouseKeyExpression( child.key, knownColumns, + jsonColumnSet, ), }))} selectedValues={group.children.reduce((acc, child) => { @@ -1647,6 +1653,7 @@ const DBSearchPageFiltersComponent = ({ const facetSqlKey = toQuotedClickHouseKeyExpression( facet.key, knownColumns, + jsonColumnSet, ); return ( { }); }); }); + +describe('toQuotedClickHouseKeyExpression with JSON columns', () => { + const knownColumns = new Set(['message', 'LogAttributes']); + const jsonColumns = new Set(['message']); + + it('keeps a dotted key under a JSON column in JSON dot access, never Map brackets', () => { + // `message` is a native JSON column; `message['email']` makes ClickHouse + // fail with "First argument for function 'arrayElement' must be array". + expect( + toQuotedClickHouseKeyExpression( + 'message.email', + knownColumns, + jsonColumns, + ), + ).toBe('toString(`message`.`email`)'); + }); + + it('splits a nested JSON path into one quoted segment per level', () => { + expect( + toQuotedClickHouseKeyExpression( + 'message.owner.profile.contact.email', + knownColumns, + jsonColumns, + ), + ).toBe('toString(`message`.`owner`.`profile`.`contact`.`email`)'); + }); + + // The row panel writes a filter on a value inside a JSON array as + // ``col.`events`[1].`id` ``. After the state cleans the backticks, the + // subscript must survive re-escaping as a subscript, not as part of a name. + it('keeps array subscripts in a JSON path as subscripts', () => { + expect( + toQuotedClickHouseKeyExpression( + 'message.events[1].detail.parameters[2].value', + knownColumns, + jsonColumns, + ), + ).toBe('toString(`message`.`events`[1].`detail`.`parameters`[2].`value`)'); + }); + + it('keeps chained array subscripts', () => { + expect( + toQuotedClickHouseKeyExpression( + 'message.grid[1][2].x', + knownColumns, + jsonColumns, + ), + ).toBe('toString(`message`.`grid`[1][2].`x`)'); + }); + + it('is idempotent on a key already in JSON form', () => { + const once = toQuotedClickHouseKeyExpression( + 'message.email', + knownColumns, + jsonColumns, + ); + expect( + toQuotedClickHouseKeyExpression(once, knownColumns, jsonColumns), + ).toBe(once); + }); + + it('still uses bracket access for a Map column', () => { + expect( + toQuotedClickHouseKeyExpression( + 'LogAttributes.host', + knownColumns, + jsonColumns, + ), + ).toBe("LogAttributes['host']"); + }); +}); diff --git a/packages/app/src/components/DBSearchPageFilters/utils.ts b/packages/app/src/components/DBSearchPageFilters/utils.ts index 6d7cb1d56d..c0a81a1ce8 100644 --- a/packages/app/src/components/DBSearchPageFilters/utils.ts +++ b/packages/app/src/components/DBSearchPageFilters/utils.ts @@ -297,10 +297,17 @@ function quoteIdentifierIfNeeded(id: string): string { * `knownColumns` is the set of real top-level column names on the table. Only * keys matching a known column or accessing a map key of a known column will * be quoted. + * + * `jsonColumns` is the subset of those columns that are native JSON columns. A + * dotted key under one of them is a JSON sub-path, not a Map key, so it is + * written as a `toString(`col`.`a`.`b`)` JSON access (the form the row panel + * produces) instead of the Map form `col['a.b']`, which ClickHouse rejects for + * a JSON column ("First argument for function 'arrayElement' must be array"). */ export function toQuotedClickHouseKeyExpression( key: string, knownColumns: Set, + jsonColumns?: Set, ): string { // A whole-key match against a real column wins: quote the entire name as one // identifier (handles flat columns whose name contains dots/hyphens/etc.). @@ -308,6 +315,30 @@ export function toQuotedClickHouseKeyExpression( return quoteIdentifierIfNeeded(key); } + if ( + jsonColumns && + jsonColumns.size > 0 && + !key.includes("['") && + !key.includes('["') && + !key.includes('`') && + !isSqlFunctionCallExpression(key) + ) { + const dotIdx = key.indexOf('.'); + const root = dotIdx > 0 ? key.slice(0, dotIdx) : ''; + if (root && jsonColumns.has(root) && dotIdx < key.length - 1) { + // A segment may end in array subscripts (`events[1]`), which belong to + // the path and must stay outside the quoted name. + const path = key + .slice(dotIdx + 1) + .split('.') + .map(segment => { + const [, name, subscripts] = /^(.*?)((?:\[\d+\])*)$/.exec(segment)!; + return `${SqlString.escapeId(name, true)}${subscripts}`; + }); + return `toString(${SqlString.escapeId(root, true)}.${path.join('.')})`; + } + } + // Normalize dot-form (ResourceAttributes.host.name) to map access form (ResourceAttributes['host.name']) const expr = toClickHouseKeyExpression(key); diff --git a/packages/app/src/searchFilters.tsx b/packages/app/src/searchFilters.tsx index c84c1079fc..211b11371f 100644 --- a/packages/app/src/searchFilters.tsx +++ b/packages/app/src/searchFilters.tsx @@ -26,10 +26,12 @@ export const IS_ROOT_SPAN_COLUMN_NAME = 'isRootSpan'; export const escapeFilterStateKeys = ( filters: FilterState, knownColumns: Set, + jsonColumns?: Set, ): FilterState => { const escaped: FilterState = {}; for (const [key, value] of Object.entries(filters)) { - escaped[toQuotedClickHouseKeyExpression(key, knownColumns)] = value; + escaped[toQuotedClickHouseKeyExpression(key, knownColumns, jsonColumns)] = + value; } return escaped; }; @@ -96,6 +98,7 @@ export const useSearchPageFilterState = ({ onFilterChange, dateTimeColumns, knownColumns, + jsonColumns, }: { searchQuery?: Filter[]; onFilterChange: (filters: Filter[]) => void; @@ -106,6 +109,8 @@ export const useSearchPageFilterState = ({ * (eg. service-name --> `service-name`). **/ knownColumns: Set; + /** Subset of `knownColumns` that are native JSON columns. */ + jsonColumns?: Set; }) => { // Access knownColumns through a ref so the returned mutators (which depend on // updateFilterQuery) keep stable identities across knownColumns reference @@ -115,6 +120,10 @@ export const useSearchPageFilterState = ({ useEffect(() => { knownColumnsRef.current = knownColumns; }, [knownColumns]); + const jsonColumnsRef = useRef | undefined>(jsonColumns); + useEffect(() => { + jsonColumnsRef.current = jsonColumns; + }, [jsonColumns]); // Persisted filters carry canonical (escaped) keys; convert back to the clean // keys the sidebar/comparisons use as they enter in-memory FilterState. @@ -146,6 +155,7 @@ export const useSearchPageFilterState = ({ const escapedFilters = escapeFilterStateKeys( newFilters, knownColumnsRef.current, + jsonColumnsRef.current, ); onFilterChange(filtersToQuery(escapedFilters, { dateTimeColumns })); }, From d5be2e2ca0bf1a35877750c3e574ba2664b29f4d Mon Sep 17 00:00:00 2001 From: ramzi Date: Thu, 12 Nov 2026 15:26:25 -0500 Subject: [PATCH 2/2] fix(app): address JSON array elements by index in row-panel filters The row panel wrote array positions inside a native JSON column as quoted keys (col.`events`.`0`.`id`). ClickHouse finds no such key, so the filter silently matched no rows. Array elements are addressed with 1-based subscripts (col.`events`[1].`id`). mergePath takes an optional set of array positions and writes [N+1] there for JSON columns. The row viewer derives the positions from the row data. Callers that pass no positions keep numeric segments as quoted keys, which facet keys from the JSON schema rely on. Co-Authored-By: Claude Sonnet 5.5 --- .changeset/json-array-paths.md | 9 ++++ packages/app/src/__tests__/utils.test.ts | 34 ++++++++++++++ .../src/components/DBRowJsonViewer.test.tsx | 45 ++++++++++++++++++- .../app/src/components/DBRowJsonViewer.tsx | 24 +++++++++- packages/app/src/utils.ts | 24 ++++++---- 5 files changed, 126 insertions(+), 10 deletions(-) create mode 100644 .changeset/json-array-paths.md diff --git a/.changeset/json-array-paths.md b/.changeset/json-array-paths.md new file mode 100644 index 0000000000..c59cdf2124 --- /dev/null +++ b/.changeset/json-array-paths.md @@ -0,0 +1,9 @@ +--- +'@hyperdx/app': patch +--- + +fix: address JSON array elements by index in row-panel filters + +"Add to Filters" on a value inside an array in a native JSON column now uses +1-based array subscripts, for example `events[1]`, instead of quoted keys such +as `events.0`, so the filter matches rows instead of silently returning none. diff --git a/packages/app/src/__tests__/utils.test.ts b/packages/app/src/__tests__/utils.test.ts index ee74129dcd..64a8d3e08d 100644 --- a/packages/app/src/__tests__/utils.test.ts +++ b/packages/app/src/__tests__/utils.test.ts @@ -1373,6 +1373,40 @@ describe('mergePath', () => { 'BodyJson.`service`.`name`', ); }); + + // Without array positions a numeric segment is a JSON object key, as + // before: facet keys come from the JSON schema, where `1` is a real key. + it('keeps a numeric segment as a quoted key when no array positions are given', () => { + expect(mergePath(['Body', 'events', '0', 'id'], ['Body'])).toBe( + 'Body.`events`.`0`.`id`', + ); + }); + + // The row viewer knows which segments index into an array. A JSON array + // element is addressed with a 1-based subscript; the quoted key + // (`.`0``) matches nothing, so the filter silently returns no rows. + it('emits a 1-based subscript for segments marked as array positions', () => { + expect( + mergePath( + ['message', 'events', '0', 'detail', 'parameters', '1', 'value'], + ['message'], + [], + new Set([2, 5]), + ), + ).toBe('message.`events`[1].`detail`.`parameters`[2].`value`'); + }); + + it('chains subscripts for nested arrays', () => { + expect( + mergePath(['m', 'grid', '0', '1', 'x'], ['m'], [], new Set([2, 3])), + ).toBe('m.`grid`[1][2].`x`'); + }); + + it('keeps a numeric object key quoted when it is not marked as an array position', () => { + expect(mergePath(['m', 'counts', '200', 'n'], ['m'], [], new Set())).toBe( + 'm.`counts`.`200`.`n`', + ); + }); }); describe('Map column (HDX-4369)', () => { diff --git a/packages/app/src/components/DBRowJsonViewer.test.tsx b/packages/app/src/components/DBRowJsonViewer.test.tsx index 4fc26a7a32..8408990cd5 100644 --- a/packages/app/src/components/DBRowJsonViewer.test.tsx +++ b/packages/app/src/components/DBRowJsonViewer.test.tsx @@ -3,7 +3,11 @@ import { fireEvent, screen, within } from '@testing-library/react'; import { useMaterializedAliasColumnsOption } from '@/hooks/useMaterializedAliasColumnsOption'; -import { buildJSONExtractQuery, DBRowJsonViewer } from './DBRowJsonViewer'; +import { + buildJSONExtractQuery, + DBRowJsonViewer, + getArrayIndexPositions, +} from './DBRowJsonViewer'; import { RowSidePanelContext } from './DBRowSidePanel'; // Mock Next.js router @@ -446,3 +450,42 @@ describe('DBRowJsonViewer', () => { }); }); }); + +describe('getArrayIndexPositions', () => { + const row = { + message: { + events: [ + { detail: { parameters: [{ name: 'a' }, { name: 'b', value: 'x' }] } }, + ], + counts: { '200': { n: 1 } }, + }, + }; + + it('marks the path positions whose parent is an array', () => { + expect( + Array.from( + getArrayIndexPositions(row, [ + 'message', + 'events', + '0', + 'detail', + 'parameters', + '1', + 'value', + ]), + ), + ).toEqual([2, 5]); + }); + + it('does not mark numeric keys of a plain object', () => { + expect( + Array.from( + getArrayIndexPositions(row, ['message', 'counts', '200', 'n']), + ), + ).toEqual([]); + }); + + it('returns an empty set for a missing path', () => { + expect(getArrayIndexPositions(row, ['nope', '0']).size).toBe(0); + }); +}); diff --git a/packages/app/src/components/DBRowJsonViewer.tsx b/packages/app/src/components/DBRowJsonViewer.tsx index 3c3ed4852b..ca74c7f591 100644 --- a/packages/app/src/components/DBRowJsonViewer.tsx +++ b/packages/app/src/components/DBRowJsonViewer.tsx @@ -422,6 +422,23 @@ function HyperJsonMenu({ ); } +// Positions in `keyPath` whose parent value in `rowData` is an array, i.e. the +// segments that are array indexes and not object keys. +export function getArrayIndexPositions( + rowData: unknown, + keyPath: string[], +): Set { + const positions = new Set(); + let node: any = rowData; + keyPath.forEach((segment, i) => { + if (Array.isArray(node)) { + positions.add(i); + } + node = node?.[segment]; + }); + return positions; +} + export function DBRowJsonViewer({ data, jsonColumns, @@ -495,7 +512,12 @@ export function DBRowJsonViewer({ const getLineActions = useCallback( ({ keyPath, value, isInParsedJson, parsedJsonRootPath }) => { const actions: LineAction[] = []; - const fieldPath = mergePath(keyPath, jsonColumns, mapColumns); + const fieldPath = mergePath( + keyPath, + jsonColumns, + mapColumns, + getArrayIndexPositions(rowData, keyPath), + ); const isJsonColumn = keyPath.length > 0 && jsonColumns?.includes(keyPath[0]); diff --git a/packages/app/src/utils.ts b/packages/app/src/utils.ts index 7cdbf972c7..223553c064 100644 --- a/packages/app/src/utils.ts +++ b/packages/app/src/utils.ts @@ -1170,24 +1170,32 @@ export const formatUptime = (seconds: number) => { const escapeSqlSingleQuoted = (v: string): string => v.replace(/\\/g, '\\\\').replace(/'/g, "\\'"); +// `arrayIndexes` (optional) holds the positions in `path` that index into an +// array. It only affects JSON columns: a numeric segment at one of those +// positions is a 1-based subscript (`Col.\`events\`[1]`), not a quoted key +// (`Col.\`events\`.\`0\``), which matches nothing. Callers that cannot tell +// arrays from objects omit it and keep numeric segments as keys. export const mergePath = ( path: string[], jsonColumns: string[] = [], mapColumns: string[] = [], + arrayIndexes?: ReadonlySet, ) => { const [key, ...rest] = path; if (rest.length === 0) { return key; } if (jsonColumns.includes(key)) { - return `${key}.${rest - .map(v => - v - .split('.') - .map(v => (v.startsWith('`') && v.endsWith('`') ? v : `\`${v}\``)) - .join('.'), - ) - .join('.')}`; + return rest.reduce((expr, v, i) => { + if (arrayIndexes?.has(i + 1) && /^\d+$/.test(v)) { + return `${expr}[${Number(v) + 1}]`; + } + const quoted = v + .split('.') + .map(v => (v.startsWith('`') && v.endsWith('`') ? v : `\`${v}\``)) + .join('.'); + return `${expr}.${quoted}`; + }, key); } if (mapColumns.includes(key)) { // Map columns always take string-key subscripts; ClickHouse's Map index