diff --git a/.changeset/json-filter-keys-reescape.md b/.changeset/json-filter-keys-reescape.md new file mode 100644 index 000000000..7984f4399 --- /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 2a90cae9c..9584b5439 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 a6eec0c22..ce0a04ed9 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 e370cda45..249b352ae 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 6d7cb1d56..c0a81a1ce 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 c84c1079f..211b11371 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 })); },