diff --git a/.changeset/sql-where-kv-direct-read.md b/.changeset/sql-where-kv-direct-read.md new file mode 100644 index 0000000000..ef1c2351a6 --- /dev/null +++ b/.changeset/sql-where-kv-direct-read.md @@ -0,0 +1,5 @@ +--- +'@hyperdx/common-utils': patch +--- + +Apply the Map KV text-index rewrite (`Map['k'] = 'v'` → `has(ItemsCol, concat('k', '=', 'v'))`, enabling ClickHouse's direct-read optimization) to SQL predicates in the top-level `where` (search box, saved searches, alerts) and to SQL `aggCondition`s copied into the WHERE clause — previously only `sql`-type `filters[]` entries were rewritten diff --git a/packages/common-utils/src/__tests__/renderChartConfig.test.ts b/packages/common-utils/src/__tests__/renderChartConfig.test.ts index 680a77dc80..f452c2c6ad 100644 --- a/packages/common-utils/src/__tests__/renderChartConfig.test.ts +++ b/packages/common-utils/src/__tests__/renderChartConfig.test.ts @@ -1489,6 +1489,139 @@ describe('renderChartConfig', () => { ); expect(sql).toContain("SeverityText = 'error'"); }); + + const buildWhereConfig = (where: string): ChartConfigWithOptDateRange => ({ + connection: 'test-connection', + from: { databaseName: 'default', tableName: 'otel_logs' }, + select: [{ aggFn: 'count', valueExpression: '' }], + where, + whereLanguage: 'sql', + timestampValueExpression: 'Timestamp', + dateRange: [new Date('2025-01-01'), new Date('2025-01-02')], + granularity: '1 minute', + }); + + it('rewrites `Map[key] = value` in a SQL `where` (search box path)', async () => { + stubKvItemsMetadata(); + const sql = parameterizedQueryToSql( + await renderChartConfig( + buildWhereConfig("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 (many)` in a SQL `where` to hasAny() on ClickHouse >= 26.5', async () => { + stubKvItemsMetadata(); + const sql = parameterizedQueryToSql( + await renderChartConfig( + buildWhereConfig("LogAttributes['k'] IN ('a', 'b')"), + mockMetadata, + querySettings, + ), + ); + expect(sql).toContain( + "hasAny(`LogAttributeItems`, array(concat('k', '=', 'a'), concat('k', '=', 'b')))", + ); + }); + + it('leaves a SQL `where` 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( + buildWhereConfig("LogAttributes['k'] = 'v'"), + mockMetadata, + querySettings, + ), + ); + expect(sql).toContain("LogAttributes['k'] = 'v'"); + expect(sql).not.toContain('has(`LogAttributeItems`'); + }); + + it('leaves a SQL `where` unchanged when the server predates direct_read support (ALIAS items column)', async () => { + stubKvItemsMetadata(); + // The stub's items column is MATERIALIZED (always eligible); switch it + // to ALIAS so the supportsDirectReadMap version gate applies. + mockMetadata.getColumns = jest.fn().mockResolvedValue([ + { + name: 'LogAttributes', + type: 'Map(String, String)', + default_type: '', + default_expression: '', + }, + { + name: 'LogAttributeItems', + type: 'Array(String)', + default_type: 'ALIAS', + default_expression: + "arrayMap((arr) -> concat(arr.1, '=', arr.2), LogAttributes::Array(Tuple(String, String)))", + }, + ]); + mockMetadata.getServerVersion = jest + .fn() + .mockResolvedValue([26, 1, 0, 0]); + + const sql = parameterizedQueryToSql( + await renderChartConfig( + buildWhereConfig("LogAttributes['k'] = 'v'"), + mockMetadata, + querySettings, + ), + ); + expect(sql).toContain("LogAttributes['k'] = 'v'"); + expect(sql).not.toContain('has(`LogAttributeItems`'); + }); + + it('rewrites a SQL aggCondition in the WHERE clause but not in the aggregate', async () => { + stubKvItemsMetadata(); + const config: ChartConfigWithOptDateRange = { + connection: 'test-connection', + from: { databaseName: 'default', tableName: 'otel_logs' }, + select: [ + { + aggFn: 'count', + aggCondition: "LogAttributes['service.name'] = 'api'", + aggConditionLanguage: 'sql', + valueExpression: '', + }, + ], + where: '', + whereLanguage: 'sql', + timestampValueExpression: 'Timestamp', + dateRange: [new Date('2025-01-01'), new Date('2025-01-02')], + granularity: '1 minute', + }; + const sql = parameterizedQueryToSql( + await renderChartConfig(config, mockMetadata, querySettings), + ); + // WHERE-clause copy is rewritten so the text index can prune granules... + expect(sql).toContain( + "has(`LogAttributeItems`, concat('service.name', '=', 'api'))", + ); + // ...while the countIf(...) copy keeps the plain Map subscript, which is + // cheaper to evaluate per-row than has() over an ALIAS items column. + expect(sql).toContain("countIf(LogAttributes['service.name'] = 'api')"); + }); }); describe('k8s semantic convention migrations', () => { diff --git a/packages/common-utils/src/core/renderChartConfig.ts b/packages/common-utils/src/core/renderChartConfig.ts index a2cd42ec14..fef4d0c7ce 100644 --- a/packages/common-utils/src/core/renderChartConfig.ts +++ b/packages/common-utils/src/core/renderChartConfig.ts @@ -1167,11 +1167,54 @@ async function renderWhere( chartConfig: BuilderChartConfigWithOptDateRangeEx, metadata: Metadata, ): Promise { + // The aggCondition-to-WHERE optimization below only kicks in when every + // select has an aggCondition (otherwise all rows are scanned anyways). + const aggConditionsInWhere = + typeof chartConfig.select != 'string' && + chartConfig.select.every(select => + isNonEmptyWhereExpr(select.aggCondition), + ); + + // The Map-KV text-index rewrite (`Map['k'] = 'v'` -> + // `has(ItemsCol, concat('k', '=', 'v'))`, enabling ClickHouse's direct-read + // optimization) applies to every SQL predicate that lands in the WHERE + // clause: the top-level `where`, `sql`-type filters, and aggConditions + // copied into WHERE. Build the lookup once, up front, when any of them is + // present. The underlying metadata calls are cached, and + // rewriteSqlFilterWithKvItems is a no-op on an empty lookup. + const hasSqlPredicate = + (isNonEmptyWhereExpr(chartConfig.where) && + (chartConfig.whereLanguage ?? 'sql') === 'sql') || + (chartConfig.filters?.some(f => f.type === 'sql') ?? false) || + (aggConditionsInWhere && + typeof chartConfig.select != 'string' && + chartConfig.select.some( + select => (select.aggConditionLanguage ?? 'sql') === 'sql', + )); + const textIndexInfoLookup: TextIndexInfoLookup = + hasSqlPredicate && + chartConfig.from.databaseName && + chartConfig.from.tableName && + !hasSubqueryCte(chartConfig.with) + ? await buildTextIndexInfoLookup({ + metadata, + databaseName: chartConfig.from.databaseName, + tableName: chartConfig.from.tableName, + connectionId: chartConfig.connection, + }) + : new Map(); + let whereSearchCondition: ChSql | [] = []; if (isNonEmptyWhereExpr(chartConfig.where)) { whereSearchCondition = wrapChSqlIfNotEmpty( await renderWhereExpression({ - condition: chartConfig.where, + condition: + (chartConfig.whereLanguage ?? 'sql') === 'sql' + ? rewriteSqlFilterWithKvItems( + chartConfig.where, + textIndexInfoLookup, + ) + : chartConfig.where, from: chartConfig.from, language: chartConfig.whereLanguage ?? 'sql', implicitColumnExpression: chartConfig.implicitColumnExpression, @@ -1188,18 +1231,26 @@ async function renderWhere( } let selectSearchConditions: ChSql[] = []; - if ( - typeof chartConfig.select != 'string' && - // Only if every select has an aggCondition, add to where clause - // otherwise we'll scan all rows anyways - chartConfig.select.every(select => isNonEmptyWhereExpr(select.aggCondition)) - ) { + if (aggConditionsInWhere && typeof chartConfig.select != 'string') { selectSearchConditions = ( await Promise.all( chartConfig.select.map(async select => { if (isNonEmptyWhereExpr(select.aggCondition)) { + // Only this WHERE-clause copy of the aggCondition is rewritten to + // the `has(...)` form — the aggFnIf(...) copy in the SELECT clause + // (renderSelectList) is deliberately left as a Map subscript. + // Index-based granule pruning only happens in WHERE; inside the + // aggregate the predicate is evaluated per-row, where + // `has(, ...)` would recompute the whole + // arrayMap per row and be slower than a plain Map subscript. return await renderWhereExpression({ - condition: select.aggCondition, + condition: + (select.aggConditionLanguage ?? 'sql') === 'sql' + ? rewriteSqlFilterWithKvItems( + select.aggCondition, + textIndexInfoLookup, + ) + : select.aggCondition, from: chartConfig.from, language: select.aggConditionLanguage ?? 'sql', implicitColumnExpression: chartConfig.implicitColumnExpression, @@ -1217,21 +1268,6 @@ async function renderWhere( ).filter(v => v !== null) as ChSql[]; } - const hasSqlFilter = - chartConfig.filters?.some(f => f.type === 'sql') ?? false; - const textIndexInfoLookup: TextIndexInfoLookup = - hasSqlFilter && - chartConfig.from.databaseName && - chartConfig.from.tableName && - !hasSubqueryCte(chartConfig.with) - ? await buildTextIndexInfoLookup({ - 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') {