fix(common-utils): keep string literals verbatim in KV items rewrite

.changeset/kv-items-literal-escaping.md

@@ -0,0 +1,5 @@
+---
+'@hyperdx/common-utils': patch
+---
+
+fix: Keep string literals as written when rewriting `Map['key'] = 'value'` filters to the key-value items index, so values containing quotes or backslashes match instead of returning no rows

packages/common-utils/src/__tests__/kvItemsRewrite.int.test.ts

@@ -0,0 +1,147 @@
+import { ClickHouseClient, createClient } from '@clickhouse/client';
+import SqlString from 'sqlstring';
+
+import { parameterizedQueryToSql } from '@/clickhouse';
+import { ClickhouseClient as HdxClickhouseClient } from '@/clickhouse/node';
+import { Metadata, MetadataCache } from '@/core/metadata';
+import { renderChartConfig } from '@/core/renderChartConfig';
+import { ChartConfigWithOptDateRange } from '@/types';
+
+// Runs the Map['k'] = 'v' -> has(items, concat(...)) rewrite against a real
+// ClickHouse table, so a literal that is escaped differently from the source
+// SQL shows up as a missing row rather than a string mismatch in a unit test.
+describe('KV items rewrite integration', () => {
+  let client: ClickHouseClient;
+  let hdxClient: HdxClickhouseClient;
+  let metadata: Metadata;
+
+  const DATABASE = 'default';
+  const TABLE_NAME = 'kv_items_rewrite_int_test';
+
+  const ROWS: Array<{ Id: number; LogAttributes: Record<string, string> }> = [
+    { Id: 1, LogAttributes: { msg: 'plain' } },
+    { Id: 2, LogAttributes: { msg: 'say "hi"' } },
+    { Id: 3, LogAttributes: { msg: "it's" } },
+    { Id: 4, LogAttributes: { msg: 'C:\\temp\\x' } },
+    { Id: 5, LogAttributes: { msg: 'line1\nline2' } },
+    {
+      Id: 6,
+      LogAttributes: {
+        msg: '["useMetadata.useTableMetadata",{"connectionId":"6a0c3937cc01bdc90179f030"}] data is undefined',
+      },
+    },
+    { Id: 7, LogAttributes: { "k'y": 'v' } },
+  ];
+
+  const msgOf = (id: number) => ROWS.find(r => r.Id === id)!.LogAttributes.msg;
+
+  const queryIds = async (where: string) => {
+    const config: ChartConfigWithOptDateRange = {
+      connection: 'test-connection',
+      from: { databaseName: DATABASE, tableName: TABLE_NAME },
+      select: 'Id',
+      where,
+      whereLanguage: 'sql',
+      timestampValueExpression: 'Timestamp',
+      dateRange: [new Date('2025-01-01'), new Date('2025-01-02')],
+      orderBy: 'Id',
+    };
+    const sql = parameterizedQueryToSql(
+      await renderChartConfig(config, metadata, undefined),
+    );
+    const result = await client.query({ query: sql, format: 'JSONEachRow' });
+    const rows = await result.json<{ Id: number }>();
+    return { sql, ids: rows.map(r => Number(r.Id)) };
+  };
+
+  beforeAll(async () => {
+    const host = process.env.CLICKHOUSE_HOST || 'http://localhost:8123';
+    const username = process.env.CLICKHOUSE_USER || 'default';
+    const password = process.env.CLICKHOUSE_PASSWORD || '';
+
+    client = createClient({ url: host, username, password });
+    hdxClient = new HdxClickhouseClient({ host, username, password });
+
+    await client.command({
+      query: `CREATE OR REPLACE TABLE ${DATABASE}.${TABLE_NAME} (
+        Timestamp DateTime64(9) CODEC(Delta(8), ZSTD(1)),
+        Id UInt32,
+        LogAttributes Map(LowCardinality(String), String) CODEC(ZSTD(1)),
+        LogAttributeItems Array(String) MATERIALIZED
+          arrayMap((x) -> concat(x.1, '=', x.2), LogAttributes::Array(Tuple(String, String)))
+          CODEC(ZSTD(1)),
+        INDEX idx_log_attr_items LogAttributeItems TYPE text(tokenizer = 'array') GRANULARITY 1
+      )
+      ENGINE = MergeTree
+      ORDER BY (Timestamp)`,
+    });
+
+    await client.insert({
+      table: `${DATABASE}.${TABLE_NAME}`,
+      values: ROWS.map(r => ({ ...r, Timestamp: '2025-01-01 12:00:00' })),
+      format: 'JSONEachRow',
+    });
+  });
+
+  beforeEach(() => {
+    metadata = new Metadata(hdxClient, new MetadataCache());
+  });
+
+  afterAll(async () => {
+    await client.command({
+      query: `DROP TABLE IF EXISTS ${DATABASE}.${TABLE_NAME}`,
+    });
+    await hdxClient.close();
+    await client.close();
+  });
+
+  it.each([
+    ['plain value', 1],
+    ['double quotes', 2],
+    ['single quote', 3],
+    ['backslashes', 4],
+    ['newline', 5],
+    ['JSON with escaped quotes', 6],
+  ])('finds the row for Map[key] = value with %s', async (_, id) => {
+    const literal = SqlString.escape(msgOf(id));
+    const rewritten = await queryIds(`LogAttributes['msg'] = ${literal}`);
+    // arrayElement() is not rewritten, so it shows what the filter should match.
+    const baseline = await queryIds(
+      `arrayElement(LogAttributes, 'msg') = ${literal}`,
+    );
+
+    expect(rewritten.sql).toContain('has(`LogAttributeItems`');
+    expect(baseline.sql).not.toContain('LogAttributeItems');
+    expect(rewritten.ids).toEqual([id]);
+    expect(baseline.ids).toEqual([id]);
+  });
+
+  it('finds the row for a doubled single quote in the value', async () => {
+    const { sql, ids } = await queryIds(`LogAttributes['msg'] = 'it''s'`);
+    expect(sql).toContain('has(`LogAttributeItems`');
+    expect(ids).toEqual([3]);
+  });
+
+  it('finds the rows for Map[key] IN (...) with escaped values', async () => {
+    const values = [2, 4, 6].map(id => SqlString.escape(msgOf(id)));
+    const { sql, ids } = await queryIds(
+      `LogAttributes['msg'] IN (${values.join(', ')})`,
+    );
+    expect(sql).toContain('hasAny(`LogAttributeItems`');
+    expect(ids).toEqual([2, 4, 6]);
+  });
+
+  it('finds the row when the map key contains an escaped quote', async () => {
+    const { sql, ids } = await queryIds(`LogAttributes['k\\'y'] = 'v'`);
+    expect(sql).toContain('has(`LogAttributeItems`');
+    expect(ids).toEqual([7]);
+  });
+
+  it('finds the row for a side panel rowWhere with JSON in an attribute', async () => {
+    const { sql, ids } = await queryIds(
+      String.raw`Timestamp = '2025-01-01 12:00:00' AND LogAttributes['msg']='[\"useMetadata.useTableMetadata\",{\"connectionId\":\"6a0c3937cc01bdc90179f030\"}] data is undefined'`,
+    );
+    expect(sql).toContain('has(`LogAttributeItems`');
+    expect(ids).toEqual([6]);
+  });
+});

packages/common-utils/src/__tests__/renderChartConfig.test.ts

@@ -1878,6 +1878,21 @@ describe('renderChartConfig', () => {
       expect(sql).not.toContain("LogAttributes['service.name'] = 'api'");
     });
 
+    it('keeps escaped quotes in a SQL `where` value as written', async () => {
+      stubKvItemsMetadata();
+      const literal = String.raw`'[\"x\"] it\'s'`;
+      const sql = parameterizedQueryToSql(
+        await renderChartConfig(
+          buildWhereConfig(`LogAttributes['error.message'] = ${literal}`),
+          mockMetadata,
+          querySettings,
+        ),
+      );
+      expect(sql).toContain(
+        `has(\`LogAttributeItems\`, concat('error.message', '=', ${literal}))`,
+      );
+    });
+
     it('rewrites `Map[key] IN (many)` in a SQL `where` to hasAny() on ClickHouse >= 26.5', async () => {
       stubKvItemsMetadata();
       const sql = parameterizedQueryToSql(

packages/common-utils/src/__tests__/rewriteSqlFilterWithKvItems.test.ts

@@ -426,6 +426,88 @@ describe('rewriteSqlFilterWithKvItems', () => {
     });
   });
 
+  describe('string literal escaping', () => {
+    // node-sql-parser keeps escape sequences undecoded in `.value`, so the
+    // rewrite must emit the original literal rather than re-escape it.
+    it.each([
+      ['backslash-escaped double quotes', String.raw`'[\"x\"]'`],
+      ['backslash-escaped single quote', String.raw`'it\'s'`],
+      ['doubled single quote', `'it''s'`],
+      ['escaped backslash', String.raw`'a\\b'`],
+    ])('keeps %s in the value as written', (_, literal) => {
+      expect(
+        rewriteSqlFilterWithKvItems(
+          `LogAttributes['k'] = ${literal}`,
+          defaultLookup,
+        ),
+      ).toBe(`has(\`LogAttributeItems\`, concat('k', '=', ${literal}))`);
+    });
+
+    it('keeps escaped characters in the map key as written', () => {
+      expect(
+        rewriteSqlFilterWithKvItems(
+          String.raw`LogAttributes['k\'y'] = 'v'`,
+          defaultLookup,
+        ),
+      ).toBe(
+        String.raw`has(` +
+          '`LogAttributeItems`' +
+          String.raw`, concat('k\'y', '=', 'v'))`,
+      );
+    });
+
+    it('keeps escaped values in a hasAny IN rewrite', () => {
+      expect(
+        rewriteSqlFilterWithKvItems(
+          String.raw`LogAttributes['k'] IN ('a\"b', 'c''d')`,
+          defaultLookup,
+        ),
+      ).toBe(
+        'hasAny(`LogAttributeItems`, array(' +
+          String.raw`concat('k', '=', 'a\"b'), concat('k', '=', 'c''d')))`,
+      );
+    });
+
+    it('keeps escaped values in the legacy has OR has IN rewrite', () => {
+      expect(
+        rewriteSqlFilterWithKvItems(
+          String.raw`LogAttributes['k'] IN ('a\"b', 'c\\d')`,
+          legacyLookup,
+        ),
+      ).toBe(
+        '(has(`LogAttributeItems`, ' +
+          String.raw`concat('k', '=', 'a\"b')) OR ` +
+          'has(`LogAttributeItems`, ' +
+          String.raw`concat('k', '=', 'c\\d')))`,
+      );
+    });
+
+    it('rewrites a side panel rowWhere with JSON in an attribute value', () => {
+      const lookup = makeLookup([
+        [
+          'SpanAttributes',
+          { columnName: 'SpanAttributeItems', separator: '=' },
+        ],
+      ]);
+      const message = String.raw`'[\"useMetadata.useTableMetadata\",{\"connectionId\":\"6a0c3937cc01bdc90179f030\"}] data is undefined'`;
+      const result = rewriteSqlFilterWithKvItems(
+        "Timestamp=parseDateTime64BestEffort('2026-10-28T13:50:49.762000000Z', 9) " +
+          "AND ServiceName='chc-hdx-app-prod' " +
+          "AND SpanAttributes['error.object']='Error' " +
+          `AND SpanAttributes['error.message']=${message} ` +
+          "AND SpanName='console.error'",
+        lookup,
+      );
+      expect(result).toContain(
+        `has(\`SpanAttributeItems\`, concat('error.message', '=', ${message}))`,
+      );
+      expect(result).toContain(
+        "has(`SpanAttributeItems`, concat('error.object', '=', 'Error'))",
+      );
+      expect(result).not.toContain('\\\\');
+    });
+  });
+
   describe('indexHint', () => {
     // The LLM dashboard gates pair a value term with indexHint(a OR b), which
     // is the first caller to put a bare boolean OR in an argument position.

packages/common-utils/src/core/renderChartConfig.ts

@@ -376,18 +376,15 @@ const fastifySQL = ({
   }
 };
 
-function generateHasSqlForKvItemsColumn(
-  column: string,
-  key: string,
+// keySql/valueSql are SQL literals taken verbatim from the parsed condition.
+// The parser's `.value` keeps escape sequences undecoded, so re-escaping it
+// with SqlString would double-escape quotes and backslashes.
+function kvItemsConcatSql(
+  keySql: string,
   separator: string,
-  value: string,
+  valueSql: string,
 ): string {
-  return SqlString.format('has(??, concat(?, ?, ?))', [
-    column,
-    key,
-    separator,
-    value,
-  ]);
+  return `concat(${keySql}, ${SqlString.format('?', [separator])}, ${valueSql})`;
 }
 
 export const rewriteSqlFilterWithKvItems = (
@@ -432,11 +429,11 @@ export const rewriteSqlFilterWithKvItems = (
       ) {
         return;
       }
-      const mapKey: string = idxNode.value;
+      const keySql = parser.exprToSQL(idxNode);
       const info = textIndexInfoLookup.get(mapColumn)?.kv;
       if (!info) return;
 
-      let values: string[];
+      const valueNodes: { value: string }[] = [];
       if (op === '=') {
         const right = node.right;
         if (
@@ -445,57 +442,46 @@ export const rewriteSqlFilterWithKvItems = (
         ) {
           return;
         }
-        values = [right.value];
+        valueNodes.push(right);
       } 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);
+          valueNodes.push(item);
         }
-        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;
+      if (valueNodes.length === 0 || valueNodes.some(v => v.value === '')) {
+        return;
+      }
+      const valueSqls = valueNodes.map(v => parser.exprToSQL(v));
+
+      const columnSql = SqlString.format('??', [info.columnName]);
+      const concatSql = (valueSql: string) =>
+        kvItemsConcatSql(keySql, info.separator, valueSql);
 
       let replacement: string;
-      if (values.length === 1) {
-        replacement = generateHasSqlForKvItemsColumn(
-          info.columnName,
-          mapKey,
-          info.separator,
-          values[0],
-        );
+      if (valueSqls.length === 1) {
+        replacement = `has(${columnSql}, ${concatSql(valueSqls[0])})`;
       } else if (info.useHasAny) {
         // ClickHouse >= 26.5 supports `hasAny` over the direct_read map items
         // column in a single call.
-        replacement = `hasAny(${SqlString.format('??', [
-          info.columnName,
-        ])}, array(${values
-          .map(v =>
-            SqlString.format('concat(?, ?, ?)', [mapKey, info.separator, v]),
-          )
+        replacement = `hasAny(${columnSql}, array(${valueSqls
+          .map(concatSql)
           .join(', ')}))`;
       } else {
         // Backport branches (26.2/26.3/26.4) support `has` but not `hasAny` over
         // the items column, so we fall back to a chain of `has(...) OR ...`.
-        replacement = `(${values
-          .map(v =>
-            generateHasSqlForKvItemsColumn(
-              info.columnName,
-              mapKey,
-              info.separator,
-              v,
-            ),
-          )
+        replacement = `(${valueSqls
+          .map(v => `has(${columnSql}, ${concatSql(v)})`)
           .join(' OR ')})`;
       }