From c25086d21819d2f360b68c8c4f49a35c9ed3f3fa Mon Sep 17 00:00:00 2001 From: Aaron Knudtson <87577305+knudtty@users.noreply.github.com> Date: Tue, 2 Jun 2026 15:28:06 -0400 Subject: [PATCH 1/3] feat(common-utils): apply direct_read KV items optimization to SQL filters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SQL `type: 'sql'` filters on Map columns now get the same has()/hasAny() rewrite that Lucene filters already use when a KV items column with a text(tokenizer=array) skip index exists. - Map['k'] = 'v' → has(Items, concat('k', '=', 'v')) - Map['k'] IN ('a','b') → hasAny(Items, array(concat(...), ...)) Extracts buildKvItemsLookup from CustomSchemaSQLSerializerV2 into a shared export. Removes debug console.log('AVK filter'). --- packages/common-utils/CHANGELOG.md | 18 ++ .../src/__tests__/renderChartConfig.test.ts | 148 ++++++++++++++++ .../src/core/renderChartConfig.ts | 146 +++++++++++++++- packages/common-utils/src/queryParser.ts | 163 ++++++++++-------- 4 files changed, 400 insertions(+), 75 deletions(-) diff --git a/packages/common-utils/CHANGELOG.md b/packages/common-utils/CHANGELOG.md index f75e287c9b..450dcf1b0e 100644 --- a/packages/common-utils/CHANGELOG.md +++ b/packages/common-utils/CHANGELOG.md @@ -4,6 +4,24 @@ ### Minor Changes +- feat: apply direct_read KV items optimization to SQL filters + + SQL `type: 'sql'` filters on Map columns (e.g. + `LogAttributes['key'] IN ('a', 'b')`) now get the same `has()` / + `hasAny()` rewrite that Lucene filters already use. When a KV items + column with a `text(tokenizer=array)` skip index exists for the Map, + the condition is rewritten at the filter site before rendering: + + - `Map['k'] = 'v'` → `has(Items, concat('k', '=', 'v'))` + - `Map['k'] IN ('a', 'b')` → `hasAny(Items, array(concat('k', '=', 'a'), concat('k', '=', 'b')))` + + Empty-string values are left unrewritten to preserve ClickHouse's + missing-key semantics (`Map(String, String)['absent'] = ''`). + + Also extracts `buildKvItemsLookup` from `CustomSchemaSQLSerializerV2` + into a shared top-level export so both the Lucene serializer and the + SQL filter rewriter can use the same lookup logic. + - 3123db53: feat: experimental promql support - dcab1cb6: feat: default the direct_read map column optimization on supported ClickHouse versions diff --git a/packages/common-utils/src/__tests__/renderChartConfig.test.ts b/packages/common-utils/src/__tests__/renderChartConfig.test.ts index 7fe70ca760..164d43292d 100644 --- a/packages/common-utils/src/__tests__/renderChartConfig.test.ts +++ b/packages/common-utils/src/__tests__/renderChartConfig.test.ts @@ -984,6 +984,154 @@ describe('renderChartConfig', () => { }); }); + describe('SQL filter KV items direct_read optimization', () => { + const stubKvItemsMetadata = () => { + mockMetadata.getColumns = jest.fn().mockResolvedValue([ + { + name: 'LogAttributes', + type: 'Map(String, String)', + default_type: '', + default_expression: '', + }, + { + name: 'LogAttributeItems', + type: 'Array(String)', + default_type: 'MATERIALIZED', + default_expression: + "arrayMap((arr) -> concat(arr.1, '=', arr.2), LogAttributes::Array(Tuple(String, String)))", + }, + ]); + mockMetadata.getSkipIndices = jest.fn().mockResolvedValue([ + { + name: 'idx_log_attr_items', + type: 'text', + typeFull: 'text(tokenizer=array)', + expression: 'LogAttributeItems', + granularity: 10000000, + }, + ]); + mockMetadata.getServerVersion = jest + .fn() + .mockResolvedValue([26, 5, 0, 0]); + mockMetadata.getMaterializedColumnsLookupTable = jest + .fn() + .mockResolvedValue(new Map()); + }; + + const buildConfig = (condition: string): ChartConfigWithOptDateRange => ({ + connection: 'test-connection', + from: { databaseName: 'default', tableName: 'otel_logs' }, + select: [{ aggFn: 'count', valueExpression: '' }], + where: '', + whereLanguage: 'sql', + filters: [{ type: 'sql', condition }], + timestampValueExpression: 'Timestamp', + dateRange: [new Date('2025-01-01'), new Date('2025-01-02')], + granularity: '1 minute', + }); + + it('rewrites `Map[key] = value` to has() when a KV items column exists', async () => { + stubKvItemsMetadata(); + const sql = parameterizedQueryToSql( + await renderChartConfig( + buildConfig("LogAttributes['service.name'] = 'api'"), + mockMetadata, + querySettings, + ), + ); + expect(sql).toContain( + "has(`LogAttributeItems`, concat('service.name', '=', 'api'))", + ); + expect(sql).not.toContain("LogAttributes['service.name'] = 'api'"); + }); + + it('rewrites `Map[key] IN (one)` to has()', async () => { + stubKvItemsMetadata(); + const sql = parameterizedQueryToSql( + await renderChartConfig( + buildConfig("LogAttributes['service.name'] IN ('api')"), + mockMetadata, + querySettings, + ), + ); + expect(sql).toContain( + "has(`LogAttributeItems`, concat('service.name', '=', 'api'))", + ); + }); + + it('rewrites `Map[key] IN (many)` to hasAny(... array(...))', async () => { + stubKvItemsMetadata(); + const sql = parameterizedQueryToSql( + await renderChartConfig( + buildConfig("LogAttributes['k'] IN ('a', 'b', 'c')"), + mockMetadata, + querySettings, + ), + ); + expect(sql).toContain( + "hasAny(`LogAttributeItems`, array(concat('k', '=', 'a'), concat('k', '=', 'b'), concat('k', '=', 'c')))", + ); + }); + + it('leaves the condition unchanged when no KV items column exists', async () => { + mockMetadata.getColumns = jest.fn().mockResolvedValue([ + { + name: 'LogAttributes', + type: 'Map(String, String)', + default_type: '', + default_expression: '', + }, + ]); + mockMetadata.getSkipIndices = jest.fn().mockResolvedValue([]); + mockMetadata.getServerVersion = jest + .fn() + .mockResolvedValue([26, 5, 0, 0]); + mockMetadata.getMaterializedColumnsLookupTable = jest + .fn() + .mockResolvedValue(new Map()); + + const sql = parameterizedQueryToSql( + await renderChartConfig( + buildConfig("LogAttributes['k'] = 'v'"), + mockMetadata, + querySettings, + ), + ); + expect(sql).toContain("LogAttributes['k'] = 'v'"); + expect(sql).not.toContain('has('); + }); + + it('does not rewrite when value is empty (Map[k]= preserves missing-key semantics)', async () => { + stubKvItemsMetadata(); + const sql = parameterizedQueryToSql( + await renderChartConfig( + buildConfig("LogAttributes['k'] = ''"), + mockMetadata, + querySettings, + ), + ); + expect(sql).toContain("LogAttributes['k'] = ''"); + expect(sql).not.toContain('has('); + }); + + it('rewrites only the matching Map subscript in a compound AND condition', async () => { + stubKvItemsMetadata(); + const sql = parameterizedQueryToSql( + await renderChartConfig( + buildConfig( + "LogAttributes['service.name'] = 'api' AND SeverityText = 'error'", + ), + mockMetadata, + querySettings, + ), + ); + expect(sql).toContain( + "has(`LogAttributeItems`, concat('service.name', '=', 'api'))", + ); + expect(sql).toContain("SeverityText = 'error'"); + }); + }); + describe('k8s semantic convention migrations', () => { it('should generate SQL with metricNameSql for k8s.pod.cpu.utilization gauge metric', async () => { const config: ChartConfigWithOptDateRange = { diff --git a/packages/common-utils/src/core/renderChartConfig.ts b/packages/common-utils/src/core/renderChartConfig.ts index 47e53bd649..59a5123286 100644 --- a/packages/common-utils/src/core/renderChartConfig.ts +++ b/packages/common-utils/src/core/renderChartConfig.ts @@ -22,7 +22,12 @@ import { isRawSqlChartConfig, } from '@/guards'; import { replaceMacros } from '@/macros'; -import { CustomSchemaSQLSerializerV2, SearchQueryBuilder } from '@/queryParser'; +import { + buildKvItemsLookup, + CustomSchemaSQLSerializerV2, + KvItemsLookup, + SearchQueryBuilder, +} from '@/queryParser'; import { QUERY_PARAMS_BY_DISPLAY_TYPE } from '@/rawSqlParams'; import { AggregateFunction, @@ -338,6 +343,124 @@ const fastifySQL = ({ } }; +const rewriteSqlFilterWithKvItems = ( + condition: string, + kvItemsLookup: KvItemsLookup, +): string => { + if (kvItemsLookup.size === 0) return condition; + try { + const parser = new SQLParser.Parser(); + const prefix = 'SELECT 1 FROM `t` WHERE '; + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + const ast = parser.astify(`${prefix}${condition}`, { + database: 'Postgresql', + }) as SQLParser.Select; + + const tryOptimize = (node: any): void => { + const op = String(node.operator ?? '').toUpperCase(); + if (op !== '=' && op !== 'IN') return; + const left = node.left; + if (left?.type !== 'column_ref' || typeof left.column === 'string') { + return; + } + const mapColumn = left.column?.expr?.value; + const arrIdx = left.array_index; + if ( + typeof mapColumn !== 'string' || + !Array.isArray(arrIdx) || + arrIdx.length !== 1 + ) { + return; + } + const idxNode = arrIdx[0]?.index; + if ( + idxNode?.type !== 'single_quote_string' || + typeof idxNode.value !== 'string' + ) { + return; + } + const mapKey: string = idxNode.value; + const info = kvItemsLookup.get(mapColumn); + if (!info) return; + + let values: string[]; + if (op === '=') { + const right = node.right; + if ( + right?.type !== 'single_quote_string' || + typeof right.value !== 'string' + ) { + return; + } + values = [right.value]; + } else { + const right = node.right; + if (right?.type !== 'expr_list' || !Array.isArray(right.value)) return; + const collected: string[] = []; + for (const item of right.value) { + if ( + item?.type !== 'single_quote_string' || + typeof item.value !== 'string' + ) { + return; + } + collected.push(item.value); + } + values = collected; + } + // Bail on empty values: `Map['k']='' ` also matches absent keys because + // Map(String, String)'s subscript default is '', which `has(items, 'k=')` + // alone does not preserve. Same rationale for empty entries in IN lists. + if (values.length === 0 || values.some(v => v === '')) return; + + const replacement = + values.length === 1 + ? SqlString.format('has(??, concat(?, ?, ?))', [ + info.kvItemsColumn, + mapKey, + info.separator, + values[0], + ]) + : `hasAny(${SqlString.format('??', [ + info.kvItemsColumn, + ])}, array(${values + .map(v => + SqlString.format('concat(?, ?, ?)', [ + mapKey, + info.separator, + v, + ]), + ) + .join(', ')}))`; + + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- astify returns union type, we expect Select + const replAst = parser.astify(`${prefix}${replacement}`, { + database: 'Postgresql', + }) as SQLParser.Select; + const newWhere = replAst.where; + if (newWhere == null) return; + for (const k of Object.keys(node)) delete node[k]; + Object.assign(node, newWhere); + }; + + const traverse = (node: any): void => { + if (node == null) return; + if (node.type === 'binary_expr') { + traverse(node.left); + traverse(node.right); + tryOptimize(node); + } else if (node.type === 'expr_list' && Array.isArray(node.value)) { + node.value.forEach(traverse); + } + }; + traverse(ast.where); + + return parser.sqlify(ast).slice(prefix.length); + } catch { + return condition; + } +}; + const aggFnExpr = ({ fn, expr, @@ -966,6 +1089,21 @@ async function renderWhere( ).filter(v => v !== null) as ChSql[]; } + const hasSqlFilter = + chartConfig.filters?.some(f => f.type === 'sql') ?? false; + const kvItemsLookup: KvItemsLookup = + hasSqlFilter && + chartConfig.from.databaseName && + chartConfig.from.tableName && + !hasSubqueryCte(chartConfig.with) + ? await buildKvItemsLookup({ + metadata, + databaseName: chartConfig.from.databaseName, + tableName: chartConfig.from.tableName, + connectionId: chartConfig.connection, + }) + : new Map(); + const filterConditions = await Promise.all( (chartConfig.filters ?? []).map(async filter => { if (filter.type === 'sql_ast') { @@ -975,9 +1113,13 @@ async function renderWhere( ')', ); } else if (filter.type === 'lucene' || filter.type === 'sql') { + const condition = + filter.type === 'sql' + ? rewriteSqlFilterWithKvItems(filter.condition, kvItemsLookup) + : filter.condition; return wrapChSqlIfNotEmpty( await renderWhereExpression({ - condition: filter.condition, + condition, from: chartConfig.from, language: filter.type, implicitColumnExpression: chartConfig.implicitColumnExpression, diff --git a/packages/common-utils/src/queryParser.ts b/packages/common-utils/src/queryParser.ts index f96ffb635b..fa2de6dd5c 100644 --- a/packages/common-utils/src/queryParser.ts +++ b/packages/common-utils/src/queryParser.ts @@ -1075,6 +1075,88 @@ const KV_ITEMS_STRATEGIES = [ parseKvItemsCastExpression, ] as const; +/** + * Builds a lookup from map column name to KV items column name. + * A KV items column is an ALIAS/MATERIALIZED column whose expression is + * arrayMap((k,v)->concat(k,'=',v), mapKeys(X), mapValues(X)) and which has + * a text(tokenizer=array) skip index. + * + * The version gate (`supportsDirectReadMap`) only applies to ALIAS items + * columns: ALIAS columns are computed at query time, so `has(items, ...)` + * against an ALIAS only realizes its speedup when the server can perform a + * direct_read against the underlying Map's tuple storage. MATERIALIZED items + * columns are physically stored on disk, so `has()` reads them directly and + * is fast on any ClickHouse version that supports the text index itself. + * + * Returns an empty Map on any failure; never throws. + */ +export async function buildKvItemsLookup({ + metadata, + databaseName, + tableName, + connectionId, +}: { + metadata: Metadata; + databaseName: string; + tableName: string; + connectionId: string; +}): Promise { + const lookup: KvItemsLookup = new Map(); + try { + const [serverVersion, columns, skipIndices] = await Promise.all([ + metadata.getServerVersion({ connectionId }), + metadata.getColumns({ databaseName, tableName, connectionId }), + metadata + .getSkipIndices({ databaseName, tableName, connectionId }) + .catch(() => [] as SkipIndexMetadata[]), + ]); + + const directReadSupported = supportsDirectReadMap(serverVersion); + + const kvItemsCandidates = columns.filter( + c => + (c.default_type === 'ALIAS' || c.default_type === 'MATERIALIZED') && + c.default_expression, + ); + + for (const candidate of kvItemsCandidates) { + if (candidate.default_type === 'ALIAS' && !directReadSupported) { + continue; + } + + const parsed = (() => { + let parsed: { mapColumn: string; separator: string } | undefined; + for (const strategy of KV_ITEMS_STRATEGIES) { + parsed = strategy(candidate.default_expression); + if (parsed) break; + } + return parsed; + })(); + if (!parsed) continue; + + const candidateName = normalizeChExpression(candidate.name); + const candidateExpr = normalizeChExpression(candidate.default_expression); + const hasArrayTextIndex = skipIndices.some(idx => { + if (idx.type !== 'text') return false; + const tokenizer = parseTokenizerFromTextIndex(idx); + if (tokenizer?.type !== 'array') return false; + const idxExpr = normalizeChExpression(idx.expression); + return idxExpr === candidateName || idxExpr === candidateExpr; + }); + + if (hasArrayTextIndex) { + lookup.set(parsed.mapColumn, { + kvItemsColumn: candidate.name, + separator: parsed.separator, + }); + } + } + } catch (error) { + console.warn('Error building KV items lookup:', error); + } + return lookup; +} + export class CustomSchemaSQLSerializerV2 extends SQLSerializer { private metadata: Metadata; private tableName: string; @@ -1130,81 +1212,16 @@ export class CustomSchemaSQLSerializerV2 extends SQLSerializer { return false; }); - // Pre-fetch KV items lookup (map column -> KV items column with text(tokenizer=array) index) - this.kvItemsLookupPromise = this.buildKvItemsLookup().catch(error => { - console.warn('Error building KV items lookup:', error); - return new Map(); - }); + this.kvItemsLookupPromise = this.buildKvItemsLookup(); } - /** - * Builds a lookup from map column name to KV items column name. - * A KV items column is an ALIAS/MATERIALIZED column whose expression is - * arrayMap((k,v)->concat(k,'=',v), mapKeys(X), mapValues(X)) and which has - * a text(tokenizer=array) skip index. - * - * The version gate (`supportsDirectReadMap`) only applies to ALIAS items - * columns: ALIAS columns are computed at query time, so `has(items, ...)` - * against an ALIAS only realizes its speedup when the server can perform a - * direct_read against the underlying Map's tuple storage. MATERIALIZED items - * columns are physically stored on disk, so `has()` reads them directly and - * is fast on any ClickHouse version that supports the text index itself. - */ - private async buildKvItemsLookup(): Promise { - const [serverVersion, columns, skipIndices] = await Promise.all([ - this.metadata.getServerVersion({ connectionId: this.connectionId }), - this.metadata.getColumns({ - databaseName: this.databaseName, - tableName: this.tableName, - connectionId: this.connectionId, - }), - this.skipIndicesPromise ?? Promise.resolve([]), - ]); - - const directReadSupported = supportsDirectReadMap(serverVersion); - - const lookup: KvItemsLookup = new Map(); - - const kvItemsCandidates = columns.filter( - c => - (c.default_type === 'ALIAS' || c.default_type === 'MATERIALIZED') && - c.default_expression, - ); - - for (const candidate of kvItemsCandidates) { - if (candidate.default_type === 'ALIAS' && !directReadSupported) { - continue; - } - - const parsed = (() => { - let parsed: { mapColumn: string; separator: string } | undefined; - for (const strategy of KV_ITEMS_STRATEGIES) { - parsed = strategy(candidate.default_expression); - if (parsed) break; - } - return parsed; - })(); - if (!parsed) continue; - - const candidateName = normalizeChExpression(candidate.name); - const candidateExpr = normalizeChExpression(candidate.default_expression); - const hasArrayTextIndex = skipIndices.some(idx => { - if (idx.type !== 'text') return false; - const tokenizer = parseTokenizerFromTextIndex(idx); - if (tokenizer?.type !== 'array') return false; - const idxExpr = normalizeChExpression(idx.expression); - return idxExpr === candidateName || idxExpr === candidateExpr; - }); - - if (hasArrayTextIndex) { - lookup.set(parsed.mapColumn, { - kvItemsColumn: candidate.name, - separator: parsed.separator, - }); - } - } - - return lookup; + private buildKvItemsLookup(): Promise { + return buildKvItemsLookup({ + metadata: this.metadata, + databaseName: this.databaseName, + tableName: this.tableName, + connectionId: this.connectionId, + }); } /** From b624f1926a668b945a63c7910937a7fdad6e3a05 Mon Sep 17 00:00:00 2001 From: Aaron Knudtson <87577305+knudtty@users.noreply.github.com> Date: Tue, 2 Jun 2026 15:45:29 -0400 Subject: [PATCH 2/3] add changeset --- .changeset/many-comics-rule.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/many-comics-rule.md diff --git a/.changeset/many-comics-rule.md b/.changeset/many-comics-rule.md new file mode 100644 index 0000000000..26eae6d49e --- /dev/null +++ b/.changeset/many-comics-rule.md @@ -0,0 +1,5 @@ +--- +"@hyperdx/common-utils": patch +--- + +feat: add direct_read optimization for filters From c9f74aed137a089ce046996e44c51b144200d706 Mon Sep 17 00:00:00 2001 From: Aaron Knudtson <87577305+knudtty@users.noreply.github.com> Date: Wed, 3 Jun 2026 16:14:53 -0400 Subject: [PATCH 3/3] address warren's feedback --- .../rewriteSqlFilterWithKvItems.test.ts | 327 ++++++++++++++++++ .../src/core/renderChartConfig.ts | 28 +- packages/common-utils/src/queryParser.ts | 13 +- 3 files changed, 352 insertions(+), 16 deletions(-) create mode 100644 packages/common-utils/src/__tests__/rewriteSqlFilterWithKvItems.test.ts diff --git a/packages/common-utils/src/__tests__/rewriteSqlFilterWithKvItems.test.ts b/packages/common-utils/src/__tests__/rewriteSqlFilterWithKvItems.test.ts new file mode 100644 index 0000000000..5ed0d03523 --- /dev/null +++ b/packages/common-utils/src/__tests__/rewriteSqlFilterWithKvItems.test.ts @@ -0,0 +1,327 @@ +import { rewriteSqlFilterWithKvItems } from '@/core/renderChartConfig'; +import { KvItemsLookup } from '@/queryParser'; + +const makeLookup = ( + entries: Array<[string, { kvItemsColumn: string; separator: string }]>, +): KvItemsLookup => new Map(entries); + +const defaultLookup: KvItemsLookup = makeLookup([ + ['LogAttributes', { kvItemsColumn: 'LogAttributeItems', separator: '=' }], +]); + +describe('rewriteSqlFilterWithKvItems', () => { + describe('early returns', () => { + it('returns the condition verbatim when the lookup is empty', () => { + expect( + rewriteSqlFilterWithKvItems('this is not valid SQL', new Map()), + ).toBe('this is not valid SQL'); + + expect( + rewriteSqlFilterWithKvItems("LogAttributes['k'] = 'v'", new Map()), + ).toBe("LogAttributes['k'] = 'v'"); + }); + + it('returns the condition verbatim when the SQL fails to parse', () => { + const condition = 'this is not valid SQL ???'; + expect(rewriteSqlFilterWithKvItems(condition, defaultLookup)).toBe( + condition, + ); + }); + + it('returns the condition verbatim for an empty condition string', () => { + expect(rewriteSqlFilterWithKvItems('', defaultLookup)).toBe(''); + }); + }); + + describe('= operator', () => { + it("rewrites Map['key'] = 'value' to has(kvItems, concat(key, sep, value))", () => { + const result = rewriteSqlFilterWithKvItems( + "LogAttributes['service.name'] = 'api'", + defaultLookup, + ); + expect(result).toBe( + "has(`LogAttributeItems`, concat('service.name', '=', 'api'))", + ); + }); + + it('does not rewrite when the value is an empty string', () => { + // Map(String, String) subscript defaults to '' for absent keys, so + // `Map['k'] = ''` would silently match records where 'k' is unset if + // rewritten to has(items, 'k='). Same rationale as the source comment. + const result = rewriteSqlFilterWithKvItems( + "LogAttributes['k'] = ''", + defaultLookup, + ); + expect(result).not.toContain('has('); + expect(result).toContain("LogAttributes['k'] = ''"); + }); + + it('does not rewrite when the right side is a numeric literal', () => { + const result = rewriteSqlFilterWithKvItems( + "LogAttributes['k'] = 5", + defaultLookup, + ); + expect(result).not.toContain('has('); + expect(result).toContain("LogAttributes['k'] = 5"); + }); + + it('does not rewrite when the right side is a column reference', () => { + const result = rewriteSqlFilterWithKvItems( + "LogAttributes['k'] = Severity", + defaultLookup, + ); + expect(result).not.toContain('has('); + }); + + it('does not rewrite when the subscript appears on the right side', () => { + const result = rewriteSqlFilterWithKvItems( + "'api' = LogAttributes['k']", + defaultLookup, + ); + expect(result).not.toContain('has('); + }); + + it('does not rewrite when the map column is not in the lookup', () => { + const result = rewriteSqlFilterWithKvItems( + "ResourceAttributes['k'] = 'v'", + defaultLookup, + ); + expect(result).not.toContain('has('); + expect(result).toContain("ResourceAttributes['k'] = 'v'"); + }); + + it('does not rewrite plain column comparisons (no subscript)', () => { + const result = rewriteSqlFilterWithKvItems( + "Severity = 'error'", + defaultLookup, + ); + expect(result).not.toContain('has('); + expect(result).toContain("Severity = 'error'"); + }); + }); + + describe('IN operator', () => { + it("rewrites Map['key'] IN ('a') (single item) to has(...) not hasAny(...)", () => { + const result = rewriteSqlFilterWithKvItems( + "LogAttributes['k'] IN ('a')", + defaultLookup, + ); + expect(result).toBe("has(`LogAttributeItems`, concat('k', '=', 'a'))"); + }); + + it("rewrites Map['key'] IN ('a','b','c') to hasAny(... array(...))", () => { + const result = rewriteSqlFilterWithKvItems( + "LogAttributes['k'] IN ('a', 'b', 'c')", + defaultLookup, + ); + expect(result).toBe( + "hasAny(`LogAttributeItems`, array(concat('k', '=', 'a'), concat('k', '=', 'b'), concat('k', '=', 'c')))", + ); + }); + + it('does not rewrite when any IN value is an empty string', () => { + const result = rewriteSqlFilterWithKvItems( + "LogAttributes['k'] IN ('a', '')", + defaultLookup, + ); + expect(result).not.toContain('has('); + expect(result).not.toContain('hasAny('); + }); + + it('does not rewrite when any IN value is non-string', () => { + const result = rewriteSqlFilterWithKvItems( + "LogAttributes['k'] IN ('a', 5)", + defaultLookup, + ); + expect(result).not.toContain('has('); + expect(result).not.toContain('hasAny('); + }); + + it('does not rewrite NOT IN', () => { + const result = rewriteSqlFilterWithKvItems( + "LogAttributes['k'] NOT IN ('a', 'b')", + defaultLookup, + ); + expect(result).not.toContain('has('); + expect(result).not.toContain('hasAny('); + }); + }); + + describe('other operators are not rewritten', () => { + it.each([ + ["LogAttributes['k'] != 'v'", '!='], + ["LogAttributes['k'] < 'v'", '<'], + ["LogAttributes['k'] > 'v'", '>'], + ["LogAttributes['k'] <= 'v'", '<='], + ["LogAttributes['k'] >= 'v'", '>='], + ["LogAttributes['k'] LIKE '%v%'", 'LIKE'], + ["LogAttributes['k'] BETWEEN 'a' AND 'z'", 'BETWEEN'], + ])('leaves %s untouched (operator: %s)', condition => { + const result = rewriteSqlFilterWithKvItems(condition, defaultLookup); + expect(result).not.toContain('has('); + expect(result).not.toContain('hasAny('); + }); + }); + + describe('compound conditions', () => { + it('rewrites only the matching subscript in an AND chain', () => { + const result = rewriteSqlFilterWithKvItems( + "LogAttributes['service.name'] = 'api' AND Severity = 'error'", + defaultLookup, + ); + expect(result).toContain( + "has(`LogAttributeItems`, concat('service.name', '=', 'api'))", + ); + expect(result).toContain("Severity = 'error'"); + expect(result).not.toContain("LogAttributes['service.name']"); + }); + + it('rewrites only the matching subscript in an OR chain', () => { + const result = rewriteSqlFilterWithKvItems( + "LogAttributes['k'] = 'v' OR Severity = 'error'", + defaultLookup, + ); + expect(result).toContain( + "has(`LogAttributeItems`, concat('k', '=', 'v'))", + ); + expect(result).toContain("Severity = 'error'"); + }); + + it('rewrites every matching subscript when several appear together', () => { + const result = rewriteSqlFilterWithKvItems( + "LogAttributes['a'] = 'x' AND LogAttributes['b'] = 'y'", + defaultLookup, + ); + expect(result).toContain( + "has(`LogAttributeItems`, concat('a', '=', 'x'))", + ); + expect(result).toContain( + "has(`LogAttributeItems`, concat('b', '=', 'y'))", + ); + expect(result).not.toContain("LogAttributes['"); + }); + + it('rewrites subscripts inside nested AND/OR groups', () => { + const result = rewriteSqlFilterWithKvItems( + "LogAttributes['k'] = 'v' AND (LogAttributes['k2'] = 'v2' OR Severity = 'x')", + defaultLookup, + ); + expect(result).toContain( + "has(`LogAttributeItems`, concat('k', '=', 'v'))", + ); + expect(result).toContain( + "has(`LogAttributeItems`, concat('k2', '=', 'v2'))", + ); + expect(result).toContain("Severity = 'x'"); + }); + + it('mixes = and IN rewrites in the same condition', () => { + const result = rewriteSqlFilterWithKvItems( + "LogAttributes['k'] = 'v' AND LogAttributes['env'] IN ('prod', 'staging')", + defaultLookup, + ); + expect(result).toContain( + "has(`LogAttributeItems`, concat('k', '=', 'v'))", + ); + expect(result).toContain( + "hasAny(`LogAttributeItems`, array(concat('env', '=', 'prod'), concat('env', '=', 'staging')))", + ); + }); + }); + + describe('lookup configuration', () => { + it('uses the configured separator in the rewritten concat', () => { + const colonLookup = makeLookup([ + [ + 'LogAttributes', + { kvItemsColumn: 'LogAttributeItems', separator: ':' }, + ], + ]); + const result = rewriteSqlFilterWithKvItems( + "LogAttributes['k'] = 'v'", + colonLookup, + ); + expect(result).toBe("has(`LogAttributeItems`, concat('k', ':', 'v'))"); + }); + + it('uses the configured kv items column name (backtick-quoted)', () => { + const lookup = makeLookup([ + [ + 'LogAttributes', + { kvItemsColumn: 'CustomItemsColumn', separator: '=' }, + ], + ]); + const result = rewriteSqlFilterWithKvItems( + "LogAttributes['k'] = 'v'", + lookup, + ); + expect(result).toBe("has(`CustomItemsColumn`, concat('k', '=', 'v'))"); + }); + + it('applies independent lookup entries to each map column', () => { + const lookup = makeLookup([ + [ + 'LogAttributes', + { kvItemsColumn: 'LogAttributeItems', separator: '=' }, + ], + [ + 'ResourceAttributes', + { kvItemsColumn: 'ResourceAttributeItems', separator: ':' }, + ], + ]); + const result = rewriteSqlFilterWithKvItems( + "LogAttributes['k'] = 'v' AND ResourceAttributes['k2'] = 'v2'", + lookup, + ); + expect(result).toContain( + "has(`LogAttributeItems`, concat('k', '=', 'v'))", + ); + expect(result).toContain( + "has(`ResourceAttributeItems`, concat('k2', ':', 'v2'))", + ); + }); + + it('rewrites only map columns present in the lookup', () => { + const result = rewriteSqlFilterWithKvItems( + "LogAttributes['k'] = 'v' AND OtherMap['k2'] = 'v2'", + defaultLookup, + ); + expect(result).toContain( + "has(`LogAttributeItems`, concat('k', '=', 'v'))", + ); + expect(result).toContain("OtherMap['k2'] = 'v2'"); + }); + }); + + describe('edge cases', () => { + it('preserves whitespace and special characters in map keys', () => { + const result = rewriteSqlFilterWithKvItems( + "LogAttributes['key with spaces'] = 'value'", + defaultLookup, + ); + expect(result).toBe( + "has(`LogAttributeItems`, concat('key with spaces', '=', 'value'))", + ); + }); + + it('does not rewrite chained subscripts (Map[a][b])', () => { + const result = rewriteSqlFilterWithKvItems( + "LogAttributes['k']['k2'] = 'v'", + defaultLookup, + ); + expect(result).not.toContain('has('); + }); + + it('is idempotent on an already-rewritten has() condition', () => { + const alreadyRewritten = + "has(`LogAttributeItems`, concat('k', '=', 'v'))"; + const result = rewriteSqlFilterWithKvItems( + alreadyRewritten, + defaultLookup, + ); + expect(result).toContain( + "has(`LogAttributeItems`, concat('k', '=', 'v'))", + ); + }); + }); +}); diff --git a/packages/common-utils/src/core/renderChartConfig.ts b/packages/common-utils/src/core/renderChartConfig.ts index 59a5123286..b399a139fb 100644 --- a/packages/common-utils/src/core/renderChartConfig.ts +++ b/packages/common-utils/src/core/renderChartConfig.ts @@ -343,7 +343,7 @@ const fastifySQL = ({ } }; -const rewriteSqlFilterWithKvItems = ( +export const rewriteSqlFilterWithKvItems = ( condition: string, kvItemsLookup: KvItemsLookup, ): string => { @@ -356,15 +356,21 @@ const rewriteSqlFilterWithKvItems = ( database: 'Postgresql', }) as SQLParser.Select; - const tryOptimize = (node: any): void => { + const tryOptimize = ( + node: SQLParser.ExpressionValue | SQLParser.ExprList, + ): void => { + if (!('operator' in node)) return; const op = String(node.operator ?? '').toUpperCase(); if (op !== '=' && op !== 'IN') return; const left = node.left; - if (left?.type !== 'column_ref' || typeof left.column === 'string') { + if ( + left?.type !== 'column_ref' || + ('column' in left && typeof left.column === 'string') + ) { return; } - const mapColumn = left.column?.expr?.value; - const arrIdx = left.array_index; + const mapColumn = left['column']?.expr?.value; + const arrIdx = left['array_index']; if ( typeof mapColumn !== 'string' || !Array.isArray(arrIdx) || @@ -443,11 +449,17 @@ const rewriteSqlFilterWithKvItems = ( Object.assign(node, newWhere); }; - const traverse = (node: any): void => { + const traverse = ( + node: SQLParser.ExpressionValue | SQLParser.ExprList | null, + ): void => { if (node == null) return; if (node.type === 'binary_expr') { - traverse(node.left); - traverse(node.right); + if ('left' in node) { + traverse(node.left); + } + if ('right' in node) { + traverse(node.right); + } tryOptimize(node); } else if (node.type === 'expr_list' && Array.isArray(node.value)) { node.value.forEach(traverse); diff --git a/packages/common-utils/src/queryParser.ts b/packages/common-utils/src/queryParser.ts index fa2de6dd5c..595a334dda 100644 --- a/packages/common-utils/src/queryParser.ts +++ b/packages/common-utils/src/queryParser.ts @@ -1124,14 +1124,11 @@ export async function buildKvItemsLookup({ continue; } - const parsed = (() => { - let parsed: { mapColumn: string; separator: string } | undefined; - for (const strategy of KV_ITEMS_STRATEGIES) { - parsed = strategy(candidate.default_expression); - if (parsed) break; - } - return parsed; - })(); + let parsed: { mapColumn: string; separator: string } | undefined; + for (const strategy of KV_ITEMS_STRATEGIES) { + parsed = strategy(candidate.default_expression); + if (parsed) break; + } if (!parsed) continue; const candidateName = normalizeChExpression(candidate.name);