fix: token rendering for *AttributeItems columns

.changeset/selfish-icons-switch.md

@@ -0,0 +1,5 @@
+---
+"@hyperdx/common-utils": patch
+---
+
+fix: fixes the functions used to trigger direct_read

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

@@ -1345,7 +1345,7 @@ describe('renderChartConfig', () => {
       );
     });
 
-    it('rewrites `Map[key] IN (many)` to hasAny(... array(...))', async () => {
+    it('rewrites `Map[key] IN (many)` to hasAny(... array(...)) on ClickHouse >= 26.5', async () => {
       stubKvItemsMetadata();
       const sql = parameterizedQueryToSql(
         await renderChartConfig(
@@ -1359,6 +1359,25 @@ describe('renderChartConfig', () => {
       );
     });
 
+    it('rewrites `Map[key] IN (many)` to a chain of has() ORs on older ClickHouse (< 26.5)', async () => {
+      stubKvItemsMetadata();
+      mockMetadata.getServerVersion = jest
+        .fn()
+        .mockResolvedValue([26, 4, 3, 37]);
+      const sql = parameterizedQueryToSql(
+        await renderChartConfig(
+          buildConfig("LogAttributes['k'] IN ('a', 'b', 'c')"),
+          mockMetadata,
+          querySettings,
+        ),
+      );
+      expect(sql).toContain("has(`LogAttributeItems`, concat('k', '=', 'a'))");
+      expect(sql).toContain("has(`LogAttributeItems`, concat('k', '=', 'b'))");
+      expect(sql).toContain("has(`LogAttributeItems`, concat('k', '=', 'c'))");
+      expect(sql).not.toContain('hasAny(');
+      expect(sql).not.toContain('array(concat(');
+    });
+
     it('leaves the condition unchanged when no KV items column exists', async () => {
       mockMetadata.getColumns = jest.fn().mockResolvedValue([
         {
@@ -1384,7 +1403,7 @@ describe('renderChartConfig', () => {
         ),
       );
       expect(sql).toContain("LogAttributes['k'] = 'v'");
-      expect(sql).not.toContain('has(');
+      expect(sql).not.toContain('has(`LogAttributeItems`');
     });
 
     it('does not rewrite when value is empty (Map[k]= preserves missing-key semantics)', async () => {
@@ -1397,7 +1416,7 @@ describe('renderChartConfig', () => {
         ),
       );
       expect(sql).toContain("LogAttributes['k'] = ''");
-      expect(sql).not.toContain('has(');
+      expect(sql).not.toContain('has(`LogAttributeItems`');
     });
 
     it('rewrites only the matching Map subscript in a compound AND condition', async () => {

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

@@ -1,14 +1,32 @@
 import { rewriteSqlFilterWithKvItems } from '@/core/renderChartConfig';
 import { KvItemsLookup } from '@/queryParser';
 
+type PartialKvItemsInfo = {
+  kvItemsColumn: string;
+  separator: string;
+  useHasAny?: boolean;
+};
+
 const makeLookup = (
-  entries: Array<[string, { kvItemsColumn: string; separator: string }]>,
-): KvItemsLookup => new Map(entries);
+  entries: Array<[string, PartialKvItemsInfo]>,
+): KvItemsLookup =>
+  new Map(entries.map(([k, v]) => [k, { useHasAny: true, ...v }]));
 
 const defaultLookup: KvItemsLookup = makeLookup([
   ['LogAttributes', { kvItemsColumn: 'LogAttributeItems', separator: '=' }],
 ]);
 
+const legacyLookup: KvItemsLookup = makeLookup([
+  [
+    'LogAttributes',
+    {
+      kvItemsColumn: 'LogAttributeItems',
+      separator: '=',
+      useHasAny: false,
+    },
+  ],
+]);
+
 describe('rewriteSqlFilterWithKvItems', () => {
   describe('early returns', () => {
     it('returns the condition verbatim when the lookup is empty', () => {
@@ -324,4 +342,84 @@ describe('rewriteSqlFilterWithKvItems', () => {
       );
     });
   });
+
+  describe('hasAny fallback (useHasAny: false)', () => {
+    it("still rewrites Map['key'] = 'value' to has(...)", () => {
+      const result = rewriteSqlFilterWithKvItems(
+        "LogAttributes['k'] = 'v'",
+        legacyLookup,
+      );
+      expect(result).toBe("has(`LogAttributeItems`, concat('k', '=', 'v'))");
+    });
+
+    it("still rewrites Map['key'] IN ('a') (single item) to has(...)", () => {
+      const result = rewriteSqlFilterWithKvItems(
+        "LogAttributes['k'] IN ('a')",
+        legacyLookup,
+      );
+      expect(result).toBe("has(`LogAttributeItems`, concat('k', '=', 'a'))");
+    });
+
+    it("rewrites Map['key'] IN ('a','b','c') to a chain of has(...) OR ...", () => {
+      const result = rewriteSqlFilterWithKvItems(
+        "LogAttributes['k'] IN ('a', 'b', 'c')",
+        legacyLookup,
+      );
+      expect(result).toContain(
+        "has(`LogAttributeItems`, concat('k', '=', 'a'))",
+      );
+      expect(result).toContain(
+        "has(`LogAttributeItems`, concat('k', '=', 'b'))",
+      );
+      expect(result).toContain(
+        "has(`LogAttributeItems`, concat('k', '=', 'c'))",
+      );
+      expect(result).not.toContain('hasAny(');
+      expect(result).not.toContain('array(');
+      const orCount = (result.match(/ OR /g) ?? []).length;
+      expect(orCount).toBeGreaterThanOrEqual(2);
+    });
+
+    it('preserves precedence when the fallback OR chain sits inside an AND (IN on the left)', () => {
+      const result = rewriteSqlFilterWithKvItems(
+        "LogAttributes['k'] IN ('a', 'b') AND Severity = 'error'",
+        legacyLookup,
+      );
+      expect(result).toContain(
+        "has(`LogAttributeItems`, concat('k', '=', 'a'))",
+      );
+      expect(result).toContain(
+        "has(`LogAttributeItems`, concat('k', '=', 'b'))",
+      );
+      expect(result).toContain("Severity = 'error'");
+      expect(result).not.toContain('hasAny(');
+      // The OR chain MUST be parenthesized so AND doesn't bind tighter than OR
+      // and cause the right-hand `AND Severity` to attach to only the last has().
+      expect(result).toMatch(
+        /\(has\([^)]+\)[^)]*\) OR has\([^)]+\)[^)]*\)\) AND /,
+      );
+    });
+
+    it('preserves precedence when the fallback OR chain sits inside an AND (IN on the right)', () => {
+      const result = rewriteSqlFilterWithKvItems(
+        "Severity = 'error' AND LogAttributes['k'] IN ('a', 'b')",
+        legacyLookup,
+      );
+      expect(result).toContain("Severity = 'error'");
+      expect(result).not.toContain('hasAny(');
+      // Same rationale as the mirror case above: precedence-sensitive parens.
+      expect(result).toMatch(
+        / AND \(has\([^)]+\)[^)]*\) OR has\([^)]+\)[^)]*\)\)/,
+      );
+    });
+
+    it('does not rewrite when any IN value is an empty string', () => {
+      const result = rewriteSqlFilterWithKvItems(
+        "LogAttributes['k'] IN ('a', '')",
+        legacyLookup,
+      );
+      expect(result).not.toContain('has(');
+      expect(result).not.toContain('hasAny(');
+    });
+  });
 });

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

@@ -343,6 +343,20 @@ const fastifySQL = ({
   }
 };
 
+function generateHasSqlForKvItemsColumn(
+  column: string,
+  key: string,
+  separator: string,
+  value: string,
+): string {
+  return SqlString.format('has(??, concat(?, ?, ?))', [
+    column,
+    key,
+    separator,
+    value,
+  ]);
+}
+
 export const rewriteSqlFilterWithKvItems = (
   condition: string,
   kvItemsLookup: KvItemsLookup,
@@ -419,25 +433,38 @@ export const rewriteSqlFilterWithKvItems = (
       // 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(?, ?, ?))', [
+      let replacement: string;
+      if (values.length === 1) {
+        replacement = generateHasSqlForKvItemsColumn(
+          info.kvItemsColumn,
+          mapKey,
+          info.separator,
+          values[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.kvItemsColumn,
+        ])}, array(${values
+          .map(v =>
+            SqlString.format('concat(?, ?, ?)', [mapKey, info.separator, v]),
+          )
+          .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.kvItemsColumn,
               mapKey,
               info.separator,
-              values[0],
-            ])
-          : `hasAny(${SqlString.format('??', [
-              info.kvItemsColumn,
-            ])}, array(${values
-              .map(v =>
-                SqlString.format('concat(?, ?, ?)', [
-                  mapKey,
-                  info.separator,
-                  v,
-                ]),
-              )
-              .join(', ')}))`;
+              v,
+            ),
+          )
+          .join(' OR ')})`;
+      }
 
       // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- astify returns union type, we expect Select
       const replAst = parser.astify(`${prefix}${replacement}`, {

packages/common-utils/src/queryParser.ts

@@ -9,7 +9,10 @@ import {
   extractInnerCHArrayJSType,
   JSDataType,
 } from '@/clickhouse';
-import { supportsDirectReadMap } from '@/core/clickhouseVersion';
+import {
+  isClickHouseVersionAtLeast,
+  supportsDirectReadMap,
+} from '@/core/clickhouseVersion';
 import {
   Metadata,
   parseKeyPath,
@@ -857,6 +860,14 @@ function renderArrayFieldExpression({
 export type KvItemsInfo = {
   kvItemsColumn: string;
   separator: string;
+  /**
+   * Whether the connected ClickHouse server supports `hasAny(items, array(...))`
+   * over the KV items column. `hasAny` on the direct_read map items column only
+   * ships in mainline 26.5+; earlier backport branches (26.2/26.3/26.4) support
+   * `has(...)` but not `hasAny(...)` and must fall back to a chain of `has()`
+   * calls combined with `OR`.
+   */
+  useHasAny: boolean;
 };
 
 /** Map from map column name to its KV items info */
@@ -1113,6 +1124,7 @@ export async function buildKvItemsLookup({
     ]);
 
     const directReadSupported = supportsDirectReadMap(serverVersion, isCloud);
+    const useHasAny = isClickHouseVersionAtLeast(serverVersion, [26, 5, 0, 0]);
 
     const kvItemsCandidates = columns.filter(
       c =>
@@ -1146,6 +1158,7 @@ export async function buildKvItemsLookup({
         lookup.set(parsed.mapColumn, {
           kvItemsColumn: candidate.name,
           separator: parsed.separator,
+          useHasAny,
         });
       }
     }
@@ -1536,10 +1549,9 @@ export class CustomSchemaSQLSerializerV2 extends SQLSerializer {
           ...(kvItemsInfo
             ? {
                 kvItemsExpression: {
-                  kvItemsColumn: kvItemsInfo.kvItemsColumn,
+                  ...kvItemsInfo,
                   mapColumn: prefixMatch.name,
                   mapKey: fieldPostfix,
-                  separator: kvItemsInfo.separator,
                 },
               }
             : {}),