diff --git a/.changeset/fix-json-column-filter-bracket-notation.md b/.changeset/fix-json-column-filter-bracket-notation.md new file mode 100644 index 0000000000..82417498d0 --- /dev/null +++ b/.changeset/fix-json-column-filter-bracket-notation.md @@ -0,0 +1,5 @@ +--- +'@hyperdx/app': patch +--- + +Fix search page filters on JSON columns generating invalid bracket access. Adding or excluding a value on a sub-key of a JSON-typed column (e.g. `ResourceAttributes.region`) previously serialized to `ResourceAttributes['region']`, which ClickHouse rejects on a JSON column ("First argument for function 'arrayElement' must be array, got 'JSON'"). Sub-keys of JSON columns now serialize with dot access (`ResourceAttributes.\`region\``). diff --git a/packages/app/src/DBSearchPage.tsx b/packages/app/src/DBSearchPage.tsx index cdae127d1e..abbd5124e8 100644 --- a/packages/app/src/DBSearchPage.tsx +++ b/packages/app/src/DBSearchPage.tsx @@ -29,6 +29,8 @@ import HyperDX from '@hyperdx/browser'; import { ClickHouseQueryError, ColumnMeta, + filterColumnMetaByType, + JSDataType, } from '@hyperdx/common-utils/dist/clickhouse'; import { tcFromSource } from '@hyperdx/common-utils/dist/core/metadata'; import { buildSearchChartConfig } from '@hyperdx/common-utils/dist/core/searchChartConfig'; @@ -1318,6 +1320,20 @@ export function DBSearchPage() { : new Set(), [inputSourceColumns], ); + // JSON-typed columns for the active source. Sub-key filters on these must + // serialize as dot access (`ResourceAttributes.`region``) rather than the + // bracket access ClickHouse rejects on a JSON column. HDX-5085. + const jsonColumns = useMemo( + () => + inputSourceColumns + ? new Set( + filterColumnMetaByType(inputSourceColumns, [JSDataType.JSON])?.map( + c => c.name, + ) ?? [], + ) + : new Set(), + [inputSourceColumns], + ); const watchedSource = useWatch({ control, @@ -1353,6 +1369,7 @@ export function DBSearchPage() { onFilterChange: handleSetFilters, dateTimeColumns, knownColumns, + jsonColumns, }); useEffect(() => { diff --git a/packages/app/src/components/DBSearchPageFilters.tsx b/packages/app/src/components/DBSearchPageFilters.tsx index 67a154aae7..e8e244f87d 100644 --- a/packages/app/src/components/DBSearchPageFilters.tsx +++ b/packages/app/src/components/DBSearchPageFilters.tsx @@ -1138,6 +1138,12 @@ const DBSearchPageFiltersComponent = ({ const { data: source } = useSource({ id: sourceId }); const sourceTableConnection = tcFromSource(source); const { data: jsonColumns } = useJsonColumns(sourceTableConnection); + // Set form for O(1) membership checks when serializing sub-key filters on + // JSON columns to dot access. HDX-5085. + const jsonColumnsSet = useMemo( + () => new Set(jsonColumns ?? []), + [jsonColumns], + ); // Special case for live tail const [dateRange, setDateRange] = useState<[Date, Date]>( @@ -1458,6 +1464,7 @@ const DBSearchPageFiltersComponent = ({ sqlKey: toQuotedClickHouseKeyExpression( child.key, knownColumns, + jsonColumnsSet, ), }))} selectedValues={group.children.reduce((acc, child) => { @@ -1524,6 +1531,7 @@ const DBSearchPageFiltersComponent = ({ const facetSqlKey = toQuotedClickHouseKeyExpression( facet.key, knownColumns, + jsonColumnsSet, ); return ( new Set(jsonColumns ?? []), + [jsonColumns], + ); const { data: allFields, @@ -135,12 +142,16 @@ function useFacets({ const sqlKeyToUiKey = new Map(); const escapedKeysToFetch = keysToFetch.map(key => { - const sqlKey = toQuotedClickHouseKeyExpression(key, knownColumns); + const sqlKey = toQuotedClickHouseKeyExpression( + key, + knownColumns, + jsonColumnsSet, + ); sqlKeyToUiKey.set(sqlKey, key); return sqlKey; }); return { escapedKeysToFetch, sqlKeyToUiKey }; - }, [isColumnsLoading, keysToFetch, knownColumns]); + }, [isColumnsLoading, keysToFetch, knownColumns, jsonColumnsSet]); const facetsChartConfig = useMemo( () => @@ -174,7 +185,11 @@ function useFacets({ const loadMoreFacetsForKey = useCallback( async (key: string): Promise => { try { - const sqlKey = toQuotedClickHouseKeyExpression(key, knownColumns); + const sqlKey = toQuotedClickHouseKeyExpression( + key, + knownColumns, + jsonColumnsSet, + ); if (mode === 'exact') { const strippedFilterState: FilterState = { ...filterState }; delete strippedFilterState[key]; @@ -184,7 +199,11 @@ function useFacets({ ...chartConfig, dateRange, filters: filtersToQuery( - escapeFilterStateKeys(strippedFilterState, knownColumns), + escapeFilterStateKeys( + strippedFilterState, + knownColumns, + jsonColumnsSet, + ), { dateTimeColumns }, ), }, @@ -231,6 +250,7 @@ function useFacets({ chartConfig, dateRange, knownColumns, + jsonColumnsSet, source, filterState, dateTimeColumns, diff --git a/packages/app/src/components/DBSearchPageFilters/jsonAddToFilter.pipeline.test.ts b/packages/app/src/components/DBSearchPageFilters/jsonAddToFilter.pipeline.test.ts index f298bb04bf..1513974c55 100644 --- a/packages/app/src/components/DBSearchPageFilters/jsonAddToFilter.pipeline.test.ts +++ b/packages/app/src/components/DBSearchPageFilters/jsonAddToFilter.pipeline.test.ts @@ -165,3 +165,79 @@ describe('parsed-JSON "Add to Filters" -> valid SQL pipeline (HDX-4427)', () => expect(entry.included).toEqual(new Set(["O'Brien"])); }); }); + +// HDX-5085: filtering a sub-key of a *native* JSON column (e.g. from clicking +// "Add to Filters" then "Exclude" in the sidebar) must serialize as dot access, +// not the bracket access ClickHouse rejects on a JSON column with "First +// argument for function 'arrayElement' must be array, got 'JSON'". +describe('JSON-column sub-key filter -> valid dot-access SQL pipeline (HDX-5085)', () => { + // ResourceAttributes is a native JSON column here (unlike the Map columns + // above), so its sub-keys must render with dot access. + const jsonKnownColumns = new Set(['ResourceAttributes', 'ServiceName']); + const jsonColumns = new Set(['ResourceAttributes']); + + const runJsonFilter = ( + key: string, + values: { included?: string[]; excluded?: string[] }, + ) => { + const state: FilterState = { + [key]: { + included: new Set(values.included ?? []), + excluded: new Set(values.excluded ?? []), + }, + }; + return filtersToQuery( + escapeFilterStateKeys(state, jsonKnownColumns, jsonColumns), + ); + }; + + it('excludes a JSON sub-key value with dot access, not bracket access', () => { + expect( + runJsonFilter('ResourceAttributes.region', { + excluded: ['eu-central-1'], + }), + ).toEqual([ + { + type: 'sql', + condition: "ResourceAttributes.`region` NOT IN ('eu-central-1')", + }, + ]); + }); + + it('never emits the illegal bracket subscript on a JSON column', () => { + const query = runJsonFilter('ResourceAttributes.region', { + included: ['us-east-1'], + excluded: ['eu-central-1'], + }); + expect(query.length).toBeGreaterThan(0); + for (const filter of query) { + expect(filter.type).toBe('sql'); + if (filter.type === 'sql') { + expect(filter.condition).not.toContain("ResourceAttributes['"); + expect(isValidFilterCondition(filter.condition, 'sql')).toBe(true); + } + } + }); + + it('renders a nested JSON path with backtick-quoted segments', () => { + expect( + runJsonFilter('ResourceAttributes.host.name', { included: ['web-1'] }), + ).toEqual([ + { + type: 'sql', + condition: "ResourceAttributes.`host`.`name` IN ('web-1')", + }, + ]); + }); + + it('round-trips the JSON sub-key back to its clean dot form', () => { + const query = runJsonFilter('ResourceAttributes.region', { + excluded: ['eu-central-1'], + }); + const back = parseQuery(query).filters; + const restoredKeys = Object.keys(back).map(cleanClickHouseExpression); + expect(restoredKeys).toEqual(['ResourceAttributes.region']); + const entry = back[Object.keys(back)[0]]; + expect(entry.excluded).toEqual(new Set(['eu-central-1'])); + }); +}); diff --git a/packages/app/src/components/DBSearchPageFilters/utils.test.ts b/packages/app/src/components/DBSearchPageFilters/utils.test.ts index 10fa53c521..b4fbe276eb 100644 --- a/packages/app/src/components/DBSearchPageFilters/utils.test.ts +++ b/packages/app/src/components/DBSearchPageFilters/utils.test.ts @@ -181,6 +181,45 @@ describe('toClickHouseKeyExpression', () => { ); }); + // HDX-5085: a sub-key on a JSON-typed column must serialize as dot access, + // not bracket access. ClickHouse rejects `ResourceAttributes['region']` on a + // JSON column with "First argument for function 'arrayElement' must be array, + // got 'JSON'". When the base column is known to be JSON, emit dot notation + // with backtick-quoted segments instead. + describe('JSON columns (HDX-5085)', () => { + const jsonColumns = new Set(['ResourceAttributes']); + + it('rewrites a dot-form JSON sub-key to dot access, not bracket access', () => { + expect( + toClickHouseKeyExpression('ResourceAttributes.region', jsonColumns), + ).toBe('ResourceAttributes.`region`'); + }); + + it('renders a nested JSON path with each segment backtick-quoted', () => { + expect( + toClickHouseKeyExpression('ResourceAttributes.host.name', jsonColumns), + ).toBe('ResourceAttributes.`host`.`name`'); + }); + + it('still uses bracket form for a Map column not listed as JSON', () => { + expect( + toClickHouseKeyExpression('LogAttributes.region', jsonColumns), + ).toBe("LogAttributes['region']"); + }); + + it('falls back to bracket form when no JSON columns are provided', () => { + expect(toClickHouseKeyExpression('ResourceAttributes.region')).toBe( + "ResourceAttributes['region']", + ); + }); + + it('leaves an already-backtick JSON path unchanged', () => { + expect( + toClickHouseKeyExpression('ResourceAttributes.`region`', jsonColumns), + ).toBe('ResourceAttributes.`region`'); + }); + }); + // HDX-4427: "Add to Filters" on a value inside parsed JSON from a String // column builds a JSONExtract* function call as the filter key. These are // already valid ClickHouse expressions and must pass through untouched. @@ -320,4 +359,41 @@ describe('toQuotedClickHouseKeyExpression', () => { ).toBe("LogAttributes['host.name']"); }); }); + + // HDX-5085: a sub-key on a JSON column must serialize as dot access. + describe('with jsonColumns (JSON-aware)', () => { + const cols = new Set(['ResourceAttributes']); + const jsonCols = new Set(['ResourceAttributes']); + + it('renders a JSON sub-key as dot access instead of bracket access', () => { + expect( + toQuotedClickHouseKeyExpression( + 'ResourceAttributes.region', + cols, + jsonCols, + ), + ).toBe('ResourceAttributes.`region`'); + }); + + it('renders a nested JSON path with backtick-quoted segments', () => { + expect( + toQuotedClickHouseKeyExpression( + 'ResourceAttributes.host.name', + cols, + jsonCols, + ), + ).toBe('ResourceAttributes.`host`.`name`'); + }); + + it('uses bracket access for a Map column absent from jsonColumns', () => { + const mapCols = new Set(['LogAttributes']); + expect( + toQuotedClickHouseKeyExpression( + 'LogAttributes.region', + mapCols, + jsonCols, + ), + ).toBe("LogAttributes['region']"); + }); + }); }); diff --git a/packages/app/src/components/DBSearchPageFilters/utils.ts b/packages/app/src/components/DBSearchPageFilters/utils.ts index 86f0465426..056e2a8e09 100644 --- a/packages/app/src/components/DBSearchPageFilters/utils.ts +++ b/packages/app/src/components/DBSearchPageFilters/utils.ts @@ -149,22 +149,33 @@ const isSqlFunctionCallExpression = (key: string): boolean => /^[A-Za-z_]\w*\(/.test(key); // Coerce a filterState key into a ClickHouse expression suitable for raw SQL. -// A dot-form Map sub-key like `LogAttributes.host.name` is rewritten to bracket -// form `LogAttributes['host.name']` via `mergePath` so the conversion stays -// consistent with the keys produced by the facet-discovery path. Bracket form, -// backtick-quoted JSON paths, raw SQL function-call expressions -// (`toString(...)`, `JSONExtractString(...)`), and plain column names are -// returned unchanged. Use this when handing a filterState key off to a SQL -// caller (e.g. "Load more" via metadata.getKeyValues), since `setFilterValue` -// normalizes Map sub-keys to dot form which ClickHouse cannot resolve as map -// access. +// A dot-form sub-key like `LogAttributes.host.name` is rewritten via `mergePath` +// so the conversion stays consistent with the keys produced by the +// facet-discovery path. Bracket form, backtick-quoted JSON paths, raw SQL +// function-call expressions (`toString(...)`, `JSONExtractString(...)`), and +// plain column names are returned unchanged. Use this when handing a filterState +// key off to a SQL caller (e.g. "Load more" via metadata.getKeyValues), since +// `setFilterValue` normalizes sub-keys to dot form which ClickHouse cannot +// resolve as map/JSON access. // -// `parseMapFieldName` already guarantees `parsed.baseName` is a Map (its only -// callers are the dot-form facet keys that originate from Map columns), so -// `mergePath` must treat it as one. Without the third argument, a numeric- -// looking sub-key like `LogAttributes.1` collapses into the Array branch and -// emits the illegal `LogAttributes[2]`. HDX-4369. -export function toClickHouseKeyExpression(key: string): string { +// The base column's type decides the accessor: +// - JSON columns take dot access with backtick-quoted segments +// (`ResourceAttributes.`region``). ClickHouse rejects bracket subscripts on +// a JSON column with "First argument for function 'arrayElement' must be +// array, got 'JSON'", so a dot-form key like `ResourceAttributes.region` +// must NOT be turned into `ResourceAttributes['region']`. HDX-5085. +// - Map columns take bracket access (`LogAttributes['host.name']`). Passing +// the base as a Map (rather than letting `mergePath` fall through to its +// default branch) also keeps a numeric-looking sub-key like +// `LogAttributes.1` from collapsing into the illegal array subscript +// `LogAttributes[2]`. HDX-4369. +// +// Callers that know the schema should pass `jsonColumns`; otherwise the base is +// treated as a Map, preserving the historical behavior. +export function toClickHouseKeyExpression( + key: string, + jsonColumns: ReadonlySet = new Set(), +): string { if ( key.includes("['") || key.includes('["') || @@ -180,6 +191,13 @@ export function toClickHouseKeyExpression(key: string): string { } const parsed = parseMapFieldName(key); if (!parsed) return key; + if (jsonColumns.has(parsed.baseName)) { + return mergePath( + [parsed.baseName, parsed.propertyPath], + [parsed.baseName], + [], + ); + } return mergePath( [parsed.baseName, parsed.propertyPath], [], @@ -205,10 +223,15 @@ 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 set of JSON-typed top-level columns; sub-keys of these + * render as dot access (`ResourceAttributes.`region``) rather than bracket + * access, since ClickHouse cannot subscript a JSON column. HDX-5085. */ export function toQuotedClickHouseKeyExpression( key: string, knownColumns: Set, + jsonColumns: ReadonlySet = new 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.). @@ -216,8 +239,9 @@ export function toQuotedClickHouseKeyExpression( return quoteIdentifierIfNeeded(key); } - // Normalize dot-form (ResourceAttributes.host.name) to map access form (ResourceAttributes['host.name']) - const expr = toClickHouseKeyExpression(key); + // Normalize dot-form to the accessor for the base column's type: Map access + // (ResourceAttributes['host.name']) or JSON dot access (ResourceAttributes.`region`). + const expr = toClickHouseKeyExpression(key, jsonColumns); // Already quoted: leave untouched if (expr.startsWith('`') || expr.startsWith('"')) { diff --git a/packages/app/src/searchFilters.tsx b/packages/app/src/searchFilters.tsx index ac193b7f26..748447a4ed 100644 --- a/packages/app/src/searchFilters.tsx +++ b/packages/app/src/searchFilters.tsx @@ -22,14 +22,19 @@ export const IS_ROOT_SPAN_COLUMN_NAME = 'isRootSpan'; // 2. The persisted `Filter[]`: Uses the quoted/bracketed ClickHouse form so it emits // valid SQL verbatim. Persisted in URL params, local storage, and MongoDB for saved searches. -// Convert the clean FilterState keys to valid SQL (quoted/bracket) keys. +// Convert the clean FilterState keys to valid SQL (quoted/bracket/dot) keys. +// `jsonColumns` lets a sub-key of a JSON column serialize as dot access +// (`ResourceAttributes.`region``) instead of the illegal bracket access +// (`ResourceAttributes['region']`) ClickHouse rejects on a JSON column. HDX-5085. export const escapeFilterStateKeys = ( filters: FilterState, knownColumns: Set, + jsonColumns: ReadonlySet = new 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; }; @@ -83,6 +88,7 @@ export const useSearchPageFilterState = ({ onFilterChange, dateTimeColumns, knownColumns, + jsonColumns, }: { searchQuery?: Filter[]; onFilterChange: (filters: Filter[]) => void; @@ -93,16 +99,27 @@ export const useSearchPageFilterState = ({ * (eg. service-name --> `service-name`). **/ knownColumns: Set; + /** + * JSON-typed top-level column names on the table, used to serialize sub-key + * filters as dot access (`ResourceAttributes.`region``) instead of the + * bracket access ClickHouse rejects on a JSON column. HDX-5085. + **/ + jsonColumns?: ReadonlySet; }) => { - // Access knownColumns through a ref so the returned mutators (which depend on - // updateFilterQuery) keep stable identities across knownColumns reference - // changes. The known columns are only used by the mutators, which are called - // on user input, not render, so avoid re-render by using a stable ref. + // Access knownColumns/jsonColumns through refs so the returned mutators (which + // depend on updateFilterQuery) keep stable identities across reference + // changes. These are only used by the mutators, which are called on user + // input, not render, so avoid re-render by using stable refs. const knownColumnsRef = useRef>(knownColumns); useEffect(() => { knownColumnsRef.current = knownColumns; }, [knownColumns]); + const jsonColumnsRef = useRef>(jsonColumns ?? new Set()); + useEffect(() => { + jsonColumnsRef.current = jsonColumns ?? new Set(); + }, [jsonColumns]); + // Persisted filters carry canonical (escaped) keys; convert back to the clean // keys the sidebar/comparisons use as they enter in-memory FilterState. const parsedQuery = useMemo(() => { @@ -133,6 +150,7 @@ export const useSearchPageFilterState = ({ const escapedFilters = escapeFilterStateKeys( newFilters, knownColumnsRef.current, + jsonColumnsRef.current, ); onFilterChange(filtersToQuery(escapedFilters, { dateTimeColumns })); },