From f58a833ff1c786d9d559a6650b0df94ec2fa4b4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Fr=C3=B6mbgen?= <23717573+mfroembgen@users.noreply.github.com> Date: Wed, 9 Sep 2026 08:06:48 +0200 Subject: [PATCH 1/2] fix: handle native JSON search filters --- .changeset/tame-json-sidebar-filters.md | 6 + packages/app/src/DBSearchPage.tsx | 7 + .../app/src/__tests__/searchFilters.test.ts | 41 ++++++ .../src/components/DBSearchPageFilters.tsx | 12 +- .../DBSearchPageFilters/hooks.test.tsx | 104 +++++++++++++-- .../components/DBSearchPageFilters/hooks.ts | 19 ++- .../DBSearchPageFilters/utils.test.ts | 42 ++++++ .../components/DBSearchPageFilters/utils.ts | 11 +- packages/app/src/hooks/useMetadata.tsx | 17 ++- packages/app/src/searchFilters.tsx | 17 ++- .../search/native-json-filters.spec.ts | 124 ++++++++++++++++++ packages/app/tests/e2e/seed-clickhouse.ts | 16 ++- .../src/__tests__/metadata.test.ts | 20 +-- packages/common-utils/src/core/metadata.ts | 4 +- packages/common-utils/src/filters.ts | 4 +- 15 files changed, 399 insertions(+), 45 deletions(-) create mode 100644 .changeset/tame-json-sidebar-filters.md create mode 100644 packages/app/tests/e2e/features/search/native-json-filters.spec.ts diff --git a/.changeset/tame-json-sidebar-filters.md b/.changeset/tame-json-sidebar-filters.md new file mode 100644 index 0000000000..63db6e7a66 --- /dev/null +++ b/.changeset/tame-json-sidebar-filters.md @@ -0,0 +1,6 @@ +--- +'@hyperdx/app': patch +'@hyperdx/common-utils': patch +--- + +Fix native JSON search filters and metadata value queries for string, numeric, boolean, and mixed-type paths, while preserving Map-column bracket access. diff --git a/packages/app/src/DBSearchPage.tsx b/packages/app/src/DBSearchPage.tsx index 9f5f62522c..10aebd7279 100644 --- a/packages/app/src/DBSearchPage.tsx +++ b/packages/app/src/DBSearchPage.tsx @@ -163,6 +163,7 @@ import { } from './components/TimePicker/utils'; import { useColumns, + useJsonColumnNames, useResolvedDateTimeColumns, useTableMetadata, } from './hooks/useMetadata'; @@ -1432,6 +1433,11 @@ function DBSearchPageContent() { : new Set(), [inputSourceColumns], ); + const jsonColumnNames = useJsonColumnNames(inputSourceColumns); + const jsonColumns = useMemo( + () => new Set(jsonColumnNames), + [jsonColumnNames], + ); const watchedSource = useWatch({ control, @@ -1470,6 +1476,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..9881db08fb 100644 --- a/packages/app/src/__tests__/searchFilters.test.ts +++ b/packages/app/src/__tests__/searchFilters.test.ts @@ -1,5 +1,6 @@ import { enableMapSet } from 'immer'; import { filtersToQuery } from '@hyperdx/common-utils/dist/filters'; +import type { Filter } from '@hyperdx/common-utils/dist/types'; import { act, renderHook } from '@testing-library/react'; import { @@ -13,6 +14,46 @@ enableMapSet(); type ConditionFilter = { type: 'sql' | 'lucene'; condition: string }; describe('searchFilters', () => { + it.each([ + [ + 'ResourceAttributes', + 'toString(ResourceAttributes.`k8s`.`namespace`.`name`)', + ], + ['LogAttributes', "LogAttributes['k8s.namespace.name']"], + ])('selects, reloads and clears a %s filter', (column, expression) => { + const key = + column === 'ResourceAttributes' + ? 'ResourceAttributes.k8s.namespace.name' + : expression; + const facetKey = + column === 'ResourceAttributes' + ? 'ResourceAttributes.`k8s`.`namespace`.`name`' + : key; + const onFilterChange = jest.fn(); + const emptyQuery: Filter[] = []; + const { result, rerender } = renderHook( + ({ searchQuery }: { searchQuery: Filter[] }) => + useSearchPageFilterState({ + searchQuery, + onFilterChange, + knownColumns: new Set(['ResourceAttributes', 'LogAttributes']), + jsonColumns: new Set(['ResourceAttributes']), + }), + { initialProps: { searchQuery: emptyQuery } }, + ); + act(() => result.current.setFilterValue(facetKey, 'production')); + const searchQuery: Filter[] = [ + { type: 'sql', condition: `${expression} IN ('production')` }, + ]; + expect(onFilterChange).toHaveBeenLastCalledWith(searchQuery); + rerender({ searchQuery }); + expect(result.current.filters[key].included).toEqual( + new Set(['production']), + ); + act(() => result.current.clearFilter(facetKey)); + expect(onFilterChange).toHaveBeenLastCalledWith([]); + }); + describe('parseQuery', () => { it('empty query', () => { const result = parseQuery([]); diff --git a/packages/app/src/components/DBSearchPageFilters.tsx b/packages/app/src/components/DBSearchPageFilters.tsx index 2b75b5fcf5..3d9e6b9573 100644 --- a/packages/app/src/components/DBSearchPageFilters.tsx +++ b/packages/app/src/components/DBSearchPageFilters.tsx @@ -57,7 +57,7 @@ import { IS_CLICKHOUSE_BUILD } from '@/config'; import { useColumns, useGetValuesDistribution, - useJsonColumns, + useJsonColumnNames, useTableMetadata, } from '@/hooks/useMetadata'; import useResizable from '@/hooks/useResizable'; @@ -1197,7 +1197,6 @@ const DBSearchPageFiltersComponent = ({ const { data: source } = useSource({ id: sourceId }); const sourceTableConnection = tcFromSource(source); - const { data: jsonColumns } = useJsonColumns(sourceTableConnection); // Special case for live tail const [dateRange, setDateRange] = useState<[Date, Date]>( @@ -1209,6 +1208,8 @@ const DBSearchPageFiltersComponent = ({ tableName: chartConfig.from.tableName, connectionId: chartConfig.connection, }); + const jsonColumns = useJsonColumnNames(columns); + const jsonColumnSet = useMemo(() => new Set(jsonColumns), [jsonColumns]); const { data: tableMetadata } = useTableMetadata(sourceTableConnection); @@ -1334,9 +1335,6 @@ const DBSearchPageFiltersComponent = ({ const _facets: FacetEntry[] = []; for (const _facet of facetsWithPinnedValues ?? []) { const facet = structuredClone(_facet); - if (jsonColumns?.some(col => facet.key.startsWith(col))) { - facet.key = `toString(${facet.key})`; - } // Skip fields already shown in the Shared Filters section if (sharedFilterKeys.has(facet.key)) { @@ -1416,7 +1414,6 @@ const DBSearchPageFiltersComponent = ({ tableMetadata, isFieldPinned, isSharedFieldPinned, - jsonColumns, sharedFilterKeys, ]); @@ -1536,6 +1533,7 @@ const DBSearchPageFiltersComponent = ({ sqlKey: toQuotedClickHouseKeyExpression( child.key, knownColumns, + jsonColumnSet, ), }))} selectedValues={group.children.reduce((acc, child) => { @@ -1602,6 +1600,7 @@ const DBSearchPageFiltersComponent = ({ const facetSqlKey = toQuotedClickHouseKeyExpression( facet.key, knownColumns, + jsonColumnSet, ); return ( ({ useSource: jest.fn(), })); -jest.mock('@/searchFilters', () => ({ - __esModule: true, - usePinnedFilters: jest.fn(), - escapeFilterStateKeys: jest.fn((state: unknown) => state), -})); +jest.mock('@/searchFilters', () => { + const actual = + jest.requireActual('@/searchFilters'); + return { + __esModule: true, + ...actual, + usePinnedFilters: jest.fn(), + escapeFilterStateKeys: jest.fn(actual.escapeFilterStateKeys), + }; +}); jest.mock('@/hooks/useMetadata', () => ({ __esModule: true, useMetadataWithSettings: jest.fn(), useColumns: jest.fn(), useDateTimeColumns: jest.fn(), - useJsonColumns: jest.fn(), + useJsonColumnNames: jest.fn(), useMapColumns: jest.fn(), useAllFields: jest.fn(), useGetKeyValues: jest.fn(), @@ -80,7 +85,7 @@ const useMetadataWithSettings = jest.mocked( ); const useColumns = jest.mocked(useMetadataModule.useColumns); const useDateTimeColumns = jest.mocked(useMetadataModule.useDateTimeColumns); -const useJsonColumns = jest.mocked(useMetadataModule.useJsonColumns); +const useJsonColumnNames = jest.mocked(useMetadataModule.useJsonColumnNames); const useMapColumns = jest.mocked(useMetadataModule.useMapColumns); const useAllFields = jest.mocked(useMetadataModule.useAllFields); const useGetKeyValues = jest.mocked(useMetadataModule.useGetKeyValues); @@ -166,10 +171,8 @@ function setupDefaultMocks({ withMVs }: { withMVs: boolean }) { isLoading: false, } as any); - useDateTimeColumns.mockReturnValue([ - { name: 'Timestamp', type: 'DateTime' }, - ] as any); - useJsonColumns.mockReturnValue({ data: [] } as any); + useDateTimeColumns.mockReturnValue(new Map([['Timestamp', 'DateTime']])); + useJsonColumnNames.mockReturnValue([]); useMapColumns.mockReturnValue({ data: [] } as any); useAllFields.mockReturnValue({ @@ -570,6 +573,85 @@ describe('useFetchFacets', () => { }); describe('loadMoreFacetsForKey (raw-tables pipeline)', () => { + it("strips a JSON facet's own selection before escaping load-more filters", async () => { + setupDefaultMocks({ withMVs: false }); + useJsonColumnNames.mockReturnValue(['ResourceAttributes']); + useColumns.mockReturnValue({ + data: [ + { name: 'Timestamp', type: 'DateTime' }, + { + name: 'ResourceAttributes', + type: 'JSON(max_dynamic_types=8, max_dynamic_paths=64)', + }, + ], + isLoading: false, + } as ReturnType); + useMetadataWithSettings.mockReturnValue({ + getKeyValuesWithMVs: jest.fn().mockResolvedValue([ + { + key: "ResourceAttributes['k8s.namespace.name']", + value: ['production'], + }, + ]), + } as any); + const filterState: FilterState = { + 'ResourceAttributes.k8s.namespace.name': { + included: new Set(['production']), + excluded: new Set(), + }, + 'ResourceAttributes.service.name': { + included: new Set(['api']), + excluded: new Set(), + }, + }; + + const { wrapper } = makeWrapper(); + const { result } = renderHook( + () => + useFetchFacets({ + chartConfig: CHART_CONFIG, + sourceId: 'source1', + dateRange: DATE_RANGE, + mode: 'exact', + filterState, + }), + { wrapper }, + ); + + await act(async () => { + await result.current.loadMoreFacetsForKey( + 'toString(ResourceAttributes.`k8s`.`namespace`.`name`)', + ); + }); + + expect(searchFiltersModule.escapeFilterStateKeys).toHaveBeenCalledWith( + { + 'ResourceAttributes.service.name': { + included: new Set(['api']), + excluded: new Set(), + }, + }, + expect.any(Set), + new Set(['ResourceAttributes']), + ); + expect( + useMetadataWithSettings.mock.results.at(-1)?.value.getKeyValuesWithMVs, + ).toHaveBeenCalledWith( + expect.objectContaining({ + keys: ["ResourceAttributes['k8s.namespace.name']"], + chartConfig: expect.objectContaining({ + filters: [ + { + type: 'sql', + condition: + "toString(ResourceAttributes.`service`.`name`) IN ('api')", + }, + ], + }), + }), + ); + }); + it('reports the key as loading while the fetch is in flight, then clears it', async () => { setupDefaultMocks({ withMVs: false }); useGetKeyValues.mockReturnValue({ diff --git a/packages/app/src/components/DBSearchPageFilters/hooks.ts b/packages/app/src/components/DBSearchPageFilters/hooks.ts index 54267aeaf0..649f355ded 100644 --- a/packages/app/src/components/DBSearchPageFilters/hooks.ts +++ b/packages/app/src/components/DBSearchPageFilters/hooks.ts @@ -22,7 +22,7 @@ import { useDateTimeColumns, useFilterKeysFetchLimit, useGetKeyValues, - useJsonColumns, + useJsonColumnNames, useMapColumns, useMetadataWithSettings, } from '@/hooks/useMetadata'; @@ -31,6 +31,7 @@ import { useSource } from '@/source'; import { mergePath } from '@/utils'; import { + cleanClickHouseExpression, listRequestedFacets, sortLowCardinalityFirst, toQuotedClickHouseKeyExpression, @@ -102,7 +103,8 @@ function useFacets({ () => (columns ? new Set(columns.map(c => c.name)) : new Set()), [columns], ); - const { data: jsonColumns } = useJsonColumns(tableConnection); + const jsonColumns = useJsonColumnNames(columns); + const jsonColumnSet = useMemo(() => new Set(jsonColumns), [jsonColumns]); const { data: mapColumns } = useMapColumns(tableConnection); const { @@ -216,17 +218,23 @@ function useFacets({ const loadMoreFacetsForKey = useCallback( async (key: string): Promise => { try { - const sqlKey = toQuotedClickHouseKeyExpression(key, knownColumns); + const cleanKey = cleanClickHouseExpression(key); + const sqlKey = toQuotedClickHouseKeyExpression(cleanKey, knownColumns); if (mode === 'exact') { const strippedFilterState: FilterState = { ...filterState }; delete strippedFilterState[key]; - if (sqlKey !== key) delete strippedFilterState[sqlKey]; + delete strippedFilterState[cleanKey]; + delete strippedFilterState[sqlKey]; const newKeyVals = await metadata.getKeyValuesWithMVs({ chartConfig: { ...chartConfig, dateRange, filters: filtersToQuery( - escapeFilterStateKeys(strippedFilterState, knownColumns), + escapeFilterStateKeys( + strippedFilterState, + knownColumns, + jsonColumnSet, + ), { dateTimeColumns }, ), }, @@ -285,6 +293,7 @@ function useFacets({ source, filterState, dateTimeColumns, + jsonColumnSet, ], ); diff --git a/packages/app/src/components/DBSearchPageFilters/utils.test.ts b/packages/app/src/components/DBSearchPageFilters/utils.test.ts index 884e714fd1..2ee03e9811 100644 --- a/packages/app/src/components/DBSearchPageFilters/utils.test.ts +++ b/packages/app/src/components/DBSearchPageFilters/utils.test.ts @@ -1,3 +1,4 @@ +import { renderJsonStringSubcolumn } from '@hyperdx/common-utils/dist/core/metadata'; import type { FilterState } from '@hyperdx/common-utils/dist/filters'; import { @@ -11,6 +12,21 @@ import { } from './utils'; describe('cleanClickHouseExpression', () => { + it.each(['nested.key', 'hyphen-key', 'back`tick', 'back\\slash', 'close)'])( + 'round-trips the shared JSON renderer for %j', + path => { + const rendered = renderJsonStringSubcolumn('Attributes', path); + const clean = cleanClickHouseExpression(rendered); + expect(clean).toBe(`Attributes.${path}`); + expect( + toQuotedClickHouseKeyExpression( + clean, + new Set(['Attributes']), + new Set(['Attributes']), + ), + ).toBe(rendered); + }, + ); it('strips backticks from a bare quoted identifier', () => { expect(cleanClickHouseExpression('`service-name`')).toBe('service-name'); }); @@ -21,6 +37,12 @@ describe('cleanClickHouseExpression', () => { ).toBe('ResourceAttributes.hdx.sdk'); }); + it('unescapes doubled backticks in quoted JSON path segments', () => { + expect( + cleanClickHouseExpression('toString(ResourceAttributes.`k8s`.`na``me`)'), + ).toBe('ResourceAttributes.k8s.na`me'); + }); + it('leaves a plain identifier unchanged', () => { expect(cleanClickHouseExpression('ServiceName')).toBe('ServiceName'); }); @@ -434,4 +456,24 @@ describe('toQuotedClickHouseKeyExpression', () => { ).toBe("LogAttributes['host.name']"); }); }); + + it.each([ + ['ResourceAttributes.region', 'toString(ResourceAttributes.`region`)'], + [ + 'ResourceAttributes.k8s.namespace.name', + 'toString(ResourceAttributes.`k8s`.`namespace`.`name`)', + ], + [ + 'ResourceAttributes.k8s.na`me', + 'toString(ResourceAttributes.`k8s`.`na``me`)', + ], + ])('renders JSON key %s as a string expression', (key, expected) => { + expect( + toQuotedClickHouseKeyExpression( + key, + new Set(['ResourceAttributes']), + new Set(['ResourceAttributes']), + ), + ).toBe(expected); + }); }); diff --git a/packages/app/src/components/DBSearchPageFilters/utils.ts b/packages/app/src/components/DBSearchPageFilters/utils.ts index 6d7cb1d56d..e7dc3f6fba 100644 --- a/packages/app/src/components/DBSearchPageFilters/utils.ts +++ b/packages/app/src/components/DBSearchPageFilters/utils.ts @@ -4,6 +4,7 @@ import SqlString from 'sqlstring'; import { type Field, parseKeyPath, + renderJsonStringSubcolumn, } from '@hyperdx/common-utils/dist/core/metadata'; import type { FilterState } from '@hyperdx/common-utils/dist/filters'; @@ -71,7 +72,9 @@ export function cleanClickHouseExpression(key: string): string { // Convert backtick dot notation to clean dot notation // e.g., `host`.`arch` -> host.arch - cleanKey = cleanKey.replace(/`([^`]+)`/g, '$1'); + cleanKey = cleanKey.replace(/`((?:``|[^`])*)`/g, (_, identifier: string) => + identifier.replace(/``/g, '`'), + ); return cleanKey; } @@ -301,6 +304,7 @@ function quoteIdentifierIfNeeded(id: string): string { export function toQuotedClickHouseKeyExpression( key: string, knownColumns: Set, + jsonColumns?: ReadonlySet, ): 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 +312,11 @@ export function toQuotedClickHouseKeyExpression( return quoteIdentifierIfNeeded(key); } + const parsed = parseMapFieldName(key); + if (parsed && jsonColumns?.has(parsed.baseName)) { + return renderJsonStringSubcolumn(parsed.baseName, parsed.propertyPath); + } + // Normalize dot-form (ResourceAttributes.host.name) to map access form (ResourceAttributes['host.name']) const expr = toClickHouseKeyExpression(key); diff --git a/packages/app/src/hooks/useMetadata.tsx b/packages/app/src/hooks/useMetadata.tsx index 7ca8d72364..aa7827b7aa 100644 --- a/packages/app/src/hooks/useMetadata.tsx +++ b/packages/app/src/hooks/useMetadata.tsx @@ -178,6 +178,17 @@ export function useResolvedDateTimeColumns( return { dateTimeColumns, onResolvedColumnsChange: setResultColumns }; } +const getJsonColumnNames = (columns: ColumnMetaType[]): string[] => + filterColumnMetaByType(columns, [JSDataType.JSON])?.map( + column => column.name, + ) ?? []; + +export function useJsonColumnNames( + columns: ColumnMetaType[] | undefined, +): string[] { + return useMemo(() => getJsonColumnNames(columns ?? []), [columns]); +} + export function useJsonColumns( tableConnection: TableConnection | undefined, options?: Partial>, @@ -188,11 +199,7 @@ export function useJsonColumns( queryFn: async () => { if (!tableConnection) return []; const columns = await metadata.getColumns(tableConnection); - return ( - filterColumnMetaByType(columns, [JSDataType.JSON])?.map( - column => column.name, - ) ?? [] - ); + return getJsonColumnNames(columns); }, enabled: tableConnection && diff --git a/packages/app/src/searchFilters.tsx b/packages/app/src/searchFilters.tsx index e9f1e50258..b2941b21d3 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?: ReadonlySet, ): 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,11 +109,16 @@ export const useSearchPageFilterState = ({ * (eg. service-name --> `service-name`). **/ knownColumns: Set; + 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. + const jsonColumnsRef = useRef(jsonColumns); + useEffect(() => { + jsonColumnsRef.current = jsonColumns; + }, [jsonColumns]); const knownColumnsRef = useRef>(knownColumns); useEffect(() => { knownColumnsRef.current = knownColumns; @@ -146,6 +154,7 @@ export const useSearchPageFilterState = ({ const escapedFilters = escapeFilterStateKeys( newFilters, knownColumnsRef.current, + jsonColumnsRef.current, ); onFilterChange(filtersToQuery(escapedFilters, { dateTimeColumns })); }, @@ -158,6 +167,7 @@ export const useSearchPageFilterState = ({ value: string | boolean, action?: 'only' | 'exclude' | 'include', ) => { + property = cleanClickHouseExpression(property); setFilters(prevFilters => { const newFilters = produce(prevFilters, draft => { if (!draft[property]) { @@ -212,7 +222,7 @@ export const useSearchPageFilterState = ({ setFilters(prevFilters => { const newFilters = produce(prevFilters, draft => { for (const { property, value } of entries) { - draft[property] = { + draft[cleanClickHouseExpression(property)] = { included: new Set([value]), excluded: new Set(), }; @@ -227,6 +237,7 @@ export const useSearchPageFilterState = ({ const setFilterRange = useCallback( (property: string, range: { min: number; max: number }) => { + property = cleanClickHouseExpression(property); setFilters(prevFilters => { const newFilters = produce(prevFilters, draft => { if (!draft[property]) { @@ -243,6 +254,7 @@ export const useSearchPageFilterState = ({ const clearFilter = useCallback( (property: string) => { + property = cleanClickHouseExpression(property); setFilters(prevFilters => { const newFilters = produce(prevFilters, draft => { delete draft[property]; @@ -264,6 +276,7 @@ export const useSearchPageFilterState = ({ newValue: string | boolean, action: 'include' | 'exclude', ) => { + property = cleanClickHouseExpression(property); setFilters(prevFilters => { const newFilters = produce(prevFilters, draft => { if (!draft[property]) { diff --git a/packages/app/tests/e2e/features/search/native-json-filters.spec.ts b/packages/app/tests/e2e/features/search/native-json-filters.spec.ts new file mode 100644 index 0000000000..935093645f --- /dev/null +++ b/packages/app/tests/e2e/features/search/native-json-filters.spec.ts @@ -0,0 +1,124 @@ +import { SearchPage } from '../../page-objects/SearchPage'; +import { INTERESTING_FILTER_KEYS_ROWS } from '../../seed-clickhouse'; +import { expect, test } from '../../utils/base-test'; +import { INTERESTING_FILTER_KEYS_SOURCE_NAME } from '../../utils/constants'; + +const [ROW1, ROW2, ROW3] = INTERESTING_FILTER_KEYS_ROWS; +const QUERY_TIMEOUT = 20_000; + +test.describe( + 'Search: native JSON filters', + { tag: ['@search', '@full-stack'] }, + () => { + for (const { path, values } of [ + { + path: 'key.subKey.subSubKey', + values: [ROW1.jsonValue, ROW2.jsonValue, ROW3.jsonValue], + }, + { + path: 'integer', + values: [ROW1.jsonInteger, ROW2.jsonInteger, ROW3.jsonInteger].map( + String, + ), + }, + { + path: 'boolean', + values: [ROW1.jsonBoolean, ROW2.jsonBoolean].map(String), + }, + { + path: 'mixed', + values: [ROW1.jsonMixed, ROW2.jsonMixed, ROW3.jsonMixed].map(String), + }, + ]) { + test(`loads, includes and excludes ${path}`, async ({ page }) => { + const searchPage = new SearchPage(page); + await searchPage.goto(); + await searchPage.selectSource(INTERESTING_FILTER_KEYS_SOURCE_NAME); + await searchPage.timePicker.selectRelativeTime('Last 1 hour'); + await searchPage.table.waitForRowsToPopulate(); + + // Native JSON discovery is disabled; existing personal pins still expose + // the field and its real "Load more" query without mocking facet values. + const sourceId = new URL(page.url()).searchParams.get('source'); + expect(sourceId).toBeTruthy(); + await page.evaluate( + ({ sourceId, field }) => { + localStorage.setItem( + 'hdx-pinned-fields', + JSON.stringify({ [sourceId!]: [field] }), + ); + }, + { sourceId, field: `ResourceAttributesJSON.${path}` }, + ); + await page.reload(); + + await page + .getByTestId(`nested-filter-group-ResourceAttributesJSON.${path}`) + .click(); + await page.getByTestId(`filter-load-more-${path}`).click(); + for (const value of values) { + await expect( + searchPage.filters.getFilterCheckboxInput(path, value), + ).toBeVisible({ timeout: QUERY_TIMEOUT }); + } + + await searchPage.filters.showDistribution(path); + await expect( + searchPage.filters.getDistributionPercentage(path, values[0]), + ).toContainText('33', { timeout: QUERY_TIMEOUT }); + + const rows = searchPage.table.getRows(); + await searchPage.filters.applyFilter(path, values[0]); + await expect(rows).toHaveCount(1, { timeout: QUERY_TIMEOUT }); + await expect(rows).toContainText(ROW1.body); + await expect(searchPage.getTableError()).toHaveCount(0); + + await page.reload(); + await expect(page).toHaveURL(/filters=/); + await expect(rows).toHaveCount(1, { timeout: QUERY_TIMEOUT }); + await expect(rows).toContainText(ROW1.body); + await expect(searchPage.getTableError()).toHaveCount(0); + + await page.getByTestId(`filter-load-more-${path}`).click(); + await expect( + searchPage.filters.getFilterCheckboxInput(path, values[1]), + ).toBeVisible({ timeout: QUERY_TIMEOUT }); + await expect( + searchPage.filters.getFilterCheckboxInput(path, values[0]), + ).toBeChecked(); + await searchPage.filters + .getFilterCheckboxInput(path, values[0]) + .click(); + await expect(rows).toHaveCount(3, { timeout: QUERY_TIMEOUT }); + await expect(searchPage.getTableError()).toHaveCount(0); + + await page.getByTestId(`filter-load-more-${path}`).click(); + await expect( + searchPage.filters.getFilterCheckboxInput(path, values[0]), + ).toBeVisible({ timeout: QUERY_TIMEOUT }); + await expect( + searchPage.filters.getFilterCheckboxInput(path, values[0]), + ).not.toBeChecked(); + await searchPage.filters.excludeFilter(path, values[0]); + await expect(rows).toHaveCount(2, { timeout: QUERY_TIMEOUT }); + await expect(rows.filter({ hasText: ROW1.body })).toHaveCount(0); + await expect(rows.filter({ hasText: ROW2.body })).toHaveCount(1); + await expect(rows.filter({ hasText: ROW3.body })).toHaveCount(1); + await expect(searchPage.getTableError()).toHaveCount(0); + + await page.reload(); + await expect(page).toHaveURL(/filters=/); + await expect(rows).toHaveCount(2, { timeout: QUERY_TIMEOUT }); + await expect(rows.filter({ hasText: ROW1.body })).toHaveCount(0); + await expect(searchPage.getTableError()).toHaveCount(0); + + await page + .getByTestId(`nested-filter-group-ResourceAttributesJSON.${path}`) + .locator('button:has(.tabler-icon-filter-off)') + .click(); + await expect(rows).toHaveCount(3, { timeout: QUERY_TIMEOUT }); + await expect(searchPage.getTableError()).toHaveCount(0); + }); + } + }, +); diff --git a/packages/app/tests/e2e/seed-clickhouse.ts b/packages/app/tests/e2e/seed-clickhouse.ts index 9736ffdaea..a2a901b091 100644 --- a/packages/app/tests/e2e/seed-clickhouse.ts +++ b/packages/app/tests/e2e/seed-clickhouse.ts @@ -93,6 +93,9 @@ export const INTERESTING_FILTER_KEYS_ROWS = [ serviceName: 'service1', resourceAttrValue: 'value1', jsonValue: 'value1', + jsonInteger: 42, + jsonMixed: 42, + jsonBoolean: true, clusterName: 'cluster1', serviceNameHyphen: 'svc-one', mapHyphenValue: 'mapval1', @@ -103,6 +106,9 @@ export const INTERESTING_FILTER_KEYS_ROWS = [ serviceName: 'service2', resourceAttrValue: 'value2', jsonValue: 'value2', + jsonInteger: 43, + jsonMixed: '43', + jsonBoolean: false, clusterName: 'cluster2', serviceNameHyphen: 'svc-two', mapHyphenValue: 'mapval2', @@ -113,6 +119,9 @@ export const INTERESTING_FILTER_KEYS_ROWS = [ serviceName: 'service3', resourceAttrValue: 'value3', jsonValue: 'value3', + jsonInteger: 44, + jsonMixed: '44', + jsonBoolean: false, clusterName: 'cluster3', serviceNameHyphen: 'svc-three', mapHyphenValue: 'mapval3', @@ -788,7 +797,12 @@ function generateK8sEventLogs( function generateInterestingFilterKeyLogData(seedRef: number): string { return INTERESTING_FILTER_KEYS_ROWS.map((row, i) => { const timestampNs = (seedRef - (i + 1) * 1000) * 1000000; - const json = `{"key": {"subKey": {"subSubKey": "${row.jsonValue}"}}}`; + const json = JSON.stringify({ + key: { subKey: { subSubKey: row.jsonValue } }, + integer: row.jsonInteger, + mixed: row.jsonMixed, + boolean: row.jsonBoolean, + }); // JSON column with a hyphenated name AND hyphenated nested keys. const jsonHyphen = `{"key-1": {"key-2": "${row.jsonHyphenValue}"}}`; return ( diff --git a/packages/common-utils/src/__tests__/metadata.test.ts b/packages/common-utils/src/__tests__/metadata.test.ts index 1489354d2e..1edf8d5d97 100644 --- a/packages/common-utils/src/__tests__/metadata.test.ts +++ b/packages/common-utils/src/__tests__/metadata.test.ts @@ -654,7 +654,7 @@ describe('Metadata', () => { }); it('renders faceted conditions the same way as the keys they constrain', async () => { - // A JSON column renders to a typed subcolumn rather than the bracket + // A JSON column renders to a string expression rather than the bracket // form the caller wrote. If the predicate kept the raw expression it // would address a different (or non-existent) column than the aggregate // wrapping it, so both halves must come out rendered. @@ -693,8 +693,8 @@ describe('Metadata', () => { if (!isBuilderChartConfig(actualConfig)) throw new Error('Expected builder config'); expect(actualConfig.select).toContain( - 'groupUniqArrayIf(10)(Attributes.`namespace`.:String, ' + - "(Attributes.`cluster`.:String IN ('prod'))) AS param1", + 'groupUniqArrayIf(10)(toString(Attributes.`namespace`), ' + + "(toString(Attributes.`cluster`) IN ('prod'))) AS param1", ); // The raw bracket form must not survive into either half. expect(actualConfig.select).not.toContain("Attributes['"); @@ -801,7 +801,7 @@ describe('Metadata', () => { expect(renderChartConfigSpy).not.toHaveBeenCalled(); }); - it('renders JSON attribute keys as typed subcolumns', async () => { + it('renders JSON attribute keys as string expressions', async () => { jest.spyOn(metadata, 'getColumn').mockImplementation(({ column }) => Promise.resolve( column === 'ResourceAttributes' @@ -830,7 +830,7 @@ describe('Metadata', () => { expect(actualConfig.with?.[0]).toMatchObject({ chartConfig: { select: - 'ResourceAttributes.`k8s`.`namespace`.`name`.:String as param0', + 'toString(ResourceAttributes.`k8s`.`namespace`.`name`) as param0', }, }); }); @@ -864,7 +864,7 @@ describe('Metadata', () => { expect(actualConfig.with?.[0]).toMatchObject({ chartConfig: { select: - 'ResourceAttributes.`foo`.`:String, count() AS injected`.:String as param0', + 'toString(ResourceAttributes.`foo`.`:String, count() AS injected`) as param0', }, }); }); @@ -1999,7 +1999,7 @@ describe('Metadata', () => { }); }); - it('renders JSON distribution keys as typed subcolumns', async () => { + it('renders JSON distribution keys as string expressions', async () => { jest.spyOn(metadata, 'getColumn').mockImplementation(({ column }) => Promise.resolve( column === 'ResourceAttributes' @@ -2025,7 +2025,7 @@ describe('Metadata', () => { if (!isBuilderChartConfig(actualConfig)) throw new Error('Expected builder config'); expect(actualConfig.select).toBe( - 'ResourceAttributes.`k8s`.`namespace`.`name`.:String AS __hdx_value, count() as __hdx_count, __hdx_count / (sum(__hdx_count) OVER ()) * 100 AS __hdx_percentage', + 'toString(ResourceAttributes.`k8s`.`namespace`.`name`) AS __hdx_value, count() as __hdx_count, __hdx_count / (sum(__hdx_count) OVER ()) * 100 AS __hdx_percentage', ); expect(actualConfig.groupBy).toBe('__hdx_value'); }); @@ -2056,7 +2056,7 @@ describe('Metadata', () => { if (!isBuilderChartConfig(actualConfig)) throw new Error('Expected builder config'); expect(actualConfig.select).toBe( - 'ResourceAttributes.`k8s`.`namespace`.`name`.:String AS __hdx_value, count() as __hdx_count, __hdx_count / (sum(__hdx_count) OVER ()) * 100 AS __hdx_percentage', + 'toString(ResourceAttributes.`k8s`.`namespace`.`name`) AS __hdx_value, count() as __hdx_count, __hdx_count / (sum(__hdx_count) OVER ()) * 100 AS __hdx_percentage', ); }); }); @@ -3042,7 +3042,7 @@ describe('Metadata', () => { const valuesCall = (mockClickhouseClient.query as jest.Mock).mock .calls[0][0]; expect(valuesCall.query).toContain( - 'ResourceAttributes.`k8s`.`namespace`.`name`.:String as value', + 'toString(ResourceAttributes.`k8s`.`namespace`.`name`) as value', ); expect(valuesCall.query).not.toContain('ResourceAttributes['); }); diff --git a/packages/common-utils/src/core/metadata.ts b/packages/common-utils/src/core/metadata.ts index c6a5f64311..757c47435f 100644 --- a/packages/common-utils/src/core/metadata.ts +++ b/packages/common-utils/src/core/metadata.ts @@ -229,7 +229,7 @@ const identifierPattern = (identifier: string): string => { const JSON_STRING_TYPE_SUFFIX = '.:String'; -const renderJsonStringSubcolumn = ( +export const renderJsonStringSubcolumn = ( column: string, jsonPath: string, options: { preserveStringTypeSuffix?: boolean } = {}, @@ -247,7 +247,7 @@ const renderJsonStringSubcolumn = ( .map(quoteJsonPathSegment) .join('.'); - return `${columnIdentifier}.${path}${JSON_STRING_TYPE_SUFFIX}`; + return `toString(${columnIdentifier}.${path})`; }; export class MetadataCache { diff --git a/packages/common-utils/src/filters.ts b/packages/common-utils/src/filters.ts index 78e3d09c66..7410c4f77b 100644 --- a/packages/common-utils/src/filters.ts +++ b/packages/common-utils/src/filters.ts @@ -126,8 +126,8 @@ export const filtersToQuery = ( * Callers that also emit the same keys elsewhere in the query (e.g. inside a * SELECT aggregate) must render both halves the same way, or the predicate * silently addresses a different expression than the one being aggregated. - * `stringifyKeys` is deliberately false: a rendered JSON path already carries - * the `.:String` type suffix, so it needs no `toString()` wrapper. + * `stringifyKeys` is deliberately false: rendered JSON paths already carry + * their own `toString()` wrapper. * * Returns undefined when nothing is selected, so callers can branch on * "constrained vs unconstrained" without inspecting the string. From 738cb2a1b090866e8b757719659b0427351bd4ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Fr=C3=B6mbgen?= <23717573+mfroembgen@users.noreply.github.com> Date: Wed, 7 Oct 2026 14:30:04 +0200 Subject: [PATCH 2/2] fix: preserve quoted SQL expression filter keys --- .../app/src/__tests__/searchFilters.test.ts | 49 +++++++++++++++++++ .../components/DBSearchPageFilters/utils.ts | 14 +++--- 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/packages/app/src/__tests__/searchFilters.test.ts b/packages/app/src/__tests__/searchFilters.test.ts index 9881db08fb..8cca209136 100644 --- a/packages/app/src/__tests__/searchFilters.test.ts +++ b/packages/app/src/__tests__/searchFilters.test.ts @@ -14,6 +14,55 @@ enableMapSet(); type ConditionFilter = { type: 'sql' | 'lucene'; condition: string }; describe('searchFilters', () => { + it.each([ + "JSONExtractString(Attrs.`my-key`, 'x')", + "toString(JSONExtractString(Attrs.`my-key`, 'x'))", + ])( + 'preserves quoted SQL expression %s through filter changes', + expression => { + const onFilterChange = jest.fn(); + const emptyQuery: Filter[] = []; + const { result, rerender } = renderHook( + ({ searchQuery }: { searchQuery: Filter[] }) => + useSearchPageFilterState({ + searchQuery, + onFilterChange, + knownColumns: new Set(['Attrs']), + jsonColumns: new Set(['Attrs']), + }), + { initialProps: { searchQuery: emptyQuery } }, + ); + act(() => result.current.setFilterValue(expression, 'USD')); + const searchQuery: Filter[] = [ + { type: 'sql', condition: `${expression} IN ('USD')` }, + ]; + expect(onFilterChange).toHaveBeenLastCalledWith(searchQuery); + rerender({ searchQuery }); + expect(result.current.filters[expression].included).toEqual( + new Set(['USD']), + ); + act(() => + result.current.replaceFilterValue(expression, 'USD', 'EUR', 'include'), + ); + expect(onFilterChange).toHaveBeenLastCalledWith([ + { type: 'sql', condition: `${expression} IN ('EUR')` }, + ]); + act(() => result.current.clearFilter(expression)); + expect(onFilterChange).toHaveBeenLastCalledWith([]); + act(() => + result.current.setOnlyFilters([{ property: expression, value: 'GBP' }]), + ); + expect(onFilterChange).toHaveBeenLastCalledWith([ + { type: 'sql', condition: `${expression} IN ('GBP')` }, + ]); + act(() => result.current.setFilterRange(expression, { min: 1, max: 5 })); + expect(onFilterChange).toHaveBeenLastCalledWith([ + { type: 'sql', condition: `${expression} IN ('GBP')` }, + { type: 'sql', condition: `${expression} BETWEEN 1 AND 5` }, + ]); + }, + ); + it.each([ [ 'ResourceAttributes', diff --git a/packages/app/src/components/DBSearchPageFilters/utils.ts b/packages/app/src/components/DBSearchPageFilters/utils.ts index e7dc3f6fba..de4785d393 100644 --- a/packages/app/src/components/DBSearchPageFilters/utils.ts +++ b/packages/app/src/components/DBSearchPageFilters/utils.ts @@ -65,10 +65,18 @@ export function listRequestedFacets({ return { facets, pendingKeys }; } +// A key that begins with `identifier(` is a raw SQL function call (e.g. +// `toString(...)`, `JSONExtractString(...)`), not a column name or a dot-form +// Map sub-key, so it is already a valid ClickHouse expression. +const isSqlFunctionCallExpression = (key: string): boolean => + /^[A-Za-z_]\w*\(/.test(key); + // Clean ClickHouse expressions to extract clean property paths export function cleanClickHouseExpression(key: string): string { // Remove toString() wrapper if present let cleanKey = key.replace(/^toString\((.+)\)$/, '$1'); + // Quoting inside a SQL function belongs to the expression, not the field key. + if (isSqlFunctionCallExpression(cleanKey)) return key; // Convert backtick dot notation to clean dot notation // e.g., `host`.`arch` -> host.arch @@ -237,12 +245,6 @@ export function getFilterStateEntry( ); } -// A key that begins with `identifier(` is a raw SQL function call (e.g. -// `toString(...)`, `JSONExtractString(...)`), not a column name or a dot-form -// Map sub-key, so it is already a valid ClickHouse expression. -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