From 2d4bc2507f45ae23b7c9c9393029212c4e84b28f Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Fri, 5 Jun 2026 10:40:26 -0400 Subject: [PATCH 01/11] feat(dashboards): cascading (faceted) filter values Dashboard filter dropdowns now narrow one another: selecting a value in one filter constrains the options shown by the others to values that co-occur with the current selection (e.g. picking a cluster limits the namespace dropdown to namespaces in that cluster). Applies to both manually-created dashboards and the bundled Kubernetes page (which also honors its free-text search). A filter never constrains its own options, so multi-select within a filter still works. HDX-4462 --- .changeset/cascading-dashboard-filters.md | 13 ++ packages/app/src/DashboardFilters.tsx | 1 + .../app/src/components/KubernetesFilters.tsx | 49 +++-- .../__tests__/KubernetesFilters.test.ts | 43 ++++ .../useDashboardFilterValues.test.tsx | 206 ++++++++++++++++++ .../src/hooks/useDashboardFilterValues.tsx | 100 +++++++-- 6 files changed, 382 insertions(+), 30 deletions(-) create mode 100644 .changeset/cascading-dashboard-filters.md create mode 100644 packages/app/src/components/__tests__/KubernetesFilters.test.ts diff --git a/.changeset/cascading-dashboard-filters.md b/.changeset/cascading-dashboard-filters.md new file mode 100644 index 0000000000..c2734ed68d --- /dev/null +++ b/.changeset/cascading-dashboard-filters.md @@ -0,0 +1,13 @@ +--- +'@hyperdx/app': patch +--- + +feat(dashboards): cascading (faceted) filter values + +Dashboard filter dropdowns now narrow one another: selecting a value in one +filter constrains the options shown by the others to values that co-occur with +the current selection (e.g. picking a `cluster` limits the `namespace` dropdown +to namespaces in that cluster). This applies to both manually-created +dashboards and the bundled Kubernetes dashboard, where the dropdowns also honor +the free-text search. A filter never constrains its own options, so +multi-select within a single filter still works. diff --git a/packages/app/src/DashboardFilters.tsx b/packages/app/src/DashboardFilters.tsx index 78a29dfdb6..3e2db9ac8f 100644 --- a/packages/app/src/DashboardFilters.tsx +++ b/packages/app/src/DashboardFilters.tsx @@ -74,6 +74,7 @@ const DashboardFilters = ({ const { data: filterValuesById, isFetching } = useDashboardFilterValues({ filters, dateRange, + filterValues, }); return ( diff --git a/packages/app/src/components/KubernetesFilters.tsx b/packages/app/src/components/KubernetesFilters.tsx index 3367ad6359..8844286364 100644 --- a/packages/app/src/components/KubernetesFilters.tsx +++ b/packages/app/src/components/KubernetesFilters.tsx @@ -27,6 +27,22 @@ type FilterSelectProps = { dataTestId?: string; }; +// Removes a single field's `resourceAttr.field:"..."` clause from a Lucene +// query string, leaving every other clause (and free-text search) intact. Used +// both to rewrite the query when a dropdown changes and to build the faceted +// `where` for each dropdown's value lookup. +export const stripFieldClause = ( + query: string, + resourceAttr: string, + field: string, +): string => { + const fullAttribute = `${resourceAttr}.${field}`; + const regex = new RegExp(`${fullAttribute}:"[^"]*"`, 'g'); + // Replace with a space and collapse runs of whitespace so removing a clause + // from the middle of the query doesn't leave a double space. + return query.replace(regex, ' ').replace(/\s+/g, ' ').trim(); +}; + const FilterSelect: React.FC = ({ metricSource, placeholder, @@ -156,19 +172,28 @@ export const KubernetesFilters: React.FC = ({ } }, [searchQuery, metricSource.resourceAttributesExpression]); - // Create chart config for fetching key values - const chartConfig: BuilderChartConfigWithDateRange = { + // Build a chart config for fetching a single field's selectable values. + // Faceted filtering: constrain the values by every OTHER active K8s filter and + // the free-text search (i.e. the whole searchQuery except this field's own + // clause), so e.g. picking a cluster narrows the namespace list. + const buildChartConfigForField = ( + field: string, + ): BuilderChartConfigWithDateRange => ({ from: { databaseName: metricSource.from.databaseName, tableName: metricSource.metricTables?.gauge || '', }, - where: '', - whereLanguage: 'sql', + where: stripFieldClause( + searchQuery, + metricSource.resourceAttributesExpression, + field, + ), + whereLanguage: 'lucene', select: '', timestampValueExpression: metricSource.timestampValueExpression || '', connection: metricSource.connection, dateRange, - }; + }); // Helper function to update search query const updateSearchQuery = ( @@ -182,9 +207,7 @@ export const KubernetesFilters: React.FC = ({ const fullAttribute = `${resourceAttr}.${attribute}`; // Remove existing filter for this attribute if it exists - let newQuery = searchQuery; - const regex = new RegExp(`${fullAttribute}:"[^"]*"`, 'g'); - newQuery = newQuery.replace(regex, '').trim(); + let newQuery = stripFieldClause(searchQuery, resourceAttr, attribute); // Add new filter if value is not null if (value) { @@ -202,7 +225,7 @@ export const KubernetesFilters: React.FC = ({ fieldName="k8s.pod.name" value={podName} onChange={value => updateSearchQuery('k8s.pod.name', value, setPodName)} - chartConfig={chartConfig} + chartConfig={buildChartConfigForField('k8s.pod.name')} dataTestId="pod-filter-select" /> @@ -214,7 +237,7 @@ export const KubernetesFilters: React.FC = ({ onChange={value => updateSearchQuery('k8s.deployment.name', value, setDeploymentName) } - chartConfig={chartConfig} + chartConfig={buildChartConfigForField('k8s.deployment.name')} dataTestId="deployment-filter-select" /> @@ -226,7 +249,7 @@ export const KubernetesFilters: React.FC = ({ onChange={value => updateSearchQuery('k8s.node.name', value, setNodeName) } - chartConfig={chartConfig} + chartConfig={buildChartConfigForField('k8s.node.name')} dataTestId="node-filter-select" /> @@ -238,7 +261,7 @@ export const KubernetesFilters: React.FC = ({ onChange={value => updateSearchQuery('k8s.namespace.name', value, setNamespaceName) } - chartConfig={chartConfig} + chartConfig={buildChartConfigForField('k8s.namespace.name')} dataTestId="namespace-filter-select" /> @@ -250,7 +273,7 @@ export const KubernetesFilters: React.FC = ({ onChange={value => updateSearchQuery('k8s.cluster.name', value, setClusterName) } - chartConfig={chartConfig} + chartConfig={buildChartConfigForField('k8s.cluster.name')} dataTestId="cluster-filter-select" /> diff --git a/packages/app/src/components/__tests__/KubernetesFilters.test.ts b/packages/app/src/components/__tests__/KubernetesFilters.test.ts new file mode 100644 index 0000000000..bf54190bfc --- /dev/null +++ b/packages/app/src/components/__tests__/KubernetesFilters.test.ts @@ -0,0 +1,43 @@ +import { stripFieldClause } from '../KubernetesFilters'; + +describe('stripFieldClause', () => { + const resourceAttr = 'ResourceAttributes'; + + it('removes only the target field clause, keeping other filters and free text', () => { + const query = + 'ResourceAttributes.k8s.cluster.name:"prod" ResourceAttributes.k8s.namespace.name:"api" error'; + + // Building the faceted `where` for the namespace dropdown drops the + // namespace clause but keeps the cluster selection and free-text search, + // so the namespace options are narrowed to the selected cluster. + expect(stripFieldClause(query, resourceAttr, 'k8s.namespace.name')).toBe( + 'ResourceAttributes.k8s.cluster.name:"prod" error', + ); + }); + + it('returns an empty string when the query only contains the target clause', () => { + expect( + stripFieldClause( + 'ResourceAttributes.k8s.cluster.name:"prod"', + resourceAttr, + 'k8s.cluster.name', + ), + ).toBe(''); + }); + + it('leaves the query unchanged when the target field is absent', () => { + const query = 'ResourceAttributes.k8s.cluster.name:"prod"'; + expect(stripFieldClause(query, resourceAttr, 'k8s.namespace.name')).toBe( + query, + ); + }); + + it('does not strip a sibling field that shares a path prefix', () => { + // `k8s.pod.name` must not match `k8s.pod.uid`. + const query = + 'ResourceAttributes.k8s.pod.name:"a" ResourceAttributes.k8s.pod.uid:"b"'; + expect(stripFieldClause(query, resourceAttr, 'k8s.pod.name')).toBe( + 'ResourceAttributes.k8s.pod.uid:"b"', + ); + }); +}); diff --git a/packages/app/src/hooks/__tests__/useDashboardFilterValues.test.tsx b/packages/app/src/hooks/__tests__/useDashboardFilterValues.test.tsx index 98837b38d8..d837125152 100644 --- a/packages/app/src/hooks/__tests__/useDashboardFilterValues.test.tsx +++ b/packages/app/src/hooks/__tests__/useDashboardFilterValues.test.tsx @@ -2,6 +2,7 @@ import React from 'react'; import { optimizeGetKeyValuesCalls } from '@hyperdx/common-utils/dist/core/materializedViews'; import { Metadata } from '@hyperdx/common-utils/dist/core/metadata'; +import { FilterState } from '@hyperdx/common-utils/dist/filters'; import { DashboardFilter, MetricsDataType, @@ -537,6 +538,7 @@ describe('useDashboardFilterValues', () => { dateRange: mockDateRange, where: '', whereLanguage: 'sql', + filters: [], select: '', }, keys: ['environment'], @@ -1012,4 +1014,208 @@ describe('useDashboardFilterValues', () => { }), ); }); + + describe('faceted filtering (cascading filters)', () => { + // Returns the `filters` array passed to the getKeyValues call whose `keys` + // exactly match, or undefined if no such call was made. + const filtersForKeys = (keys: string[]) => + (mockMetadata.getKeyValues.mock.calls as any[]).find( + ([arg]) => JSON.stringify(arg.keys) === JSON.stringify(keys), + )?.[0]?.chartConfig?.filters; + + const envAndStatus: DashboardFilter[] = [ + { + id: 'filter1', + type: 'QUERY_EXPRESSION', + name: 'Environment', + expression: 'environment', + source: 'logs-source', + }, + { + id: 'filter2', + type: 'QUERY_EXPRESSION', + name: 'Status', + expression: 'status', + source: 'logs-source', + }, + ]; + + beforeEach(() => { + jest.spyOn(sourceModule, 'useSources').mockReturnValue({ + data: mockSources, + isLoading: false, + } as any); + // A prior test may have overridden the implementation (clearAllMocks + // resets call data but not implementations); restore the passthrough so + // each key group reaches getKeyValues with its own keys + chartConfig + // (including the faceting `filters`). + jest + .mocked(optimizeGetKeyValuesCalls) + .mockImplementation(async ({ keys, chartConfig }) => [ + { keys, chartConfig }, + ]); + }); + + it('narrows sibling filters by the current selection and excludes self', async () => { + const { result } = renderHook( + () => + useDashboardFilterValues({ + filters: envAndStatus, + dateRange: mockDateRange, + filterValues: { + environment: { + included: new Set(['production']), + excluded: new Set(), + }, + }, + }), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + // The two filters can no longer batch: each has a distinct constraint set. + expect(mockMetadata.getKeyValues).toHaveBeenCalledTimes(2); + + // `status` values are constrained by the `environment` selection. + expect(filtersForKeys(['status'])).toEqual([ + { type: 'sql', condition: "environment IN ('production')" }, + ]); + + // `environment` is NOT constrained by its own selection (exclude-self), + // so the user can still see and change among all environments. + expect(filtersForKeys(['environment'])).toEqual([]); + }); + + it('batches unselected same-source filters into one unconstrained query', async () => { + const { result } = renderHook( + () => + useDashboardFilterValues({ + filters: envAndStatus, + dateRange: mockDateRange, + filterValues: {}, + }), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + expect(mockMetadata.getKeyValues).toHaveBeenCalledTimes(1); + expect(filtersForKeys(['environment', 'status'])).toEqual([]); + }); + + it('keeps unselected siblings batched while the selected filter splits off', async () => { + const filters: DashboardFilter[] = [ + ...envAndStatus, + { + id: 'filter3', + type: 'QUERY_EXPRESSION', + name: 'Log Level', + expression: 'log_level', + source: 'logs-source', + }, + ]; + + const { result } = renderHook( + () => + useDashboardFilterValues({ + filters, + dateRange: mockDateRange, + filterValues: { + environment: { + included: new Set(['production']), + excluded: new Set(), + }, + }, + }), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + // Two queries: the selected filter alone, and the two unselected siblings + // batched together (they share the same constraint set). + expect(mockMetadata.getKeyValues).toHaveBeenCalledTimes(2); + expect(filtersForKeys(['status', 'log_level'])).toEqual([ + { type: 'sql', condition: "environment IN ('production')" }, + ]); + expect(filtersForKeys(['environment'])).toEqual([]); + }); + + it('does not apply a selection from one source to filters on another source', async () => { + const filters: DashboardFilter[] = [ + { + id: 'filter1', + type: 'QUERY_EXPRESSION', + name: 'Environment', + expression: 'environment', + source: 'logs-source', + }, + { + id: 'filter2', + type: 'QUERY_EXPRESSION', + name: 'Service', + expression: 'service.name', + source: 'traces-source', + }, + ]; + + const { result } = renderHook( + () => + useDashboardFilterValues({ + filters, + dateRange: mockDateRange, + filterValues: { + environment: { + included: new Set(['production']), + excluded: new Set(), + }, + }, + }), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + // A logs-source selection must not leak into a traces-source value query + // (the column may not exist there). + expect(filtersForKeys(['service.name'])).toEqual([]); + expect(filtersForKeys(['environment'])).toEqual([]); + }); + + it('refetches with updated constraints when a selection changes', async () => { + const { result, rerender } = renderHook( + ({ filterValues }) => + useDashboardFilterValues({ + filters: envAndStatus, + dateRange: mockDateRange, + filterValues, + }), + { + wrapper, + initialProps: { filterValues: {} as FilterState }, + }, + ); + + await waitFor(() => expect(result.current.isFetching).toBe(false)); + // Nothing selected → both filters batch into one query. + expect(mockMetadata.getKeyValues).toHaveBeenCalledTimes(1); + + rerender({ + filterValues: { + environment: { + included: new Set(['production']), + excluded: new Set(), + }, + }, + }); + + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + // `status` is re-fetched with the new constraint. + expect(filtersForKeys(['status'])).toEqual([ + { type: 'sql', condition: "environment IN ('production')" }, + ]); + }); + }); }); diff --git a/packages/app/src/hooks/useDashboardFilterValues.tsx b/packages/app/src/hooks/useDashboardFilterValues.tsx index 8e36a05668..e65f1f0c55 100644 --- a/packages/app/src/hooks/useDashboardFilterValues.tsx +++ b/packages/app/src/hooks/useDashboardFilterValues.tsx @@ -4,9 +4,14 @@ import { GetKeyValueCall, optimizeGetKeyValuesCalls, } from '@hyperdx/common-utils/dist/core/materializedViews'; +import { + FilterState, + filtersToQuery, +} from '@hyperdx/common-utils/dist/filters'; import { BuilderChartConfigWithDateRange, DashboardFilter, + Filter, isLogSource, isTraceSource, } from '@hyperdx/common-utils/dist/types'; @@ -37,9 +42,6 @@ const filterToKey = (filter: DashboardFilter): string => whereLanguage: filter.whereLanguage ?? 'sql', } satisfies FilterSourceKey); -const filterFromKey = (key: string): FilterSourceKey => - JSON.parse(key) as FilterSourceKey; - type EnrichedCall = GetKeyValueCall & { /** filterIds[i] = array of filter IDs whose values come from keys[i] */ filterIds: string[][]; @@ -48,36 +50,93 @@ type EnrichedCall = GetKeyValueCall & { function useOptimizedKeyValuesCalls({ filters, dateRange, + filterValues, }: { filters: DashboardFilter[]; dateRange: [Date, Date]; + filterValues: FilterState; }) { const clickhouseClient = useClickhouseClient(); const metadata = useMetadataWithSettings(); const { data: sources, isLoading: isLoadingSources } = useSources(); - // Group filters by (source, metricType, where, whereLanguage) so that we can test each group for MV compatibility separately. + // Faceted filtering: each filter's selectable values are narrowed by the + // CURRENT selections of its sibling filters. For every filter, build the + // constraint from the selections of the OTHER filters that target the same + // source + metric type (so the constrained columns are guaranteed to exist in + // the queried table), excluding the filter's own expression (otherwise a + // multi-select would collapse to only its already-selected values). + const constraintsByFilterId = useMemo(() => { + const byId = new Map(); + for (const filter of filters) { + const prunedState: FilterState = {}; + for (const sibling of filters) { + if ( + sibling.source !== filter.source || + sibling.sourceMetricType !== filter.sourceMetricType || + // Exclude-self: FilterState is keyed by expression, so a sibling that + // shares this filter's expression carries this filter's own selection. + sibling.expression === filter.expression + ) { + continue; + } + const selection = filterValues[sibling.expression]; + if ( + selection && + (selection.included.size > 0 || + selection.excluded.size > 0 || + selection.range != null) + ) { + prunedState[sibling.expression] = selection; + } + } + byId.set( + filter.id, + filtersToQuery(prunedState, { stringifyKeys: false }), + ); + } + return byId; + }, [filters, filterValues]); + + // Group filters by (source, metricType, where, whereLanguage) AND their + // effective constraint signature, so each group can be tested for MV + // compatibility separately. Filters with an identical constraint set — in + // particular every currently-unselected filter of a source — stay batched in a + // single key-values query; each selected filter (which omits its own + // expression) splits into its own query. const filtersByGroupKey = useMemo(() => { - const filtersByGroupKey = new Map(); + const byGroupKey = new Map< + string, + { filters: DashboardFilter[]; constraints: Filter[] } + >(); for (const filter of filters) { - const key = filterToKey(filter); - if (!filtersByGroupKey.has(key)) { - filtersByGroupKey.set(key, [filter]); + const constraints = constraintsByFilterId.get(filter.id) ?? []; + const constraintsSig = constraints + .map(c => JSON.stringify(c)) + .sort() + .join('|'); + const key = `${filterToKey(filter)}::${constraintsSig}`; + const existing = byGroupKey.get(key); + if (existing) { + existing.filters.push(filter); } else { - filtersByGroupKey.get(key)!.push(filter); + byGroupKey.set(key, { filters: [filter], constraints }); } } - return filtersByGroupKey; - }, [filters]); + return byGroupKey; + }, [filters, constraintsByFilterId]); const results: UseQueryResult[] = useQueries({ - queries: Array.from(filtersByGroupKey.entries()) - .filter(([key]) => - sources?.some(s => s.id === filterFromKey(key).sourceId), + queries: Array.from(filtersByGroupKey.values()) + .filter(({ filters: filtersInGroup }) => + sources?.some(s => s.id === filtersInGroup[0].source), ) - .map(([key, filtersInGroup]) => { - const { sourceId, metricType, where, whereLanguage } = - filterFromKey(key); + .map(({ filters: filtersInGroup, constraints }) => { + const representative = filtersInGroup[0]; + const sourceId = representative.source; + const metricType = representative.sourceMetricType; + const where = representative.where ?? ''; + const whereLanguage = representative.whereLanguage ?? 'sql'; const source = sources!.find(s => s.id === sourceId)!; const keyExpressions = filtersInGroup.map(f => f.expression); const tableName = getMetricTableName(source, metricType) ?? ''; @@ -104,6 +163,9 @@ function useOptimizedKeyValuesCalls({ source: source.id, where, whereLanguage, + // Sibling-selection constraints (faceted filtering); combined with the + // static `where` via AND inside renderChartConfig. + filters: constraints, select: '', }; @@ -116,6 +178,7 @@ function useOptimizedKeyValuesCalls({ keyExpressions, where, whereLanguage, + constraints, ], enabled: !isLoadingSources, staleTime: 1000 * 60 * 5, // Cache every 5 min @@ -152,9 +215,11 @@ function useOptimizedKeyValuesCalls({ export function useDashboardFilterValues({ filters, dateRange, + filterValues = {}, }: { filters: DashboardFilter[]; dateRange: [Date, Date]; + filterValues?: FilterState; }) { const metadata = useMetadataWithSettings(); const { @@ -164,6 +229,7 @@ export function useDashboardFilterValues({ } = useOptimizedKeyValuesCalls({ filters, dateRange, + filterValues, }); const { data: sources, isLoading: isSourcesLoading } = useSources(); From e6a2ed49537584cb9a7672594c4abb71177d9648 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Fri, 5 Jun 2026 11:40:47 -0400 Subject: [PATCH 02/11] fix(k8s-filters): escape filter regexes and memoize chart configs stripFieldClause and extractValueFromSearchQuery interpolated the attribute name straight into a RegExp. Escape it (lodash escapeRegExp) so dots match literally instead of as wildcards, and metacharacters like '(' or '[' in a resource-attribute expression can't throw a SyntaxError. Memoize the five FilterSelect chart configs in a single useMemo (keyed on searchQuery, dateRange, metricSource) so they keep a stable identity across re-renders unrelated to the filters, avoiding repeated React Query key serialization. (useCallback on the builder wouldn't help: calling it still mints a new object per render.) Add tests covering literal-dot matching and the metacharacter case. --- .../app/src/components/KubernetesFilters.tsx | 67 +++++++++++-------- .../__tests__/KubernetesFilters.test.ts | 16 +++++ 2 files changed, 55 insertions(+), 28 deletions(-) diff --git a/packages/app/src/components/KubernetesFilters.tsx b/packages/app/src/components/KubernetesFilters.tsx index 8844286364..0d4611c954 100644 --- a/packages/app/src/components/KubernetesFilters.tsx +++ b/packages/app/src/components/KubernetesFilters.tsx @@ -1,4 +1,5 @@ import React, { useEffect, useMemo, useRef, useState } from 'react'; +import { escapeRegExp } from 'lodash'; import { useForm, useWatch } from 'react-hook-form'; import { tcFromSource } from '@hyperdx/common-utils/dist/core/metadata'; import { @@ -37,7 +38,7 @@ export const stripFieldClause = ( field: string, ): string => { const fullAttribute = `${resourceAttr}.${field}`; - const regex = new RegExp(`${fullAttribute}:"[^"]*"`, 'g'); + const regex = new RegExp(`${escapeRegExp(fullAttribute)}:"[^"]*"`, 'g'); // Replace with a space and collapse runs of whitespace so removing a clause // from the middle of the query doesn't leave a double space. return query.replace(regex, ' ').replace(/\s+/g, ' ').trim(); @@ -131,8 +132,9 @@ export const KubernetesFilters: React.FC = ({ resourceAttr: string = '', attribute: string, ) => { + const fullAttribute = `${resourceAttr}.${attribute}`; const match = searchQuery.match( - new RegExp(`${resourceAttr}\\.${attribute}:"([^"]+)"`, 'i'), + new RegExp(`${escapeRegExp(fullAttribute)}:"([^"]+)"`, 'i'), ); return match ? match[1] : null; }; @@ -172,28 +174,37 @@ export const KubernetesFilters: React.FC = ({ } }, [searchQuery, metricSource.resourceAttributesExpression]); - // Build a chart config for fetching a single field's selectable values. - // Faceted filtering: constrain the values by every OTHER active K8s filter and - // the free-text search (i.e. the whole searchQuery except this field's own + // Build a chart config for fetching each field's selectable values. Memoized so + // the config objects keep a stable identity across re-renders unrelated to the + // filters (each is spread into useGetKeyValues' React Query key). + // Faceted filtering: constrain a field's values by every OTHER active K8s filter + // and the free-text search (i.e. the whole searchQuery except this field's own // clause), so e.g. picking a cluster narrows the namespace list. - const buildChartConfigForField = ( - field: string, - ): BuilderChartConfigWithDateRange => ({ - from: { - databaseName: metricSource.from.databaseName, - tableName: metricSource.metricTables?.gauge || '', - }, - where: stripFieldClause( - searchQuery, - metricSource.resourceAttributesExpression, - field, - ), - whereLanguage: 'lucene', - select: '', - timestampValueExpression: metricSource.timestampValueExpression || '', - connection: metricSource.connection, - dateRange, - }); + const chartConfigs = useMemo(() => { + const build = (field: string): BuilderChartConfigWithDateRange => ({ + from: { + databaseName: metricSource.from.databaseName, + tableName: metricSource.metricTables?.gauge || '', + }, + where: stripFieldClause( + searchQuery, + metricSource.resourceAttributesExpression, + field, + ), + whereLanguage: 'lucene', + select: '', + timestampValueExpression: metricSource.timestampValueExpression || '', + connection: metricSource.connection, + dateRange, + }); + return { + 'k8s.pod.name': build('k8s.pod.name'), + 'k8s.deployment.name': build('k8s.deployment.name'), + 'k8s.node.name': build('k8s.node.name'), + 'k8s.namespace.name': build('k8s.namespace.name'), + 'k8s.cluster.name': build('k8s.cluster.name'), + }; + }, [searchQuery, dateRange, metricSource]); // Helper function to update search query const updateSearchQuery = ( @@ -225,7 +236,7 @@ export const KubernetesFilters: React.FC = ({ fieldName="k8s.pod.name" value={podName} onChange={value => updateSearchQuery('k8s.pod.name', value, setPodName)} - chartConfig={buildChartConfigForField('k8s.pod.name')} + chartConfig={chartConfigs['k8s.pod.name']} dataTestId="pod-filter-select" /> @@ -237,7 +248,7 @@ export const KubernetesFilters: React.FC = ({ onChange={value => updateSearchQuery('k8s.deployment.name', value, setDeploymentName) } - chartConfig={buildChartConfigForField('k8s.deployment.name')} + chartConfig={chartConfigs['k8s.deployment.name']} dataTestId="deployment-filter-select" /> @@ -249,7 +260,7 @@ export const KubernetesFilters: React.FC = ({ onChange={value => updateSearchQuery('k8s.node.name', value, setNodeName) } - chartConfig={buildChartConfigForField('k8s.node.name')} + chartConfig={chartConfigs['k8s.node.name']} dataTestId="node-filter-select" /> @@ -261,7 +272,7 @@ export const KubernetesFilters: React.FC = ({ onChange={value => updateSearchQuery('k8s.namespace.name', value, setNamespaceName) } - chartConfig={buildChartConfigForField('k8s.namespace.name')} + chartConfig={chartConfigs['k8s.namespace.name']} dataTestId="namespace-filter-select" /> @@ -273,7 +284,7 @@ export const KubernetesFilters: React.FC = ({ onChange={value => updateSearchQuery('k8s.cluster.name', value, setClusterName) } - chartConfig={buildChartConfigForField('k8s.cluster.name')} + chartConfig={chartConfigs['k8s.cluster.name']} dataTestId="cluster-filter-select" /> diff --git a/packages/app/src/components/__tests__/KubernetesFilters.test.ts b/packages/app/src/components/__tests__/KubernetesFilters.test.ts index bf54190bfc..f5a63c998b 100644 --- a/packages/app/src/components/__tests__/KubernetesFilters.test.ts +++ b/packages/app/src/components/__tests__/KubernetesFilters.test.ts @@ -40,4 +40,20 @@ describe('stripFieldClause', () => { 'ResourceAttributes.k8s.pod.uid:"b"', ); }); + + it('treats dots in the attribute as literal characters, not regex wildcards', () => { + // Unescaped, the dots would act as wildcards and wrongly strip this clause + // where the path separators are different characters. Escaping keeps them + // literal, so a non-dotted lookalike is left untouched. + const query = 'ResourceAttributesXk8sXpodXname:"a"'; + expect(stripFieldClause(query, resourceAttr, 'k8s.pod.name')).toBe(query); + }); + + it('does not throw when the resource attribute contains regex metacharacters', () => { + // An unescaped `(` would produce an "unterminated group" SyntaxError. + expect(() => + stripFieldClause('foo', 'attr(', 'k8s.pod.name'), + ).not.toThrow(); + expect(stripFieldClause('foo', 'attr(', 'k8s.pod.name')).toBe('foo'); + }); }); From 38fef0093b7db874e15e3ced174495a3c43b41d2 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Fri, 5 Jun 2026 14:52:07 -0400 Subject: [PATCH 03/11] feat(dashboards): opt-in 'link filters' toggle with lazy faceting Add a bidirectional-arrow toggle at the end of the dashboard and Kubernetes filter bars that opts into linked (faceted) filter values, off by default. When linked, each dropdown's values are narrowed by the other selections (and the K8s free-text search) and fetched lazily only when the dropdown is opened, bounding the cost of contingent value lookups that can't use per-key rollups. Search-page filters are untouched. --- .changeset/cascading-dashboard-filters.md | 18 ++--- packages/app/src/DashboardFilters.tsx | 72 ++++++++++++++++--- .../app/src/components/FilterLinkToggle.tsx | 44 ++++++++++++ .../app/src/components/KubernetesFilters.tsx | 72 ++++++++++++++----- .../VirtualMultiSelect/VirtualMultiSelect.tsx | 17 ++++- .../useDashboardFilterValues.test.tsx | 52 ++++++++++++++ .../src/hooks/useDashboardFilterValues.tsx | 18 ++++- 7 files changed, 254 insertions(+), 39 deletions(-) create mode 100644 packages/app/src/components/FilterLinkToggle.tsx diff --git a/.changeset/cascading-dashboard-filters.md b/.changeset/cascading-dashboard-filters.md index c2734ed68d..b48787f466 100644 --- a/.changeset/cascading-dashboard-filters.md +++ b/.changeset/cascading-dashboard-filters.md @@ -2,12 +2,14 @@ '@hyperdx/app': patch --- -feat(dashboards): cascading (faceted) filter values +feat(dashboards): opt-in linked (faceted) filter values -Dashboard filter dropdowns now narrow one another: selecting a value in one -filter constrains the options shown by the others to values that co-occur with -the current selection (e.g. picking a `cluster` limits the `namespace` dropdown -to namespaces in that cluster). This applies to both manually-created -dashboards and the bundled Kubernetes dashboard, where the dropdowns also honor -the free-text search. A filter never constrains its own options, so -multi-select within a single filter still works. +Dashboard and Kubernetes filter bars gain a "link filters" toggle (the +bidirectional-arrow button at the end of the bar). When enabled, each filter +dropdown only shows values that co-occur with the other current selections — +e.g. picking a `cluster` narrows the `namespace` dropdown to namespaces in that +cluster (the K8s bar also factors in the free-text search). A filter never +constrains its own options, so multi-select still works. It is off by default +and, when on, a dropdown's narrowed values are fetched lazily only when it is +opened, since contingent value lookups can't use the cheap per-key rollups and +are more expensive at scale. Search-page filters are unaffected. diff --git a/packages/app/src/DashboardFilters.tsx b/packages/app/src/DashboardFilters.tsx index a008dd7a54..0527c28bf4 100644 --- a/packages/app/src/DashboardFilters.tsx +++ b/packages/app/src/DashboardFilters.tsx @@ -1,8 +1,10 @@ +import { useCallback, useState } from 'react'; import { FilterState } from '@hyperdx/common-utils/dist/filters'; import { DashboardFilter } from '@hyperdx/common-utils/dist/types'; import { Group, Stack, Text, Tooltip } from '@mantine/core'; import { IconAlertTriangle, IconHelp, IconRefresh } from '@tabler/icons-react'; +import { FilterLinkToggle } from './components/FilterLinkToggle'; import { VirtualMultiSelect } from './components/VirtualMultiSelect/VirtualMultiSelect'; import { useDashboardFilterValues } from './hooks/useDashboardFilterValues'; @@ -13,6 +15,8 @@ interface DashboardFilterSelectProps { values?: string[]; isLoading?: boolean; isError?: boolean; + onDropdownOpen?: () => void; + onDropdownClose?: () => void; } const getAppliesToTooltip = (filter: DashboardFilter) => { @@ -28,6 +32,8 @@ const DashboardFilterSelect = ({ values, isLoading, isError, + onDropdownOpen, + onDropdownClose, }: DashboardFilterSelectProps) => { const sortedValues = values?.toSorted() || []; const tooltipText = getAppliesToTooltip(filter); @@ -63,11 +69,14 @@ const DashboardFilterSelect = ({ placeholder={value.length === 0 ? filter.name : undefined} values={value} data={sortedValues} - // Disable only while values are genuinely loading. A completed query - // that returned no rows (or failed) must stay interactive so the user - // can still clear/adjust the selection instead of being stuck. - disabled={isLoading} + // Surface loading as a dropdown hint rather than disabling the control: + // it must stay openable so lazy (link-mode) fetches can trigger on open, + // and a completed/empty/failed query must stay interactive so the user + // can still clear or adjust the selection. + loading={isLoading} onChange={onChange} + onDropdownOpen={onDropdownOpen} + onDropdownClose={onDropdownClose} data-testid={`dashboard-filter-select-${filter.name}`} /> @@ -88,6 +97,28 @@ const DashboardFilters = ({ filterValues, onSetFilterValue, }: DashboardFilterProps) => { + // "Link" mode (opt-in, off by default): each dropdown's values are narrowed by + // the others' selections. Off by default because contingent value lookups + // can't use the cheap per-key rollups and are far more expensive at scale. + const [linked, setLinked] = useState(false); + // In link mode, only fetch a filter's (constrained) values once its dropdown + // is open — bounds the extra scans to what the user actually looks at. + const [openFilterIds, setOpenFilterIds] = useState>( + () => new Set(), + ); + const setFilterOpen = useCallback((id: string, open: boolean) => { + setOpenFilterIds(prev => { + if (open === prev.has(id)) return prev; + const next = new Set(prev); + if (open) { + next.add(id); + } else { + next.delete(id); + } + return next; + }); + }, []); + const { data: filterValuesById, erroredFilterIds, @@ -95,7 +126,10 @@ const DashboardFilters = ({ } = useDashboardFilterValues({ filters, dateRange, - filterValues, + // Only narrow by sibling selections when linked. + filterValues: linked ? filterValues : {}, + // Lazy fetch (open dropdowns only) when linked; eager (all) when not. + activeFilterIds: linked ? openFilterIds : undefined, }); return ( @@ -106,12 +140,13 @@ const DashboardFilters = ({ const selectedValues = included ? Array.from(included).map(v => v.toString()) : []; - // Fall back to the hook-level fetching state only until this filter's - // query has produced an entry; once it has (even with empty values), - // honor its own loading flag so a finished query never stays disabled. + // In link mode a closed (never-opened) dropdown isn't fetched, so it + // must read as "not loading" to stay openable; otherwise fall back to + // the hook-level fetching state until this filter has produced an entry. + const isInactive = linked && !openFilterIds.has(filter.id); const isLoadingValues = queriedFilterValues ? queriedFilterValues.isLoading - : isFetching; + : !isInactive && isFetching; return ( onSetFilterValue(filter.expression, values)} values={queriedFilterValues?.values} value={selectedValues} + onDropdownOpen={ + linked ? () => setFilterOpen(filter.id, true) : undefined + } + onDropdownClose={ + linked ? () => setFilterOpen(filter.id, false) : undefined + } /> ); })} + {filters.length >= 2 && ( + + {/* Spacer to align the toggle with the inputs (filters have a label row above). */} + +   + + + + )} {isFetching && } ); diff --git a/packages/app/src/components/FilterLinkToggle.tsx b/packages/app/src/components/FilterLinkToggle.tsx new file mode 100644 index 0000000000..93ac6aa82e --- /dev/null +++ b/packages/app/src/components/FilterLinkToggle.tsx @@ -0,0 +1,44 @@ +import { ActionIcon, Tooltip } from '@mantine/core'; +import { IconArrowsLeftRight } from '@tabler/icons-react'; + +type FilterLinkToggleProps = { + linked: boolean; + onChange: (linked: boolean) => void; + 'data-testid'?: string; +}; + +/** + * Opt-in toggle that "links" a set of filter dropdowns so each one's selectable + * values are narrowed by the others' current selections (faceted / filter-aware + * values). Off by default because contingent value lookups can't be served from + * the cheap per-key rollups and are far more expensive at scale. + */ +export function FilterLinkToggle({ + linked, + onChange, + 'data-testid': dataTestId = 'filter-link-toggle', +}: FilterLinkToggleProps) { + return ( + + onChange(!linked)} + aria-label="Link filters" + aria-pressed={linked} + data-testid={dataTestId} + > + + + + ); +} diff --git a/packages/app/src/components/KubernetesFilters.tsx b/packages/app/src/components/KubernetesFilters.tsx index 0d4611c954..a7332deb8a 100644 --- a/packages/app/src/components/KubernetesFilters.tsx +++ b/packages/app/src/components/KubernetesFilters.tsx @@ -8,6 +8,7 @@ import { } from '@hyperdx/common-utils/dist/types'; import { Box, Group, Select } from '@mantine/core'; +import { FilterLinkToggle } from '@/components/FilterLinkToggle'; import SearchInputV2 from '@/components/SearchInput/SearchInputV2'; import { useGetKeyValues } from '@/hooks/useMetadata'; @@ -26,6 +27,8 @@ type FilterSelectProps = { onChange: (value: string | null) => void; chartConfig: BuilderChartConfigWithDateRange; dataTestId?: string; + /** Lazy (link) mode: only fetch this dropdown's values once it's opened. */ + lazy?: boolean; }; // Removes a single field's `resourceAttr.field:"..."` clause from a Lucene @@ -52,21 +55,32 @@ const FilterSelect: React.FC = ({ onChange, chartConfig, dataTestId, + lazy, }) => { - const { data, isLoading } = useGetKeyValues({ - chartConfig, - keys: [`${metricSource.resourceAttributesExpression}['${fieldName}']`], - disableRowLimit: true, - limit: 1000000, - }); + const [opened, setOpened] = useState(false); + const { data, isLoading } = useGetKeyValues( + { + chartConfig, + keys: [`${metricSource.resourceAttributesExpression}['${fieldName}']`], + disableRowLimit: true, + limit: 1000000, + }, + // Lazy mode only fetches once the dropdown has been opened. + { enabled: lazy ? opened : true }, + ); - const options = useMemo( - () => + const options = useMemo(() => { + const opts = data?.[0]?.value - .map(value => ({ value, label: value })) - .sort((a, b) => a.value.localeCompare(b.value)) || [], // Sort alphabetically for better search results - [data], - ); + .map(v => ({ value: v, label: v })) + .sort((a, b) => a.value.localeCompare(b.value)) || []; // Sort alphabetically for better search results + // Keep the current selection visible even before this dropdown's values + // have been fetched (lazy mode), where it wouldn't yet be in `data`. + if (value && !opts.some(o => o.value === value)) { + opts.unshift({ value, label: value }); + } + return opts; + }, [data, value]); return ( setOpened(true)} - onDropdownClose={() => setOpened(false)} searchable clearable allowDeselect @@ -121,6 +134,22 @@ export const KubernetesFilters: React.FC = ({ // value lookups can't use the cheap per-key rollups and cost far more at scale. const [linked, setLinked] = useState(false); + const resourceAttr = metricSource.resourceAttributesExpression; + const valueByField: Record = { + 'k8s.pod.name': podName, + 'k8s.deployment.name': deploymentName, + 'k8s.node.name': nodeName, + 'k8s.namespace.name': namespaceName, + 'k8s.cluster.name': clusterName, + }; + const setterByField: Record void> = { + 'k8s.pod.name': setPodName, + 'k8s.deployment.name': setDeploymentName, + 'k8s.node.name': setNodeName, + 'k8s.namespace.name': setNamespaceName, + 'k8s.cluster.name': setClusterName, + }; + const { control, setValue } = useForm({ defaultValues: { searchQuery: searchQuery, @@ -162,73 +191,100 @@ export const KubernetesFilters: React.FC = ({ // Initialize filter values from search query useEffect(() => { if (searchQuery) { - const resourceAttr = metricSource.resourceAttributesExpression; + for (const { field } of K8S_FILTER_FIELDS) { + setterByField[field]( + extractValueFromSearchQuery(searchQuery, resourceAttr, field), + ); + } + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [searchQuery, resourceAttr]); - setPodName( - extractValueFromSearchQuery(searchQuery, resourceAttr, 'k8s.pod.name'), - ); - setDeploymentName( - extractValueFromSearchQuery( - searchQuery, - resourceAttr, - 'k8s.deployment.name', - ), - ); - setNodeName( - extractValueFromSearchQuery(searchQuery, resourceAttr, 'k8s.node.name'), - ); - setNamespaceName( - extractValueFromSearchQuery( - searchQuery, - resourceAttr, - 'k8s.namespace.name', - ), - ); - setClusterName( - extractValueFromSearchQuery( - searchQuery, - resourceAttr, - 'k8s.cluster.name', - ), + const keys = useMemo( + () => K8S_FILTER_FIELDS.map(({ field }) => `${resourceAttr}['${field}']`), + [resourceAttr], + ); + + // Faceted (linked) lookups: build a per-field SQL predicate from the OTHER + // selected fields (exclude-self) so all five value lists are computed in a + // single `groupUniqArrayIf` scan. The free-text search is applied as a shared + // WHERE. When unlinked there are no constraints and every dropdown lists all + // values (still a single batched query). + const keyConditions = useMemo(() => { + if (!linked) return undefined; + return K8S_FILTER_FIELDS.map(({ field }) => { + const others: FilterState = {}; + for (const { field: otherField } of K8S_FILTER_FIELDS) { + const value = valueByField[otherField]; + if (otherField !== field && value) { + others[`${resourceAttr}['${otherField}']`] = { + included: new Set([value]), + excluded: new Set(), + }; + } + } + const predicates = filtersToQuery(others, { stringifyKeys: false }).map( + f => f.condition, ); - } - }, [searchQuery, metricSource.resourceAttributesExpression]); + return predicates.length + ? predicates.map(c => `(${c})`).join(' AND ') + : undefined; + }); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [ + linked, + resourceAttr, + podName, + deploymentName, + nodeName, + namespaceName, + clusterName, + ]); + + // Free-text portion of the search (everything except the structured field + // clauses), applied as a shared WHERE so the dropdowns honor it when linked. + const facetWhere = useMemo( + () => + linked + ? K8S_FILTER_FIELDS.reduce( + (query, { field }) => stripFieldClause(query, resourceAttr, field), + searchQuery, + ) + : '', + [linked, resourceAttr, searchQuery], + ); - // Build a chart config for fetching each field's selectable values. Memoized so - // the config objects keep a stable identity across re-renders unrelated to the - // filters (each is spread into useGetKeyValues' React Query key). - // Faceted filtering: constrain a field's values by every OTHER active K8s filter - // and the free-text search (i.e. the whole searchQuery except this field's own - // clause), so e.g. picking a cluster narrows the namespace list. - const chartConfigs = useMemo(() => { - const build = (field: string): BuilderChartConfigWithDateRange => ({ + const chartConfig: BuilderChartConfigWithDateRange = useMemo( + () => ({ from: { databaseName: metricSource.from.databaseName, tableName: metricSource.metricTables?.gauge || '', }, - // Only constrain by the other selections when linked; otherwise each - // dropdown lists all values independently (no `where`). - where: linked - ? stripFieldClause( - searchQuery, - metricSource.resourceAttributesExpression, - field, - ) - : '', + where: facetWhere, whereLanguage: 'lucene', select: '', timestampValueExpression: metricSource.timestampValueExpression || '', connection: metricSource.connection, dateRange, - }); - return { - 'k8s.pod.name': build('k8s.pod.name'), - 'k8s.deployment.name': build('k8s.deployment.name'), - 'k8s.node.name': build('k8s.node.name'), - 'k8s.namespace.name': build('k8s.namespace.name'), - 'k8s.cluster.name': build('k8s.cluster.name'), - }; - }, [searchQuery, dateRange, metricSource, linked]); + }), + [metricSource, facetWhere, dateRange], + ); + + const { data, isLoading } = useGetKeyValues({ + chartConfig, + keys, + keyConditions, + disableRowLimit: true, + limit: 1000000, + }); + + const valuesByKey = useMemo(() => { + const map = new Map(); + for (const entry of data ?? []) { + map.set(entry.key, entry.value as string[]); + } + return map; + }, [data]); // Helper function to update search query const updateSearchQuery = ( @@ -238,7 +294,6 @@ export const KubernetesFilters: React.FC = ({ ) => { setter(value); - const resourceAttr = metricSource.resourceAttributesExpression; const fullAttribute = `${resourceAttr}.${attribute}`; // Remove existing filter for this attribute if it exists @@ -254,68 +309,19 @@ export const KubernetesFilters: React.FC = ({ return ( - updateSearchQuery('k8s.pod.name', value, setPodName)} - chartConfig={chartConfigs['k8s.pod.name']} - lazy={linked} - dataTestId="pod-filter-select" - /> - - - updateSearchQuery('k8s.deployment.name', value, setDeploymentName) - } - chartConfig={chartConfigs['k8s.deployment.name']} - lazy={linked} - dataTestId="deployment-filter-select" - /> - - - updateSearchQuery('k8s.node.name', value, setNodeName) - } - chartConfig={chartConfigs['k8s.node.name']} - lazy={linked} - dataTestId="node-filter-select" - /> - - - updateSearchQuery('k8s.namespace.name', value, setNamespaceName) - } - chartConfig={chartConfigs['k8s.namespace.name']} - lazy={linked} - dataTestId="namespace-filter-select" - /> - - - updateSearchQuery('k8s.cluster.name', value, setClusterName) - } - chartConfig={chartConfigs['k8s.cluster.name']} - lazy={linked} - dataTestId="cluster-filter-select" - /> + {K8S_FILTER_FIELDS.map(({ field, placeholder, dataTestId }) => ( + + updateSearchQuery(field, value, setterByField[field]) + } + options={valuesByKey.get(`${resourceAttr}['${field}']`) ?? []} + loading={isLoading} + dataTestId={dataTestId} + /> + ))} void; - onDropdownOpen?: () => void; - onDropdownClose?: () => void; 'data-testid'?: string; }; @@ -38,8 +36,6 @@ export function VirtualMultiSelect({ placeholder, values, onChange, - onDropdownOpen, - onDropdownClose, 'data-testid': dataTestId, }: VirtualMultiSelectProps) { const viewportRef = useRef(null); @@ -59,14 +55,10 @@ export function VirtualMultiSelect({ }); const combobox = useCombobox({ - onDropdownClose: () => { - combobox.resetSelectedOption(); - onDropdownClose?.(); - }, + onDropdownClose: () => combobox.resetSelectedOption(), onDropdownOpen: () => { combobox.updateSelectedOptionIndex('active'); virtualizer.measure(); - onDropdownOpen?.(); }, }); diff --git a/packages/app/src/hooks/__tests__/useDashboardFilterValues.test.tsx b/packages/app/src/hooks/__tests__/useDashboardFilterValues.test.tsx index 4b849f7552..b9d32bdca9 100644 --- a/packages/app/src/hooks/__tests__/useDashboardFilterValues.test.tsx +++ b/packages/app/src/hooks/__tests__/useDashboardFilterValues.test.tsx @@ -538,7 +538,6 @@ describe('useDashboardFilterValues', () => { dateRange: mockDateRange, where: '', whereLanguage: 'sql', - filters: [], select: '', }, keys: ['environment'], @@ -1061,12 +1060,13 @@ describe('useDashboardFilterValues', () => { }); describe('faceted filtering (cascading filters)', () => { - // Returns the `filters` array passed to the getKeyValues call whose `keys` - // exactly match, or undefined if no such call was made. - const filtersForKeys = (keys: string[]) => - (mockMetadata.getKeyValues.mock.calls as any[]).find( - ([arg]) => JSON.stringify(arg.keys) === JSON.stringify(keys), - )?.[0]?.chartConfig?.filters; + // Returns the single arg object passed to the MOST RECENT getKeyValues call + // whose `keys` exactly match, or undefined if no such call was made. (Most + // recent matters when a rerender issues a fresh call for the same keys.) + const callForKeys = (keys: string[]) => + (mockMetadata.getKeyValues.mock.calls as any[]) + .filter(([arg]) => JSON.stringify(arg.keys) === JSON.stringify(keys)) + .at(-1)?.[0]; const envAndStatus: DashboardFilter[] = [ { @@ -1090,10 +1090,8 @@ describe('useDashboardFilterValues', () => { data: mockSources, isLoading: false, } as any); - // A prior test may have overridden the implementation (clearAllMocks - // resets call data but not implementations); restore the passthrough so - // each key group reaches getKeyValues with its own keys + chartConfig - // (including the faceting `filters`). + // Unconstrained groups still go through the optimizer; restore its + // passthrough (clearAllMocks resets call data but not implementations). jest .mocked(optimizeGetKeyValuesCalls) .mockImplementation(async ({ keys, chartConfig }) => [ @@ -1101,7 +1099,7 @@ describe('useDashboardFilterValues', () => { ]); }); - it('narrows sibling filters by the current selection and excludes self', async () => { + it('resolves every key in one faceted scan, constraining each by the others (exclude-self)', async () => { const { result } = renderHook( () => useDashboardFilterValues({ @@ -1119,20 +1117,17 @@ describe('useDashboardFilterValues', () => { await waitFor(() => expect(result.current.isFetching).toBe(false)); - // The two filters can no longer batch: each has a distinct constraint set. - expect(mockMetadata.getKeyValues).toHaveBeenCalledTimes(2); - - // `status` values are constrained by the `environment` selection. - expect(filtersForKeys(['status'])).toEqual([ - { type: 'sql', condition: "environment IN ('production')" }, + // A single scan for the whole source — not one query per filter. + expect(mockMetadata.getKeyValues).toHaveBeenCalledTimes(1); + // `status` is constrained by the environment selection; `environment` is + // NOT constrained by its own selection (exclude-self → undefined). + expect(callForKeys(['environment', 'status'])?.keyConditions).toEqual([ + undefined, + "(environment IN ('production'))", ]); - - // `environment` is NOT constrained by its own selection (exclude-self), - // so the user can still see and change among all environments. - expect(filtersForKeys(['environment'])).toEqual([]); }); - it('batches unselected same-source filters into one unconstrained query', async () => { + it('runs one unconstrained query when nothing is selected', async () => { const { result } = renderHook( () => useDashboardFilterValues({ @@ -1146,10 +1141,13 @@ describe('useDashboardFilterValues', () => { await waitFor(() => expect(result.current.isFetching).toBe(false)); expect(mockMetadata.getKeyValues).toHaveBeenCalledTimes(1); - expect(filtersForKeys(['environment', 'status'])).toEqual([]); + // No conditions → plain groupUniqArray (no keyConditions passed). + expect( + callForKeys(['environment', 'status'])?.keyConditions, + ).toBeUndefined(); }); - it('keeps unselected siblings batched while the selected filter splits off', async () => { + it('still uses a single scan for many filters when one is selected', async () => { const filters: DashboardFilter[] = [ ...envAndStatus, { @@ -1178,26 +1176,22 @@ describe('useDashboardFilterValues', () => { await waitFor(() => expect(result.current.isFetching).toBe(false)); - // Two queries: the selected filter alone, and the two unselected siblings - // batched together (they share the same constraint set). - expect(mockMetadata.getKeyValues).toHaveBeenCalledTimes(2); - expect(filtersForKeys(['status', 'log_level'])).toEqual([ - { type: 'sql', condition: "environment IN ('production')" }, + // One faceted scan regardless of how many filters/selections. + expect(mockMetadata.getKeyValues).toHaveBeenCalledTimes(1); + expect( + callForKeys(['environment', 'status', 'log_level'])?.keyConditions, + ).toEqual([ + undefined, + "(environment IN ('production'))", + "(environment IN ('production'))", ]); - expect(filtersForKeys(['environment'])).toEqual([]); }); it('does not apply a selection from one source to filters on another source', async () => { const filters: DashboardFilter[] = [ + ...envAndStatus, { - id: 'filter1', - type: 'QUERY_EXPRESSION', - name: 'Environment', - expression: 'environment', - source: 'logs-source', - }, - { - id: 'filter2', + id: 'filter3', type: 'QUERY_EXPRESSION', name: 'Service', expression: 'service.name', @@ -1222,13 +1216,16 @@ describe('useDashboardFilterValues', () => { await waitFor(() => expect(result.current.isFetching).toBe(false)); - // A logs-source selection must not leak into a traces-source value query - // (the column may not exist there). - expect(filtersForKeys(['service.name'])).toEqual([]); - expect(filtersForKeys(['environment'])).toEqual([]); + // The logs group is faceted (status narrowed by env)... + expect(callForKeys(['environment', 'status'])?.keyConditions).toEqual([ + undefined, + "(environment IN ('production'))", + ]); + // ...but the traces filter is never constrained by the logs selection. + expect(callForKeys(['service.name'])?.keyConditions).toBeUndefined(); }); - it('refetches with updated constraints when a selection changes', async () => { + it('refetches with updated conditions when a selection changes', async () => { const { result, rerender } = renderHook( ({ filterValues }) => useDashboardFilterValues({ @@ -1243,8 +1240,9 @@ describe('useDashboardFilterValues', () => { ); await waitFor(() => expect(result.current.isFetching).toBe(false)); - // Nothing selected → both filters batch into one query. - expect(mockMetadata.getKeyValues).toHaveBeenCalledTimes(1); + expect( + callForKeys(['environment', 'status'])?.keyConditions, + ).toBeUndefined(); rerender({ filterValues: { @@ -1257,62 +1255,10 @@ describe('useDashboardFilterValues', () => { await waitFor(() => expect(result.current.isFetching).toBe(false)); - // `status` is re-fetched with the new constraint. - expect(filtersForKeys(['status'])).toEqual([ - { type: 'sql', condition: "environment IN ('production')" }, - ]); - }); - - it('only fetches filters whose id is in activeFilterIds (lazy mode)', async () => { - const { result } = renderHook( - () => - useDashboardFilterValues({ - filters: envAndStatus, - dateRange: mockDateRange, - filterValues: {}, - // Only the `environment` dropdown has been opened. - activeFilterIds: new Set(['filter1']), - }), - { wrapper }, - ); - - await waitFor(() => expect(result.current.isFetching).toBe(false)); - - // The un-opened `status` filter is never queried. - expect(mockMetadata.getKeyValues).toHaveBeenCalledTimes(1); - expect(filtersForKeys(['environment'])).toEqual([]); - expect(filtersForKeys(['status'])).toBeUndefined(); - expect(result.current.data?.has('filter1')).toBe(true); - expect(result.current.data?.has('filter2')).toBe(false); - }); - - it('narrows an open filter by a selection from a filter that was never opened', async () => { - const { result } = renderHook( - () => - useDashboardFilterValues({ - filters: envAndStatus, - dateRange: mockDateRange, - // `environment` is selected, but only `status` is open. - filterValues: { - environment: { - included: new Set(['production']), - excluded: new Set(), - }, - }, - activeFilterIds: new Set(['filter2']), - }), - { wrapper }, - ); - - await waitFor(() => expect(result.current.isFetching).toBe(false)); - - // Constraints derive from ALL filters' selections, so `status` is narrowed - // by the environment selection even though that dropdown was never fetched. - expect(mockMetadata.getKeyValues).toHaveBeenCalledTimes(1); - expect(filtersForKeys(['status'])).toEqual([ - { type: 'sql', condition: "environment IN ('production')" }, + expect(callForKeys(['environment', 'status'])?.keyConditions).toEqual([ + undefined, + "(environment IN ('production'))", ]); - expect(filtersForKeys(['environment'])).toBeUndefined(); }); }); }); diff --git a/packages/app/src/hooks/useDashboardFilterValues.tsx b/packages/app/src/hooks/useDashboardFilterValues.tsx index 7962d83f8f..4447215f49 100644 --- a/packages/app/src/hooks/useDashboardFilterValues.tsx +++ b/packages/app/src/hooks/useDashboardFilterValues.tsx @@ -11,7 +11,6 @@ import { import { BuilderChartConfigWithDateRange, DashboardFilter, - Filter, isLogSource, isTraceSource, } from '@hyperdx/common-utils/dist/types'; @@ -45,37 +44,33 @@ const filterToKey = (filter: DashboardFilter): string => type EnrichedCall = GetKeyValueCall & { /** filterIds[i] = array of filter IDs whose values come from keys[i] */ filterIds: string[][]; + /** Per-key SQL predicate for faceted lookups, aligned with `keys`. */ + keyConditions?: (string | undefined)[]; }; function useOptimizedKeyValuesCalls({ filters, dateRange, filterValues, - activeFilterIds, }: { filters: DashboardFilter[]; dateRange: [Date, Date]; filterValues: FilterState; - /** - * When provided, only these filter IDs are fetched (lazy mode). Used to fetch - * a filter's constrained values only once its dropdown is opened. Constraints - * are still derived from ALL filters' selections, so an open filter is - * correctly narrowed by selections made in filters that were never opened. - */ - activeFilterIds?: Set; }) { const clickhouseClient = useClickhouseClient(); const metadata = useMetadataWithSettings(); const { data: sources, isLoading: isLoadingSources } = useSources(); // Faceted filtering: each filter's selectable values are narrowed by the - // CURRENT selections of its sibling filters. For every filter, build the - // constraint from the selections of the OTHER filters that target the same - // source + metric type (so the constrained columns are guaranteed to exist in - // the queried table), excluding the filter's own expression (otherwise a - // multi-select would collapse to only its already-selected values). - const constraintsByFilterId = useMemo(() => { - const byId = new Map(); + // CURRENT selections of its sibling filters. For every filter, build a SQL + // predicate from the selections of the OTHER filters that target the same + // source + metric type (so the constrained columns exist in the queried + // table), EXCLUDING the filter's own expression (otherwise a multi-select + // would collapse to only its already-selected values). Expressing it as a + // per-key predicate lets all of a source's filters resolve in one + // `groupUniqArrayIf` scan instead of one query per filter. + const conditionByFilterId = useMemo(() => { + const byId = new Map(); for (const filter of filters) { const prunedState: FilterState = {}; for (const sibling of filters) { @@ -98,52 +93,43 @@ function useOptimizedKeyValuesCalls({ prunedState[sibling.expression] = selection; } } + const predicates = filtersToQuery(prunedState, { + stringifyKeys: false, + }).map(f => f.condition); byId.set( filter.id, - filtersToQuery(prunedState, { stringifyKeys: false }), + predicates.length + ? predicates.map(c => `(${c})`).join(' AND ') + : undefined, ); } return byId; }, [filters, filterValues]); - // Group filters by (source, metricType, where, whereLanguage) AND their - // effective constraint signature, so each group can be tested for MV - // compatibility separately. Filters with an identical constraint set — in - // particular every currently-unselected filter of a source — stay batched in a - // single key-values query; each selected filter (which omits its own - // expression) splits into its own query. + // Group filters by (source, metricType, where, whereLanguage). Every filter in + // a group is resolved together: an unconstrained group goes through the MV + // optimizer (one batched, rollup-eligible query); a constrained group runs a + // single faceted `groupUniqArrayIf` scan carrying a per-key condition. const filtersByGroupKey = useMemo(() => { - const byGroupKey = new Map< - string, - { filters: DashboardFilter[]; constraints: Filter[] } - >(); + const byGroupKey = new Map(); for (const filter of filters) { - // Lazy mode: skip filters whose dropdown hasn't been opened. - if (activeFilterIds !== undefined && !activeFilterIds.has(filter.id)) { - continue; - } - const constraints = constraintsByFilterId.get(filter.id) ?? []; - const constraintsSig = constraints - .map(c => JSON.stringify(c)) - .sort() - .join('|'); - const key = `${filterToKey(filter)}::${constraintsSig}`; + const key = filterToKey(filter); const existing = byGroupKey.get(key); if (existing) { - existing.filters.push(filter); + existing.push(filter); } else { - byGroupKey.set(key, { filters: [filter], constraints }); + byGroupKey.set(key, [filter]); } } return byGroupKey; - }, [filters, constraintsByFilterId, activeFilterIds]); + }, [filters]); const results: UseQueryResult[] = useQueries({ queries: Array.from(filtersByGroupKey.values()) - .filter(({ filters: filtersInGroup }) => + .filter(filtersInGroup => sources?.some(s => s.id === filtersInGroup[0].source), ) - .map(({ filters: filtersInGroup, constraints }) => { + .map(filtersInGroup => { const representative = filtersInGroup[0]; const sourceId = representative.source; const metricType = representative.sourceMetricType; @@ -151,6 +137,10 @@ function useOptimizedKeyValuesCalls({ const whereLanguage = representative.whereLanguage ?? 'sql'; const source = sources!.find(s => s.id === sourceId)!; const keyExpressions = filtersInGroup.map(f => f.expression); + const keyConditions = filtersInGroup.map(f => + conditionByFilterId.get(f.id), + ); + const isFaceted = keyConditions.some(c => c != null); const tableName = getMetricTableName(source, metricType) ?? ''; const chartConfig: BuilderChartConfigWithDateRange = { @@ -175,12 +165,16 @@ function useOptimizedKeyValuesCalls({ source: source.id, where, whereLanguage, - // Sibling-selection constraints (faceted filtering); combined with the - // static `where` via AND inside renderChartConfig. - filters: constraints, select: '', }; + const filterIdsForKeys = (keys: string[]) => + keys.map(expression => + filtersInGroup + .filter(f => f.expression === expression) + .map(f => f.id), + ); + return { queryKey: [ 'dashboard-filters-key-value-calls', @@ -190,11 +184,25 @@ function useOptimizedKeyValuesCalls({ keyExpressions, where, whereLanguage, - constraints, + keyConditions, ], enabled: !isLoadingSources, staleTime: 1000 * 60 * 5, // Cache every 5 min - queryFn: async ({ signal }) => { + queryFn: async ({ signal }): Promise => { + // Constrained: resolve every key in one faceted scan + // (groupUniqArrayIf), since a per-key condition can't be split + // across single-key materialized views. + if (isFaceted) { + return [ + { + chartConfig, + keys: keyExpressions, + keyConditions, + filterIds: filterIdsForKeys(keyExpressions), + }, + ]; + } + // Unconstrained: let the MV optimizer batch / route to rollups. const calls = await optimizeGetKeyValuesCalls({ chartConfig, source, @@ -203,14 +211,9 @@ function useOptimizedKeyValuesCalls({ keys: keyExpressions, signal, }); - // Enrich each call with the filter IDs that correspond to each key expression return calls.map(call => ({ ...call, - filterIds: call.keys.map(expression => - filtersInGroup - .filter(f => f.expression === expression) - .map(f => f.id), - ), + filterIds: filterIdsForKeys(call.keys), })); }, }; @@ -228,13 +231,10 @@ export function useDashboardFilterValues({ filters, dateRange, filterValues = {}, - activeFilterIds, }: { filters: DashboardFilter[]; dateRange: [Date, Date]; filterValues?: FilterState; - /** Lazy mode: only fetch these filter IDs (e.g. dropdowns that are open). */ - activeFilterIds?: Set; }) { const metadata = useMetadataWithSettings(); const { @@ -245,7 +245,6 @@ export function useDashboardFilterValues({ filters, dateRange, filterValues, - activeFilterIds, }); const { data: sources, isLoading: isSourcesLoading } = useSources(); @@ -255,7 +254,7 @@ export function useDashboardFilterValues({ type TQueryData = { key: string; value: string[] }[]; const results: UseQueryResult[] = useQueries({ - queries: calls.map(({ chartConfig, keys }) => { + queries: calls.map(({ chartConfig, keys, keyConditions }) => { // Construct a query key prefix which will allow us to use placeholder data from the previous query for the same keys const queryKeyPrefix = [ 'dashboard-filter-key-values', @@ -266,7 +265,7 @@ export function useDashboardFilterValues({ const source = sourcesLookup.get(chartConfig.source); return { - queryKey: [...queryKeyPrefix, chartConfig], + queryKey: [...queryKeyPrefix, chartConfig, keyConditions], placeholderData: () => { // Use placeholder data from the most recently cached query with the same key prefix const cached = queryClient @@ -288,6 +287,7 @@ export function useDashboardFilterValues({ metadata.getKeyValues({ chartConfig, keys, + keyConditions, limit: 10000, disableRowLimit: true, signal, diff --git a/packages/app/src/hooks/useMetadata.tsx b/packages/app/src/hooks/useMetadata.tsx index 2f170d980f..7b31f15130 100644 --- a/packages/app/src/hooks/useMetadata.tsx +++ b/packages/app/src/hooks/useMetadata.tsx @@ -227,6 +227,7 @@ export function useMultipleGetKeyValues( { chartConfigs, keys, + keyConditions, limit, disableRowLimit, mode = 'exact', @@ -236,6 +237,8 @@ export function useMultipleGetKeyValues( | BuilderChartConfigWithDateRange | BuilderChartConfigWithDateRange[]; keys: string[]; + /** Per-key SQL predicates for faceted ('exact' mode) value lookups. */ + keyConditions?: (string | undefined)[]; limit?: number; disableRowLimit?: boolean; mode?: 'all' | 'exact'; @@ -261,6 +264,7 @@ export function useMultipleGetKeyValues( metadataMVsOverride, ...chartConfigsArr.map(cc => ({ ...cc })), ...keys, + keyConditions, disableRowLimit, maxKeys, ], @@ -306,6 +310,7 @@ export function useMultipleGetKeyValues( return metadata.getKeyValuesWithMVs({ chartConfig, keys: keys.slice(0, maxKeys), + keyConditions: keyConditions?.slice(0, maxKeys), limit, disableRowLimit, source, @@ -366,6 +371,7 @@ export function useGetKeyValues( { chartConfig, keys, + keyConditions, limit, disableRowLimit, mode, @@ -373,6 +379,8 @@ export function useGetKeyValues( }: { chartConfig?: BuilderChartConfigWithDateRange; keys: string[]; + /** Per-key SQL predicates for faceted value lookups (groupUniqArrayIf). */ + keyConditions?: (string | undefined)[]; limit?: number; disableRowLimit?: boolean; mode?: 'all' | 'exact'; @@ -384,6 +392,7 @@ export function useGetKeyValues( { chartConfigs: chartConfig ? [chartConfig] : [], keys, + keyConditions, limit, disableRowLimit, mode, diff --git a/packages/common-utils/src/__tests__/metadata.test.ts b/packages/common-utils/src/__tests__/metadata.test.ts index bbd40781ef..b6691fb515 100644 --- a/packages/common-utils/src/__tests__/metadata.test.ts +++ b/packages/common-utils/src/__tests__/metadata.test.ts @@ -485,6 +485,34 @@ describe('Metadata', () => { ); }); + it('uses groupUniqArrayIf for keys that have a condition (faceted lookup)', async () => { + const renderChartConfigSpy = jest.spyOn( + renderChartConfigModule, + 'renderChartConfig', + ); + + await metadata.getKeyValues({ + chartConfig: mockChartConfig, + keys: ['colA', 'colB'], + keyConditions: [undefined, "(colA IN ('x'))"], + limit: 10, + disableRowLimit: true, + source, + }); + + const actualConfig = renderChartConfigSpy.mock.calls.at(-1)![0]; + if (!isBuilderChartConfig(actualConfig)) + throw new Error('Expected builder config'); + // No condition → plain groupUniqArray; condition → groupUniqArrayIf so + // both facets resolve in a single scan. + expect(actualConfig.select).toContain( + 'groupUniqArray(10)(colA) AS param0', + ); + expect(actualConfig.select).toContain( + "groupUniqArrayIf(10)(colB, (colA IN ('x'))) AS param1", + ); + }); + it('should apply row limit by default when disableRowLimit is not specified', async () => { await metadata.getKeyValues({ chartConfig: mockChartConfig, diff --git a/packages/common-utils/src/core/metadata.ts b/packages/common-utils/src/core/metadata.ts index a80feb1fad..67d0cf8e2c 100644 --- a/packages/common-utils/src/core/metadata.ts +++ b/packages/common-utils/src/core/metadata.ts @@ -1579,6 +1579,7 @@ export class Metadata { async getKeyValues({ chartConfig, keys, + keyConditions, limit = 20, disableRowLimit = false, signal, @@ -1586,6 +1587,14 @@ export class Metadata { }: { chartConfig: BuilderChartConfigWithDateRange; keys: string[]; + /** + * Optional per-key SQL predicate, aligned with `keys`. When provided for a + * key, its values are gathered with `groupUniqArrayIf(key, condition)` so + * several "faceted" value lists (each constrained by a different condition) + * can be computed in a single table scan. Only applied when + * `disableRowLimit` is true (the path used by filter dropdowns). + */ + keyConditions?: (string | undefined)[]; limit?: number; disableRowLimit?: boolean; signal?: AbortSignal; @@ -1603,6 +1612,7 @@ export class Metadata { 'filters', ]), keys, + keyConditions, disableRowLimit, }; return this.cache.getOrFetch( @@ -1616,7 +1626,15 @@ export class Metadata { ? { ...chartConfig, select: keys - .map((k, i) => `groupUniqArray(${limit})(${k}) AS param${i}`) + .map((k, i) => { + const condition = keyConditions?.[i]; + // `groupUniqArrayIf` lets each key be constrained by its own + // predicate, so multiple faceted value lists are computed in a + // single scan instead of one query per key. + return condition + ? `groupUniqArrayIf(${limit})(${k}, ${condition}) AS param${i}` + : `groupUniqArray(${limit})(${k}) AS param${i}`; + }) .join(', '), } : await (async () => { @@ -1698,6 +1716,7 @@ export class Metadata { async getKeyValuesWithMVs({ chartConfig, keys, + keyConditions, source, limit = 20, disableRowLimit, @@ -1705,6 +1724,8 @@ export class Metadata { }: { chartConfig: BuilderChartConfigWithDateRange; keys: string[]; + /** Per-key SQL predicates for faceted value lookups; see getKeyValues. */ + keyConditions?: (string | undefined)[]; source: TSource | undefined; limit?: number; disableRowLimit?: boolean; @@ -1720,6 +1741,7 @@ export class Metadata { 'filters', ]), keys, + keyConditions, disableRowLimit, }; return this.cache.getOrFetch( @@ -1727,6 +1749,20 @@ export class Metadata { async () => { if (keys.length === 0) return []; + // Faceted lookups apply a different predicate per key, so they can't be + // split across single-key materialized views — run one direct scan. + if (keyConditions?.some(c => c != null)) { + return this.getKeyValues({ + chartConfig, + keys, + keyConditions, + limit, + disableRowLimit, + signal, + source, + }); + } + const defaultKeyValueCall = { chartConfig, keys }; const canHaveMVs = source && From 138bc12a9e3275213ec4a0beb8c850ced7e6a67f Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Thu, 11 Jun 2026 10:41:25 -0400 Subject: [PATCH 08/11] fix(dashboards): narrow Filter union when reading condition (tsc) filtersToQuery returns the Filter union, whose sql_ast member has no `condition`; use a 'condition' in f guard so the faceting predicate build type-checks. (A stale common-utils dist / nx cache masked this locally.) --- packages/app/src/components/KubernetesFilters.tsx | 8 +++++--- packages/app/src/hooks/useDashboardFilterValues.tsx | 4 +++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/app/src/components/KubernetesFilters.tsx b/packages/app/src/components/KubernetesFilters.tsx index 3ad75547bf..770035c515 100644 --- a/packages/app/src/components/KubernetesFilters.tsx +++ b/packages/app/src/components/KubernetesFilters.tsx @@ -223,9 +223,11 @@ export const KubernetesFilters: React.FC = ({ }; } } - const predicates = filtersToQuery(others, { stringifyKeys: false }).map( - f => f.condition, - ); + // filtersToQuery only emits `sql` filters (which carry `condition`); the + // `in` guard narrows away the `sql_ast` member of the Filter union. + const predicates = filtersToQuery(others, { + stringifyKeys: false, + }).flatMap(f => ('condition' in f ? [f.condition] : [])); return predicates.length ? predicates.map(c => `(${c})`).join(' AND ') : undefined; diff --git a/packages/app/src/hooks/useDashboardFilterValues.tsx b/packages/app/src/hooks/useDashboardFilterValues.tsx index 4447215f49..eb6c368b5c 100644 --- a/packages/app/src/hooks/useDashboardFilterValues.tsx +++ b/packages/app/src/hooks/useDashboardFilterValues.tsx @@ -95,7 +95,9 @@ function useOptimizedKeyValuesCalls({ } const predicates = filtersToQuery(prunedState, { stringifyKeys: false, - }).map(f => f.condition); + // filtersToQuery only emits `sql` filters (which carry `condition`); the + // `in` guard narrows away the `sql_ast` member of the Filter union. + }).flatMap(f => ('condition' in f ? [f.condition] : [])); byId.set( filter.id, predicates.length From 8d896bd1161e2a4ef4d205663e4db71be6d19133 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Thu, 11 Jun 2026 16:25:51 -0400 Subject: [PATCH 09/11] perf(dashboards): route faceted filter scans to a covering materialized view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback: the single-pass groupUniqArrayIf scan previously always hit the raw table when linked. Add optimizeFacetedKeyValuesConfig — when one MV's dimensions cover every filter column, EXPLAIN-validate the faceted query against it and use it (cheapest row estimate), else fall back to raw. Wired into the dashboard hook and getKeyValuesWithMVs. Keeps the single-query win and restores MV leverage at scale; off-by-default unchanged. --- .../useDashboardFilterValues.test.tsx | 53 ++++++++- .../src/hooks/useDashboardFilterValues.tsx | 17 ++- .../__tests__/materializedViews.test.ts | 110 ++++++++++++++++++ .../src/core/materializedViews.ts | 96 +++++++++++++++ packages/common-utils/src/core/metadata.ts | 18 ++- 5 files changed, 287 insertions(+), 7 deletions(-) diff --git a/packages/app/src/hooks/__tests__/useDashboardFilterValues.test.tsx b/packages/app/src/hooks/__tests__/useDashboardFilterValues.test.tsx index b9d32bdca9..0af87f8d1d 100644 --- a/packages/app/src/hooks/__tests__/useDashboardFilterValues.test.tsx +++ b/packages/app/src/hooks/__tests__/useDashboardFilterValues.test.tsx @@ -1,6 +1,9 @@ /* eslint-disable @typescript-eslint/no-unsafe-type-assertion */ import React from 'react'; -import { optimizeGetKeyValuesCalls } from '@hyperdx/common-utils/dist/core/materializedViews'; +import { + optimizeFacetedKeyValuesConfig, + optimizeGetKeyValuesCalls, +} from '@hyperdx/common-utils/dist/core/materializedViews'; import { Metadata } from '@hyperdx/common-utils/dist/core/metadata'; import { FilterState } from '@hyperdx/common-utils/dist/filters'; import { @@ -26,6 +29,10 @@ jest.mock('@hyperdx/common-utils/dist/core/materializedViews', () => ({ .mockImplementation(async ({ keys, chartConfig }) => [ { keys, chartConfig }, ]), + // Default: no covering MV → faceted query runs against the raw config. + optimizeFacetedKeyValuesConfig: jest + .fn() + .mockImplementation(async ({ chartConfig }) => chartConfig), })); describe('useDashboardFilterValues', () => { @@ -1097,6 +1104,10 @@ describe('useDashboardFilterValues', () => { .mockImplementation(async ({ keys, chartConfig }) => [ { keys, chartConfig }, ]); + // Default: faceted queries run against the raw config (no covering MV). + jest + .mocked(optimizeFacetedKeyValuesConfig) + .mockImplementation(async ({ chartConfig }) => chartConfig); }); it('resolves every key in one faceted scan, constraining each by the others (exclude-self)', async () => { @@ -1260,5 +1271,45 @@ describe('useDashboardFilterValues', () => { "(environment IN ('production'))", ]); }); + + it('runs the faceted scan against a covering materialized view when one is found', async () => { + // Simulate a covering MV: the resolver points the faceted query at the + // rollup table. + jest + .mocked(optimizeFacetedKeyValuesConfig) + .mockImplementation(async ({ chartConfig }) => ({ + ...chartConfig, + from: { databaseName: 'telemetry', tableName: 'logs_rollup_1m' }, + })); + + const { result } = renderHook( + () => + useDashboardFilterValues({ + filters: envAndStatus, + dateRange: mockDateRange, + filterValues: { + environment: { + included: new Set(['production']), + excluded: new Set(), + }, + }, + }), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + const call = callForKeys(['environment', 'status']); + // The single faceted scan targets the rollup, still carrying the per-key + // conditions. + expect(call?.chartConfig?.from).toEqual({ + databaseName: 'telemetry', + tableName: 'logs_rollup_1m', + }); + expect(call?.keyConditions).toEqual([ + undefined, + "(environment IN ('production'))", + ]); + }); }); }); diff --git a/packages/app/src/hooks/useDashboardFilterValues.tsx b/packages/app/src/hooks/useDashboardFilterValues.tsx index eb6c368b5c..93bd91198b 100644 --- a/packages/app/src/hooks/useDashboardFilterValues.tsx +++ b/packages/app/src/hooks/useDashboardFilterValues.tsx @@ -2,6 +2,7 @@ import { useMemo } from 'react'; import { pick } from 'lodash'; import { GetKeyValueCall, + optimizeFacetedKeyValuesConfig, optimizeGetKeyValuesCalls, } from '@hyperdx/common-utils/dist/core/materializedViews'; import { @@ -192,12 +193,22 @@ function useOptimizedKeyValuesCalls({ staleTime: 1000 * 60 * 5, // Cache every 5 min queryFn: async ({ signal }): Promise => { // Constrained: resolve every key in one faceted scan - // (groupUniqArrayIf), since a per-key condition can't be split - // across single-key materialized views. + // (groupUniqArrayIf). A per-key condition can't be split across + // single-key MVs, but it can run against one MV whose dimensions + // cover every filter column (else the raw table). if (isFaceted) { + const facetedConfig = await optimizeFacetedKeyValuesConfig({ + chartConfig, + keys: keyExpressions, + keyConditions, + source, + clickhouseClient, + metadata, + signal, + }); return [ { - chartConfig, + chartConfig: facetedConfig, keys: keyExpressions, keyConditions, filterIds: filterIdsForKeys(keyExpressions), diff --git a/packages/common-utils/src/clickhouse/__tests__/materializedViews.test.ts b/packages/common-utils/src/clickhouse/__tests__/materializedViews.test.ts index 361673e280..6a63bbe453 100644 --- a/packages/common-utils/src/clickhouse/__tests__/materializedViews.test.ts +++ b/packages/common-utils/src/clickhouse/__tests__/materializedViews.test.ts @@ -1,5 +1,6 @@ import { isUnsupportedCountFunction, + optimizeFacetedKeyValuesConfig, optimizeGetKeyValuesCalls, tryConvertConfigToMaterializedViewSelect, tryOptimizeConfigWithMaterializedView, @@ -2545,4 +2546,113 @@ describe('materializedViews', () => { expect(result[0].keys).toEqual(['environment', 'service']); }); }); + + describe('optimizeFacetedKeyValuesConfig', () => { + const mockClickHouseClient = { + testChartConfigValidity: jest.fn(), + } as unknown as jest.Mocked; + + const facetedChartConfig: ChartConfigWithOptDateRange = { + from: { databaseName: 'default', tableName: 'otel_spans' }, + select: '', + where: '', + connection: 'test-connection', + }; + + const facetedSource = { + kind: SourceKind.Log, + from: { databaseName: 'default', tableName: 'otel_spans' }, + materializedViews: [MV_CONFIG_METRIC_ROLLUP_1M], + } as TLogSource; + + beforeEach(() => { + jest.clearAllMocks(); + mockClickHouseClient.testChartConfigValidity.mockResolvedValue({ + isValid: true, + rowEstimate: 1000, + error: undefined, + }); + }); + + it('routes to a covering MV and EXPLAINs the faceted select', async () => { + const result = await optimizeFacetedKeyValuesConfig({ + chartConfig: facetedChartConfig, + keys: ['ServiceName', 'StatusCode'], + keyConditions: [undefined, "(ServiceName IN ('x'))"], + source: facetedSource, + clickhouseClient: mockClickHouseClient, + metadata, + }); + + // The faceted query runs against the rollup, using its timestamp column. + expect(result.from).toEqual({ + databaseName: 'default', + tableName: 'metrics_rollup_1m', + }); + expect(result.timestampValueExpression).toBe('Timestamp'); + // Validation used groupUniqArrayIf only for the constrained key. + expect(mockClickHouseClient.testChartConfigValidity).toHaveBeenCalledWith( + expect.objectContaining({ + config: expect.objectContaining({ + from: { databaseName: 'default', tableName: 'metrics_rollup_1m' }, + select: + "groupUniqArray(1)(ServiceName) AS param0, groupUniqArrayIf(1)(StatusCode, (ServiceName IN ('x'))) AS param1", + }), + }), + ); + }); + + it('falls back to the raw table when a key is not an MV dimension', async () => { + const result = await optimizeFacetedKeyValuesConfig({ + chartConfig: facetedChartConfig, + keys: ['ServiceName', 'NotADimension'], + keyConditions: [undefined, "(ServiceName IN ('x'))"], + source: facetedSource, + clickhouseClient: mockClickHouseClient, + metadata, + }); + + expect(result).toBe(facetedChartConfig); + expect( + mockClickHouseClient.testChartConfigValidity, + ).not.toHaveBeenCalled(); + }); + + it('falls back to the raw table when the MV EXPLAIN is invalid', async () => { + mockClickHouseClient.testChartConfigValidity.mockResolvedValue({ + isValid: false, + error: '', + }); + + const result = await optimizeFacetedKeyValuesConfig({ + chartConfig: facetedChartConfig, + keys: ['ServiceName', 'StatusCode'], + keyConditions: [undefined, "(ServiceName IN ('x'))"], + source: facetedSource, + clickhouseClient: mockClickHouseClient, + metadata, + }); + + expect(result).toBe(facetedChartConfig); + }); + + it('returns the raw table for a source without materialized views', async () => { + const result = await optimizeFacetedKeyValuesConfig({ + chartConfig: facetedChartConfig, + keys: ['ServiceName'], + keyConditions: [undefined], + source: { + kind: SourceKind.Log, + from: { databaseName: 'default', tableName: 'otel_spans' }, + } as TLogSource, + clickhouseClient: mockClickHouseClient, + metadata, + }); + + expect(result).toBe(facetedChartConfig); + expect( + mockClickHouseClient.testChartConfigValidity, + ).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/common-utils/src/core/materializedViews.ts b/packages/common-utils/src/core/materializedViews.ts index 810dd79063..c62ad221c2 100644 --- a/packages/common-utils/src/core/materializedViews.ts +++ b/packages/common-utils/src/core/materializedViews.ts @@ -899,3 +899,99 @@ export async function optimizeGetKeyValuesCalls< return calls; } + +/** + * Resolve which table a *faceted* key-values lookup should scan. A faceted + * lookup applies a different predicate per key (`groupUniqArrayIf`), so it can't + * be split across single-key rollups like `optimizeGetKeyValuesCalls` — but it + * CAN run against one materialized view whose dimension columns cover every + * filter column. Returns an MV-pointed chartConfig when a covering MV validates + * via EXPLAIN (cheapest row estimate wins), otherwise the original (raw) config. + */ +export async function optimizeFacetedKeyValuesConfig< + C extends BuilderChartConfigWithOptDateRange, +>({ + chartConfig, + keys, + keyConditions, + source, + clickhouseClient, + metadata, + signal, +}: { + chartConfig: C; + keys: string[]; + /** Per-key SQL predicate aligned with `keys` (undefined = unconstrained). */ + keyConditions: (string | undefined)[]; + source: TSource | undefined; + clickhouseClient: BaseClickhouseClient; + metadata: Metadata; + signal?: AbortSignal; +}): Promise { + const mvs = + source && (isTraceSource(source) || isLogSource(source)) + ? (source.materializedViews ?? []) + : []; + if (mvs.length === 0) return chartConfig; + + // A covering MV must expose every filter column as a dimension. The per-key + // conditions only reference other keys, so requiring all `keys` covers them + // too; anything else (e.g. the static `where`) is caught by the EXPLAIN below. + const coveringMvs = mvs.filter(mv => { + const intervalsInDateRange = chartConfig.dateRange + ? countIntervalsInDateRange(chartConfig.dateRange, mv.minGranularity) + : Infinity; + if ( + !mvConfigSupportsDateRange(mv, chartConfig) || + intervalsInDateRange < 3 + ) { + return false; + } + const dimensionColumns = splitAndTrimWithBracket(mv.dimensionColumns); + return keys.every(k => dimensionColumns.includes(k)); + }); + if (coveringMvs.length === 0) return chartConfig; + + // Build + EXPLAIN-validate a faceted config against each covering MV. The + // EXPLAIN only needs the same columns, so the limit is irrelevant here. + const explainResults = await Promise.all( + coveringMvs.map(async mv => { + const config = { + ...structuredClone(chartConfig), + timestampValueExpression: mv.timestampColumn, + from: { databaseName: mv.databaseName, tableName: mv.tableName }, + select: keys + .map((k, i) => { + const condition = keyConditions[i]; + return condition + ? `groupUniqArrayIf(1)(${k}, ${condition}) AS param${i}` + : `groupUniqArray(1)(${k}) AS param${i}`; + }) + .join(', '), + }; + const { isValid, rowEstimate = Number.POSITIVE_INFINITY } = + await clickhouseClient.testChartConfigValidity({ + config, + metadata, + opts: { abort_signal: signal }, + querySettings: source?.querySettings, + }); + return { mv, isValid, rowEstimate }; + }), + ); + + const best = explainResults + .filter(r => r.isValid) + .sort((a, b) => a.rowEstimate - b.rowEstimate)[0]; + if (!best) return chartConfig; + + const optimizedConfig: C = { + ...structuredClone(chartConfig), + timestampValueExpression: best.mv.timestampColumn, + from: { + databaseName: best.mv.databaseName, + tableName: best.mv.tableName, + }, + }; + return optimizedConfig; +} diff --git a/packages/common-utils/src/core/metadata.ts b/packages/common-utils/src/core/metadata.ts index 67d0cf8e2c..370ab5829d 100644 --- a/packages/common-utils/src/core/metadata.ts +++ b/packages/common-utils/src/core/metadata.ts @@ -23,6 +23,7 @@ import { isLogSource, isTraceSource, SourceKind } from '@/types'; import { ClickHouseVersion, parseClickHouseVersion } from './clickhouseVersion'; import { + optimizeFacetedKeyValuesConfig, optimizeGetKeyValuesCalls, renderStartOfBucketExpr, } from './materializedViews'; @@ -1750,12 +1751,23 @@ export class Metadata { if (keys.length === 0) return []; // Faceted lookups apply a different predicate per key, so they can't be - // split across single-key materialized views — run one direct scan. - if (keyConditions?.some(c => c != null)) { - return this.getKeyValues({ + // split across single-key materialized views — but they can run as one + // scan against a materialized view whose dimensions cover every filter + // column (else fall back to the raw table). + if (keyConditions && keyConditions.some(c => c != null)) { + const facetedConfig = await optimizeFacetedKeyValuesConfig({ chartConfig, keys, keyConditions, + source, + clickhouseClient: this.clickhouseClient, + metadata: this, + signal, + }); + return this.getKeyValues({ + chartConfig: facetedConfig, + keys, + keyConditions, limit, disableRowLimit, signal, From 99f55190628600728c2cd17d7eeae959491104bb Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Wed, 29 Jul 2026 13:20:49 -0400 Subject: [PATCH 10/11] fix(dashboards): render faceting predicates like the keys they constrain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getKeyValues renders each key through renderMetadataKeyExpression before putting it in the SELECT, so a JSON attribute is aggregated as a typed subcolumn rather than the bracket form the caller wrote. The faceting conditions were opaque SQL strings built app-side from the raw expressions, which meant a linked filter on a JSON source emitted groupUniqArrayIf(Attributes.`ns`.:String, (Attributes['cluster'] IN (...))) — a predicate naming a different expression than the aggregate wrapping it. Pass the selections down as FilterState instead of SQL and build the predicate inside getKeyValues, where the rendered keys are available. Both halves now come out rendered. The MV EXPLAIN probe in optimizeFacetedKeyValuesConfig gets the same treatment so it validates the SQL that will actually run; its dimensionColumns coverage check stays on the raw keys, since that config is authored by hand in the source form. Map columns and bare identifiers render to themselves, so the default OTel schema and the Kubernetes filter bar are unaffected. Adds filterStateToPredicate and serializeFilterState to common-utils. The latter is required because react-query hashes keys with JSON.stringify, which flattens a Set to {} — a raw FilterState in a query key would collide every distinct selection onto one cache entry. --- .../app/src/components/KubernetesFilters.tsx | 21 ++-- .../useDashboardFilterValues.test.tsx | 70 +++++++++-- .../src/hooks/useDashboardFilterValues.tsx | 59 +++++---- packages/app/src/hooks/useMetadata.tsx | 16 ++- .../src/__tests__/filters.test.ts | 116 ++++++++++++++++++ .../src/__tests__/metadata.test.ts | 53 +++++++- .../__tests__/materializedViews.test.ts | 21 +++- .../src/core/materializedViews.ts | 31 ++++- packages/common-utils/src/core/metadata.ts | 92 +++++++++++--- packages/common-utils/src/filters.ts | 57 +++++++++ 10 files changed, 452 insertions(+), 84 deletions(-) diff --git a/packages/app/src/components/KubernetesFilters.tsx b/packages/app/src/components/KubernetesFilters.tsx index 770035c515..7a5c003941 100644 --- a/packages/app/src/components/KubernetesFilters.tsx +++ b/packages/app/src/components/KubernetesFilters.tsx @@ -2,10 +2,7 @@ import React, { useEffect, useMemo, useRef, useState } from 'react'; import { escapeRegExp } from 'lodash'; import { useForm, useWatch } from 'react-hook-form'; import { tcFromSource } from '@hyperdx/common-utils/dist/core/metadata'; -import { - FilterState, - filtersToQuery, -} from '@hyperdx/common-utils/dist/filters'; +import { FilterState } from '@hyperdx/common-utils/dist/filters'; import { BuilderChartConfigWithDateRange, TMetricSource, @@ -205,15 +202,19 @@ export const KubernetesFilters: React.FC = ({ [resourceAttr], ); - // Faceted (linked) lookups: build a per-field SQL predicate from the OTHER + // Faceted (linked) lookups: build a per-field constraint from the OTHER // selected fields (exclude-self) so all five value lists are computed in a // single `groupUniqArrayIf` scan. The free-text search is applied as a shared // WHERE. When unlinked there are no constraints and every dropdown lists all // values (still a single batched query). + // + // Kept as FilterState rather than SQL so getKeyValues can render the keys the + // same way it renders them in the SELECT. const keyConditions = useMemo(() => { if (!linked) return undefined; return K8S_FILTER_FIELDS.map(({ field }) => { const others: FilterState = {}; + let hasSelection = false; for (const { field: otherField } of K8S_FILTER_FIELDS) { const value = valueByField[otherField]; if (otherField !== field && value) { @@ -221,16 +222,10 @@ export const KubernetesFilters: React.FC = ({ included: new Set([value]), excluded: new Set(), }; + hasSelection = true; } } - // filtersToQuery only emits `sql` filters (which carry `condition`); the - // `in` guard narrows away the `sql_ast` member of the Filter union. - const predicates = filtersToQuery(others, { - stringifyKeys: false, - }).flatMap(f => ('condition' in f ? [f.condition] : [])); - return predicates.length - ? predicates.map(c => `(${c})`).join(' AND ') - : undefined; + return hasSelection ? others : undefined; }); // eslint-disable-next-line react-hooks/exhaustive-deps }, [ diff --git a/packages/app/src/hooks/__tests__/useDashboardFilterValues.test.tsx b/packages/app/src/hooks/__tests__/useDashboardFilterValues.test.tsx index a027ff1181..9de069de5d 100644 --- a/packages/app/src/hooks/__tests__/useDashboardFilterValues.test.tsx +++ b/packages/app/src/hooks/__tests__/useDashboardFilterValues.test.tsx @@ -1109,6 +1109,16 @@ describe('useDashboardFilterValues', () => { .mockImplementation(async ({ chartConfig }) => chartConfig); }); + // What a filter narrowed by an `environment: production` selection should + // receive. The hook passes the selection down as state, not as SQL, so + // getKeyValues can render its keys the same way it renders the SELECT. + const envProductionConstraint = { + environment: { + included: new Set(['production']), + excluded: new Set(), + }, + }; + it('resolves every key in one faceted scan, constraining each by the others (exclude-self)', async () => { const { result } = renderHook( () => @@ -1133,7 +1143,7 @@ describe('useDashboardFilterValues', () => { // NOT constrained by its own selection (exclude-self → undefined). expect(callForKeys(['environment', 'status'])?.keyConditions).toEqual([ undefined, - "(environment IN ('production'))", + envProductionConstraint, ]); }); @@ -1190,11 +1200,7 @@ describe('useDashboardFilterValues', () => { expect(mockMetadata.getKeyValues).toHaveBeenCalledTimes(1); expect( callForKeys(['environment', 'status', 'log_level'])?.keyConditions, - ).toEqual([ - undefined, - "(environment IN ('production'))", - "(environment IN ('production'))", - ]); + ).toEqual([undefined, envProductionConstraint, envProductionConstraint]); }); it('does not apply a selection from one source to filters on another source', async () => { @@ -1229,12 +1235,55 @@ describe('useDashboardFilterValues', () => { // The logs group is faceted (status narrowed by env)... expect(callForKeys(['environment', 'status'])?.keyConditions).toEqual([ undefined, - "(environment IN ('production'))", + envProductionConstraint, ]); // ...but the traces filter is never constrained by the logs selection. expect(callForKeys(['service.name'])?.keyConditions).toBeUndefined(); }); + it('does not apply a selection across metric types of the same source', async () => { + // Same source, different metric type → different physical table, so the + // constrained column need not exist there. + const filters: DashboardFilter[] = [ + { + id: 'gauge-filter', + type: 'QUERY_EXPRESSION', + name: 'Gauge Metric', + expression: 'MetricName', + source: 'metric-source', + sourceMetricType: MetricsDataType.Gauge, + }, + { + id: 'sum-filter', + type: 'QUERY_EXPRESSION', + name: 'Sum Metric', + expression: 'SumMetricName', + source: 'metric-source', + sourceMetricType: MetricsDataType.Sum, + }, + ]; + + const { result } = renderHook( + () => + useDashboardFilterValues({ + filters, + dateRange: mockDateRange, + filterValues: { + MetricName: { + included: new Set(['CPU_Usage']), + excluded: new Set(), + }, + }, + }), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + expect(callForKeys(['MetricName'])?.keyConditions).toBeUndefined(); + expect(callForKeys(['SumMetricName'])?.keyConditions).toBeUndefined(); + }); + it('refetches with updated conditions when a selection changes', async () => { const { result, rerender } = renderHook( ({ filterValues }) => @@ -1267,7 +1316,7 @@ describe('useDashboardFilterValues', () => { expect(callForKeys(['environment', 'status'])?.keyConditions).toEqual([ undefined, - "(environment IN ('production'))", + envProductionConstraint, ]); }); @@ -1305,10 +1354,7 @@ describe('useDashboardFilterValues', () => { databaseName: 'telemetry', tableName: 'logs_rollup_1m', }); - expect(call?.keyConditions).toEqual([ - undefined, - "(environment IN ('production'))", - ]); + expect(call?.keyConditions).toEqual([undefined, envProductionConstraint]); }); }); }); diff --git a/packages/app/src/hooks/useDashboardFilterValues.tsx b/packages/app/src/hooks/useDashboardFilterValues.tsx index 462d00aa9a..1b25ff518c 100644 --- a/packages/app/src/hooks/useDashboardFilterValues.tsx +++ b/packages/app/src/hooks/useDashboardFilterValues.tsx @@ -7,7 +7,7 @@ import { } from '@hyperdx/common-utils/dist/core/materializedViews'; import { FilterState, - filtersToQuery, + serializeFilterState, } from '@hyperdx/common-utils/dist/filters'; import { BuilderChartConfigWithDateRange, @@ -45,8 +45,8 @@ const filterToKey = (filter: DashboardFilter): string => type EnrichedCall = GetKeyValueCall & { /** filterIds[i] = array of filter IDs whose values come from keys[i] */ filterIds: string[][]; - /** Per-key SQL predicate for faceted lookups, aligned with `keys`. */ - keyConditions?: (string | undefined)[]; + /** Per-key constraint for faceted lookups, aligned with `keys`. */ + keyConditions?: (FilterState | undefined)[]; }; function useOptimizedKeyValuesCalls({ @@ -63,17 +63,22 @@ function useOptimizedKeyValuesCalls({ const { data: sources, isLoading: isLoadingSources } = useSources(); // Faceted filtering: each filter's selectable values are narrowed by the - // CURRENT selections of its sibling filters. For every filter, build a SQL - // predicate from the selections of the OTHER filters that target the same - // source + metric type (so the constrained columns exist in the queried - // table), EXCLUDING the filter's own expression (otherwise a multi-select - // would collapse to only its already-selected values). Expressing it as a - // per-key predicate lets all of a source's filters resolve in one - // `groupUniqArrayIf` scan instead of one query per filter. - const conditionByFilterId = useMemo(() => { - const byId = new Map(); + // CURRENT selections of its sibling filters. For every filter, collect the + // selections of the OTHER filters that target the same source + metric type + // (so the constrained columns exist in the queried table), EXCLUDING the + // filter's own expression (otherwise a multi-select would collapse to only + // its already-selected values). Passing this down as a per-key constraint + // lets all of a source's filters resolve in one `groupUniqArrayIf` scan + // instead of one query per filter. + // + // The constraint stays a FilterState rather than SQL: getKeyValues renders + // it against the same key expressions it puts in the SELECT, which raw SQL + // built here could not line up with on JSON columns. + const constraintByFilterId = useMemo(() => { + const byId = new Map(); for (const filter of filters) { const prunedState: FilterState = {}; + let hasSelection = false; for (const sibling of filters) { if ( sibling.source !== filter.source || @@ -92,19 +97,10 @@ function useOptimizedKeyValuesCalls({ selection.range != null) ) { prunedState[sibling.expression] = selection; + hasSelection = true; } } - const predicates = filtersToQuery(prunedState, { - stringifyKeys: false, - // filtersToQuery only emits `sql` filters (which carry `condition`); the - // `in` guard narrows away the `sql_ast` member of the Filter union. - }).flatMap(f => ('condition' in f ? [f.condition] : [])); - byId.set( - filter.id, - predicates.length - ? predicates.map(c => `(${c})`).join(' AND ') - : undefined, - ); + byId.set(filter.id, hasSelection ? prunedState : undefined); } return byId; }, [filters, filterValues]); @@ -141,9 +137,14 @@ function useOptimizedKeyValuesCalls({ const source = sources!.find(s => s.id === sourceId)!; const keyExpressions = filtersInGroup.map(f => f.expression); const keyConditions = filtersInGroup.map(f => - conditionByFilterId.get(f.id), + constraintByFilterId.get(f.id), ); const isFaceted = keyConditions.some(c => c != null); + // Sets don't survive react-query's JSON.stringify key hashing, so the + // key carries a serialized projection instead of the raw state. + const keyConditionsSignature = keyConditions.map( + c => c && serializeFilterState(c), + ); const tableName = getMetricTableName(source, metricType) ?? ''; const chartConfig: BuilderChartConfigWithDateRange = { @@ -187,7 +188,7 @@ function useOptimizedKeyValuesCalls({ keyExpressions, where, whereLanguage, - keyConditions, + keyConditionsSignature, ], enabled: !isLoadingSources, staleTime: 1000 * 60 * 5, // Cache every 5 min @@ -278,7 +279,13 @@ export function useDashboardFilterValues({ const source = sourcesLookup.get(chartConfig.source); return { - queryKey: [...queryKeyPrefix, chartConfig, keyConditions], + queryKey: [ + ...queryKeyPrefix, + chartConfig, + // Serialized: react-query hashes keys with JSON.stringify, which + // would flatten every distinct Set selection to `{}`. + keyConditions?.map(c => c && serializeFilterState(c)), + ], placeholderData: () => { // Use placeholder data from the most recently cached query with the same key prefix const cached = queryClient diff --git a/packages/app/src/hooks/useMetadata.tsx b/packages/app/src/hooks/useMetadata.tsx index db6242719b..09fe642f66 100644 --- a/packages/app/src/hooks/useMetadata.tsx +++ b/packages/app/src/hooks/useMetadata.tsx @@ -11,6 +11,10 @@ import { TableConnection, TableMetadata, } from '@hyperdx/common-utils/dist/core/metadata'; +import { + FilterState, + serializeFilterState, +} from '@hyperdx/common-utils/dist/filters'; import { BuilderChartConfigWithDateRange, isLogSource, @@ -341,8 +345,8 @@ export function useMultipleGetKeyValues( | BuilderChartConfigWithDateRange | BuilderChartConfigWithDateRange[]; keys: string[]; - /** Per-key SQL predicates for faceted ('exact' mode) value lookups. */ - keyConditions?: (string | undefined)[]; + /** Per-key constraints for faceted ('exact' mode) value lookups. */ + keyConditions?: (FilterState | undefined)[]; limit?: number; disableRowLimit?: boolean; mode?: 'all' | 'exact'; @@ -368,7 +372,9 @@ export function useMultipleGetKeyValues( metadataMVsOverride, ...chartConfigsArr.map(cc => ({ ...cc })), ...keys, - keyConditions, + // Serialized: react-query hashes keys with JSON.stringify, which would + // flatten every distinct Set selection to `{}`. + keyConditions?.map(c => c && serializeFilterState(c)), disableRowLimit, maxKeys, ], @@ -484,8 +490,8 @@ export function useGetKeyValues( }: { chartConfig?: BuilderChartConfigWithDateRange; keys: string[]; - /** Per-key SQL predicates for faceted value lookups (groupUniqArrayIf). */ - keyConditions?: (string | undefined)[]; + /** Per-key constraints for faceted value lookups (groupUniqArrayIf). */ + keyConditions?: (FilterState | undefined)[]; limit?: number; disableRowLimit?: boolean; mode?: 'all' | 'exact'; diff --git a/packages/common-utils/src/__tests__/filters.test.ts b/packages/common-utils/src/__tests__/filters.test.ts index 702aa2e570..595c8f624e 100644 --- a/packages/common-utils/src/__tests__/filters.test.ts +++ b/packages/common-utils/src/__tests__/filters.test.ts @@ -1,7 +1,10 @@ import { + FilterState, + filterStateToPredicate, filtersToQuery, isRenderablePinnedFilter, parseQuery, + serializeFilterState, validateDashboardFilterQueries, validateSavedFilterValues, validateSavedQuery, @@ -357,6 +360,119 @@ describe('filters', () => { }); }); + describe('filterStateToPredicate', () => { + const identity = (k: string) => k; + + it('returns undefined when nothing is selected', () => { + expect(filterStateToPredicate({}, identity)).toBeUndefined(); + expect( + filterStateToPredicate( + { colA: { included: new Set(), excluded: new Set() } }, + identity, + ), + ).toBeUndefined(); + }); + + it('wraps a single condition in parentheses', () => { + expect( + filterStateToPredicate( + { colA: { included: new Set(['x']), excluded: new Set() } }, + identity, + ), + ).toBe("(colA IN ('x'))"); + }); + + it('AND-joins conditions across keys and across include/exclude', () => { + const predicate = filterStateToPredicate( + { + colA: { included: new Set(['x']), excluded: new Set(['y']) }, + colB: { included: new Set(['z']), excluded: new Set() }, + }, + identity, + ); + expect(predicate).toBe( + "(colA IN ('x')) AND (colA NOT IN ('y')) AND (colB IN ('z'))", + ); + }); + + it('addresses each key through renderKey', () => { + // The whole point of the callback: a JSON column is aggregated as a + // typed subcolumn, so the predicate has to name it the same way. + expect( + filterStateToPredicate( + { + "Attributes['cluster']": { + included: new Set(['prod']), + excluded: new Set(), + }, + }, + () => '`Attributes`.`cluster`.:String', + ), + ).toBe("(`Attributes`.`cluster`.:String IN ('prod'))"); + }); + }); + + describe('serializeFilterState', () => { + it('distinguishes different selections', () => { + const a: FilterState = { + colA: { included: new Set(['x']), excluded: new Set() }, + }; + const b: FilterState = { + colA: { included: new Set(['y']), excluded: new Set() }, + }; + expect(serializeFilterState(a)).not.toBe(serializeFilterState(b)); + // JSON.stringify alone would flatten both Sets to {} and collide, which + // is exactly what this helper exists to prevent in react-query keys. + expect(JSON.stringify(a)).toBe(JSON.stringify(b)); + }); + + it('distinguishes an included value from an excluded one', () => { + expect( + serializeFilterState({ + colA: { included: new Set(['x']), excluded: new Set() }, + }), + ).not.toBe( + serializeFilterState({ + colA: { included: new Set(), excluded: new Set(['x']) }, + }), + ); + }); + + it('is stable across key and member insertion order', () => { + expect( + serializeFilterState({ + colA: { included: new Set(['x', 'y']), excluded: new Set() }, + colB: { included: new Set(['z']), excluded: new Set() }, + }), + ).toBe( + serializeFilterState({ + colB: { included: new Set(['z']), excluded: new Set() }, + colA: { included: new Set(['y', 'x']), excluded: new Set() }, + }), + ); + }); + + it('includes the range bound', () => { + expect( + serializeFilterState({ + colA: { + included: new Set(), + excluded: new Set(), + range: { min: 1, max: 2 }, + }, + }), + ).not.toBe( + serializeFilterState({ + colA: { + included: new Set(), + excluded: new Set(), + range: { min: 1, max: 3 }, + }, + }), + ); + }); + }); + describe('validateSavedFilterValues', () => { it('returns no issues for an empty array', () => { expect(validateSavedFilterValues([])).toEqual([]); diff --git a/packages/common-utils/src/__tests__/metadata.test.ts b/packages/common-utils/src/__tests__/metadata.test.ts index 3e8e32655d..f238321e89 100644 --- a/packages/common-utils/src/__tests__/metadata.test.ts +++ b/packages/common-utils/src/__tests__/metadata.test.ts @@ -1,3 +1,4 @@ +import { ColumnMeta } from '@/clickhouse'; import { ClickhouseClient } from '@/clickhouse/node'; import { GET_ALL_KEY_VALUES_CHUNK_SIZE, @@ -628,7 +629,10 @@ describe('Metadata', () => { await metadata.getKeyValues({ chartConfig: mockChartConfig, keys: ['colA', 'colB'], - keyConditions: [undefined, "(colA IN ('x'))"], + keyConditions: [ + undefined, + { colA: { included: new Set(['x']), excluded: new Set() } }, + ], limit: 10, disableRowLimit: true, source, @@ -647,6 +651,53 @@ 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 + // 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. + const jsonMetadata = new Metadata(mockClickhouseClient, mockCache); + jest + .spyOn(jsonMetadata, 'getColumn') + .mockImplementation(async ({ column }) => + column === 'Attributes' + ? ({ name: 'Attributes', type: 'JSON' } as ColumnMeta) + : undefined, + ); + + const renderChartConfigSpy = jest.spyOn( + renderChartConfigModule, + 'renderChartConfig', + ); + + await jsonMetadata.getKeyValues({ + chartConfig: mockChartConfig, + keys: ["Attributes['cluster']", "Attributes['namespace']"], + keyConditions: [ + undefined, + { + "Attributes['cluster']": { + included: new Set(['prod']), + excluded: new Set(), + }, + }, + ], + limit: 10, + disableRowLimit: true, + source, + }); + + const actualConfig = renderChartConfigSpy.mock.calls.at(-1)![0]; + 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", + ); + // The raw bracket form must not survive into either half. + expect(actualConfig.select).not.toContain("Attributes['"); + }); + it('should apply row limit by default when disableRowLimit is not specified', async () => { await metadata.getKeyValues({ chartConfig: mockChartConfig, diff --git a/packages/common-utils/src/clickhouse/__tests__/materializedViews.test.ts b/packages/common-utils/src/clickhouse/__tests__/materializedViews.test.ts index 26668cef0a..5c4c2f59c8 100644 --- a/packages/common-utils/src/clickhouse/__tests__/materializedViews.test.ts +++ b/packages/common-utils/src/clickhouse/__tests__/materializedViews.test.ts @@ -42,6 +42,12 @@ describe('materializedViews', () => { }; return columns[column]; }), + // These fixtures use plain identifiers, which render to themselves. + renderKeyExpressions: jest + .fn() + .mockImplementation(({ keys }: { keys: string[] }) => + Promise.resolve(keys), + ), } as unknown as Metadata; const MV_CONFIG_METRIC_ROLLUP_1M: MaterializedViewConfiguration = { @@ -2577,7 +2583,10 @@ describe('materializedViews', () => { const result = await optimizeFacetedKeyValuesConfig({ chartConfig: facetedChartConfig, keys: ['ServiceName', 'StatusCode'], - keyConditions: [undefined, "(ServiceName IN ('x'))"], + keyConditions: [ + undefined, + { ServiceName: { included: new Set(['x']), excluded: new Set() } }, + ], source: facetedSource, clickhouseClient: mockClickHouseClient, metadata, @@ -2605,7 +2614,10 @@ describe('materializedViews', () => { const result = await optimizeFacetedKeyValuesConfig({ chartConfig: facetedChartConfig, keys: ['ServiceName', 'NotADimension'], - keyConditions: [undefined, "(ServiceName IN ('x'))"], + keyConditions: [ + undefined, + { ServiceName: { included: new Set(['x']), excluded: new Set() } }, + ], source: facetedSource, clickhouseClient: mockClickHouseClient, metadata, @@ -2626,7 +2638,10 @@ describe('materializedViews', () => { const result = await optimizeFacetedKeyValuesConfig({ chartConfig: facetedChartConfig, keys: ['ServiceName', 'StatusCode'], - keyConditions: [undefined, "(ServiceName IN ('x'))"], + keyConditions: [ + undefined, + { ServiceName: { included: new Set(['x']), excluded: new Set() } }, + ], source: facetedSource, clickhouseClient: mockClickHouseClient, metadata, diff --git a/packages/common-utils/src/core/materializedViews.ts b/packages/common-utils/src/core/materializedViews.ts index c62ad221c2..77c9773108 100644 --- a/packages/common-utils/src/core/materializedViews.ts +++ b/packages/common-utils/src/core/materializedViews.ts @@ -1,6 +1,7 @@ import { differenceInSeconds } from 'date-fns'; import { BaseClickhouseClient, ChSql, chSql } from '@/clickhouse'; +import { FilterState, filterStateToPredicate } from '@/filters'; import { BuilderChartConfigWithOptDateRange, CteChartConfig, @@ -921,8 +922,8 @@ export async function optimizeFacetedKeyValuesConfig< }: { chartConfig: C; keys: string[]; - /** Per-key SQL predicate aligned with `keys` (undefined = unconstrained). */ - keyConditions: (string | undefined)[]; + /** Per-key constraint aligned with `keys` (undefined = unconstrained). */ + keyConditions: (FilterState | undefined)[]; source: TSource | undefined; clickhouseClient: BaseClickhouseClient; metadata: Metadata; @@ -937,6 +938,10 @@ export async function optimizeFacetedKeyValuesConfig< // A covering MV must expose every filter column as a dimension. The per-key // conditions only reference other keys, so requiring all `keys` covers them // too; anything else (e.g. the static `where`) is caught by the EXPLAIN below. + // + // Matched against the RAW keys on purpose: `dimensionColumns` is configured + // by hand in the source form using the same expressions the filters use, not + // the rendered physical form. Only the generated SQL below is rendered. const coveringMvs = mvs.filter(mv => { const intervalsInDateRange = chartConfig.dateRange ? countIntervalsInDateRange(chartConfig.dateRange, mv.minGranularity) @@ -956,13 +961,31 @@ export async function optimizeFacetedKeyValuesConfig< // EXPLAIN only needs the same columns, so the limit is irrelevant here. const explainResults = await Promise.all( coveringMvs.map(async mv => { + // Render against the MV, since that is the table getKeyValues will + // render against once this config is returned. An EXPLAIN built from raw + // expressions would validate different SQL than the one we end up running. + const renderedKeys = await metadata.renderKeyExpressions({ + databaseName: mv.databaseName, + tableName: mv.tableName, + connectionId: chartConfig.connection, + keys, + }); + const rawToRendered = new Map( + keys.map((key, i) => [key, renderedKeys[i]]), + ); const config = { ...structuredClone(chartConfig), timestampValueExpression: mv.timestampColumn, from: { databaseName: mv.databaseName, tableName: mv.tableName }, - select: keys + select: renderedKeys .map((k, i) => { - const condition = keyConditions[i]; + const state = keyConditions[i]; + const condition = + state && + filterStateToPredicate( + state, + key => rawToRendered.get(key) ?? key, + ); return condition ? `groupUniqArrayIf(1)(${k}, ${condition}) AS param${i}` : `groupUniqArray(1)(${k}) AS param${i}`; diff --git a/packages/common-utils/src/core/metadata.ts b/packages/common-utils/src/core/metadata.ts index f723098566..1fdc377f4b 100644 --- a/packages/common-utils/src/core/metadata.ts +++ b/packages/common-utils/src/core/metadata.ts @@ -14,6 +14,11 @@ import { tableExpr, } from '@/clickhouse'; import { renderChartConfig, timeFilterExpr } from '@/core/renderChartConfig'; +import { + FilterState, + filterStateToPredicate, + serializeFilterState, +} from '@/filters'; import { buildTextIndexInfoLookup, skipIndexMatches, @@ -335,6 +340,37 @@ export class Metadata { return keyExpression; } + /** + * Batch-render key expressions to their physical SQL form. + * + * Public so that callers building SQL which must line up with what + * `getKeyValues` emits — e.g. the materialized-view EXPLAIN probe in + * `optimizeFacetedKeyValuesConfig` — can render the same way rather than + * interpolating the raw expressions and diverging on JSON columns. + */ + async renderKeyExpressions({ + databaseName, + tableName, + connectionId, + keys, + }: { + databaseName: string; + tableName: string; + connectionId: string; + keys: string[]; + }): Promise { + return Promise.all( + keys.map(key => + this.renderMetadataKeyExpression({ + databaseName, + tableName, + connectionId, + keyExpression: key, + }), + ), + ); + } + private async queryTableMetadata({ database, table, @@ -2398,13 +2434,18 @@ export class Metadata { chartConfig: BuilderChartConfigWithDateRange; keys: string[]; /** - * Optional per-key SQL predicate, aligned with `keys`. When provided for a - * key, its values are gathered with `groupUniqArrayIf(key, condition)` so - * several "faceted" value lists (each constrained by a different condition) - * can be computed in a single table scan. Only applied when - * `disableRowLimit` is true (the path used by filter dropdowns). + * Optional per-key constraint, aligned with `keys`: the selections that + * should narrow this key's values. When provided for a key, its values are + * gathered with `groupUniqArrayIf(key, )` so several "faceted" + * value lists (each constrained differently) resolve in a single table + * scan. Only applied when `disableRowLimit` is true (the path used by + * filter dropdowns). + * + * Keys inside the state are RAW expressions; they are rendered here the + * same way as `keys`, so the predicate addresses the same physical column + * as the aggregate wrapping it. */ - keyConditions?: (string | undefined)[]; + keyConditions?: (FilterState | undefined)[]; limit?: number; disableRowLimit?: boolean; signal?: AbortSignal; @@ -2422,7 +2463,8 @@ export class Metadata { 'filters', ]), keys, - keyConditions, + // Serialize rather than hashing the raw state: the selections are Sets. + keyConditions: keyConditions?.map(s => s && serializeFilterState(s)), disableRowLimit, }; return this.cache.getOrFetch( @@ -2430,15 +2472,24 @@ export class Metadata { async () => { if (keys.length === 0) return []; - const renderedKeys = await Promise.all( - keys.map(key => - this.renderMetadataKeyExpression({ - databaseName: chartConfig.from.databaseName, - tableName: chartConfig.from.tableName, - connectionId: chartConfig.connection, - keyExpression: key, - }), - ), + const renderedKeys = await this.renderKeyExpressions({ + databaseName: chartConfig.from.databaseName, + tableName: chartConfig.from.tableName, + connectionId: chartConfig.connection, + keys, + }); + + // Render each constraint against the same expressions used in the + // SELECT below. A constraint only ever references sibling keys from + // this same call, so the map always covers it; `?? rawKey` is a + // defensive fallback rather than an expected path. + const rawToRendered = new Map( + keys.map((key, i) => [key, renderedKeys[i]]), + ); + const renderedConditions = keyConditions?.map( + state => + state && + filterStateToPredicate(state, key => rawToRendered.get(key) ?? key), ); // When disableRowLimit is true, query directly without CTE @@ -2448,7 +2499,7 @@ export class Metadata { ...chartConfig, select: renderedKeys .map((k, i) => { - const condition = keyConditions?.[i]; + const condition = renderedConditions?.[i]; // `groupUniqArrayIf` lets each key be constrained by its own // predicate, so multiple faceted value lists are computed in a // single scan instead of one query per key. @@ -2546,8 +2597,8 @@ export class Metadata { }: { chartConfig: BuilderChartConfigWithDateRange; keys: string[]; - /** Per-key SQL predicates for faceted value lookups; see getKeyValues. */ - keyConditions?: (string | undefined)[]; + /** Per-key constraints for faceted value lookups; see getKeyValues. */ + keyConditions?: (FilterState | undefined)[]; source: TSource | undefined; limit?: number; disableRowLimit?: boolean; @@ -2563,7 +2614,8 @@ export class Metadata { 'filters', ]), keys, - keyConditions, + // Serialize rather than hashing the raw state: the selections are Sets. + keyConditions: keyConditions?.map(s => s && serializeFilterState(s)), disableRowLimit, }; return this.cache.getOrFetch( diff --git a/packages/common-utils/src/filters.ts b/packages/common-utils/src/filters.ts index e15e138b01..aa50e9d153 100644 --- a/packages/common-utils/src/filters.ts +++ b/packages/common-utils/src/filters.ts @@ -100,6 +100,63 @@ export const filtersToQuery = ( }); }; +/** + * Render a FilterState as a single AND-joined SQL predicate, remapping every + * key through `renderKey` first. + * + * 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. + * + * Returns undefined when nothing is selected, so callers can branch on + * "constrained vs unconstrained" without inspecting the string. + */ +export const filterStateToPredicate = ( + state: FilterState, + renderKey: (rawKey: string) => string, +): string | undefined => { + const rendered: FilterState = {}; + for (const [rawKey, selection] of Object.entries(state)) { + rendered[renderKey(rawKey)] = selection; + } + const conditions = filtersToQuery(rendered).flatMap(f => + // filtersToQuery only emits `sql` filters (which carry `condition`); the + // `in` guard narrows away the `sql_ast` member of the Filter union. + 'condition' in f ? [f.condition] : [], + ); + return conditions.length + ? conditions.map(c => `(${c})`).join(' AND ') + : undefined; +}; + +/** + * Stable, JSON-safe projection of a FilterState, for use in react-query keys. + * + * A raw FilterState cannot be used as a query key: its selections are `Set`s, + * and TanStack Query hashes keys with JSON.stringify, which serializes any Set + * to `{}` — so every distinct selection would collide on one cache entry. + * Keys and members are sorted so that insertion order alone never produces a + * spurious cache miss. + */ +export const serializeFilterState = (state: FilterState): string => + JSON.stringify( + Object.keys(state) + .sort() + .map(key => { + const { included, excluded, range } = state[key]; + const sortMembers = (values: Set) => + Array.from(values).map(String).sort(); + return [ + key, + sortMembers(included), + sortMembers(excluded), + range ?? null, + ]; + }), + ); + // Helper function to parse a string value as boolean if possible, or otherwise // return as string with surrounding quotes removed and SQL-escaped quotes unescaped. const getBooleanOrUnquotedString = (value: string): string | boolean => { From 6fcfc1c04f11d5a01bd134110397d132f0ce6e76 Mon Sep 17 00:00:00 2001 From: Tom Alexander Date: Wed, 29 Jul 2026 13:32:35 -0400 Subject: [PATCH 11/11] refactor(common-utils): key rendered expressions by raw key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit renderKeyExpressions returns a Map instead of a positional array. Both call sites immediately needed the lookup rather than the array, and building it required indexing a parallel array by loop counter — which tripped security/detect-object-injection and pushed common-utils two warnings over its --max-warnings 92 cap. Also rewrites serializeFilterState over Object.entries so it stops indexing the state by a computed key, for the same reason. No behavior change; common-utils goes from 94 to 89 warnings. --- .../__tests__/materializedViews.test.ts | 2 +- .../src/core/materializedViews.ts | 20 +++---- packages/common-utils/src/core/metadata.ts | 60 ++++++++++--------- packages/common-utils/src/filters.ts | 28 ++++----- 4 files changed, 53 insertions(+), 57 deletions(-) diff --git a/packages/common-utils/src/clickhouse/__tests__/materializedViews.test.ts b/packages/common-utils/src/clickhouse/__tests__/materializedViews.test.ts index 5c4c2f59c8..f25ad35367 100644 --- a/packages/common-utils/src/clickhouse/__tests__/materializedViews.test.ts +++ b/packages/common-utils/src/clickhouse/__tests__/materializedViews.test.ts @@ -46,7 +46,7 @@ describe('materializedViews', () => { renderKeyExpressions: jest .fn() .mockImplementation(({ keys }: { keys: string[] }) => - Promise.resolve(keys), + Promise.resolve(new Map(keys.map(k => [k, k]))), ), } as unknown as Metadata; diff --git a/packages/common-utils/src/core/materializedViews.ts b/packages/common-utils/src/core/materializedViews.ts index 77c9773108..e7ba7d132d 100644 --- a/packages/common-utils/src/core/materializedViews.ts +++ b/packages/common-utils/src/core/materializedViews.ts @@ -964,28 +964,22 @@ export async function optimizeFacetedKeyValuesConfig< // Render against the MV, since that is the table getKeyValues will // render against once this config is returned. An EXPLAIN built from raw // expressions would validate different SQL than the one we end up running. - const renderedKeys = await metadata.renderKeyExpressions({ + const renderedByKey = await metadata.renderKeyExpressions({ databaseName: mv.databaseName, tableName: mv.tableName, connectionId: chartConfig.connection, keys, }); - const rawToRendered = new Map( - keys.map((key, i) => [key, renderedKeys[i]]), - ); + const renderKey = (key: string) => renderedByKey.get(key) ?? key; const config = { ...structuredClone(chartConfig), timestampValueExpression: mv.timestampColumn, from: { databaseName: mv.databaseName, tableName: mv.tableName }, - select: renderedKeys - .map((k, i) => { - const state = keyConditions[i]; - const condition = - state && - filterStateToPredicate( - state, - key => rawToRendered.get(key) ?? key, - ); + select: keys + .map((key, i) => { + const k = renderKey(key); + const state = keyConditions.at(i); + const condition = state && filterStateToPredicate(state, renderKey); return condition ? `groupUniqArrayIf(1)(${k}, ${condition}) AS param${i}` : `groupUniqArray(1)(${k}) AS param${i}`; diff --git a/packages/common-utils/src/core/metadata.ts b/packages/common-utils/src/core/metadata.ts index 1fdc377f4b..c443ce58f7 100644 --- a/packages/common-utils/src/core/metadata.ts +++ b/packages/common-utils/src/core/metadata.ts @@ -341,7 +341,8 @@ export class Metadata { } /** - * Batch-render key expressions to their physical SQL form. + * Batch-render key expressions to their physical SQL form, keyed by the raw + * expression the caller passed in. * * Public so that callers building SQL which must line up with what * `getKeyValues` emits — e.g. the materialized-view EXPLAIN probe in @@ -358,17 +359,22 @@ export class Metadata { tableName: string; connectionId: string; keys: string[]; - }): Promise { - return Promise.all( - keys.map(key => - this.renderMetadataKeyExpression({ - databaseName, - tableName, - connectionId, - keyExpression: key, - }), - ), + }): Promise> { + const rendered = new Map(); + await Promise.all( + keys.map(async key => { + rendered.set( + key, + await this.renderMetadataKeyExpression({ + databaseName, + tableName, + connectionId, + keyExpression: key, + }), + ); + }), ); + return rendered; } private async queryTableMetadata({ @@ -2472,34 +2478,32 @@ export class Metadata { async () => { if (keys.length === 0) return []; - const renderedKeys = await this.renderKeyExpressions({ + // A constraint only ever references sibling keys from this same call, + // so the map always covers it; `?? key` is a defensive fallback rather + // than an expected path. + const renderedByKey = await this.renderKeyExpressions({ databaseName: chartConfig.from.databaseName, tableName: chartConfig.from.tableName, connectionId: chartConfig.connection, keys, }); - - // Render each constraint against the same expressions used in the - // SELECT below. A constraint only ever references sibling keys from - // this same call, so the map always covers it; `?? rawKey` is a - // defensive fallback rather than an expected path. - const rawToRendered = new Map( - keys.map((key, i) => [key, renderedKeys[i]]), - ); - const renderedConditions = keyConditions?.map( - state => - state && - filterStateToPredicate(state, key => rawToRendered.get(key) ?? key), - ); + const renderKey = (key: string) => renderedByKey.get(key) ?? key; + const renderedKeys = keys.map(renderKey); // When disableRowLimit is true, query directly without CTE // Otherwise, use CTE with row limits for sampling const sqlConfig = disableRowLimit ? { ...chartConfig, - select: renderedKeys - .map((k, i) => { - const condition = renderedConditions?.[i]; + select: keys + .map((key, i) => { + const k = renderKey(key); + const state = keyConditions?.at(i); + // Render the constraint against the same expressions used in + // the SELECT, or the predicate would address a different + // physical column than the aggregate wrapping it. + const condition = + state && filterStateToPredicate(state, renderKey); // `groupUniqArrayIf` lets each key be constrained by its own // predicate, so multiple faceted value lists are computed in a // single scan instead of one query per key. diff --git a/packages/common-utils/src/filters.ts b/packages/common-utils/src/filters.ts index aa50e9d153..3bc425baea 100644 --- a/packages/common-utils/src/filters.ts +++ b/packages/common-utils/src/filters.ts @@ -140,22 +140,20 @@ export const filterStateToPredicate = ( * Keys and members are sorted so that insertion order alone never produces a * spurious cache miss. */ -export const serializeFilterState = (state: FilterState): string => - JSON.stringify( - Object.keys(state) - .sort() - .map(key => { - const { included, excluded, range } = state[key]; - const sortMembers = (values: Set) => - Array.from(values).map(String).sort(); - return [ - key, - sortMembers(included), - sortMembers(excluded), - range ?? null, - ]; - }), +export const serializeFilterState = (state: FilterState): string => { + const sortMembers = (values: Set) => + Array.from(values).map(String).sort(); + return JSON.stringify( + Object.entries(state) + .sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0)) + .map(([key, { included, excluded, range }]) => [ + key, + sortMembers(included), + sortMembers(excluded), + range ?? null, + ]), ); +}; // Helper function to parse a string value as boolean if possible, or otherwise // return as string with surrounding quotes removed and SQL-escaped quotes unescaped.