fix(row-panel): emit Map['key'] subscript for Map columns in mergePath

.changeset/hdx-4369-mergepath-map-subscript.md

@@ -0,0 +1,23 @@
+---
+"@hyperdx/app": patch
+---
+
+fix(row-panel): mergePath now emits string-key subscripts for Map columns,
+preventing a crash when expanding rows with numeric-looking attribute keys
+
+`mergePath` converted numeric path segments to 1-based array subscripts
+(`[N+1]`) regardless of whether the parent column was a Map or an Array.
+On a `Map(String, String)` column this produced SQL like `LogAttributes[2]`,
+which ClickHouse rejects with `Illegal types of arguments:
+Map(String, String), UInt8 for function arrayElement`. The grid row
+"expand" view failed for any row whose attribute path included a
+numeric-looking key under a Map column.
+
+`mergePath` now accepts a `mapColumns` argument alongside `jsonColumns`.
+For Map-typed parents, sub-keys always render as string subscripts
+(`Map['1']`) regardless of whether the key looks numeric. The three
+callers (`useAutoCompleteOptions`, `DBRowJsonViewer` via the row panels,
+`DBSearchPageFilters`) now thread Map-column names from the source
+schema. A new `useMapColumns` hook mirrors `useJsonColumns`.
+
+Fixes HDX-4369.

packages/app/src/__tests__/utils.test.ts

@@ -14,6 +14,7 @@ import {
   getColorFromCSSToken,
   getMetricTableName,
   mapKeyBy,
+  mergePath,
   orderByStringToSortingState,
   parseTimestampToMs,
   sortingStateToOrderByString,
@@ -1205,6 +1206,116 @@ describe('formatDurationMsCompact', () => {
   });
 });
 
+describe('mergePath', () => {
+  describe('default (Array / unknown column)', () => {
+    it('returns the bare key for a single-segment path', () => {
+      expect(mergePath(['Body'])).toBe('Body');
+    });
+
+    it('numeric sub-segment becomes 1-based array index', () => {
+      // ClickHouse arrays are 1-based but flattened data uses 0-based indices.
+      expect(mergePath(['SomeArray', '0'])).toBe('SomeArray[1]');
+      expect(mergePath(['SomeArray', '4'])).toBe('SomeArray[5]');
+    });
+
+    it('non-numeric sub-segment becomes string-key subscript', () => {
+      expect(mergePath(['SomeColumn', 'service.name'])).toBe(
+        "SomeColumn['service.name']",
+      );
+    });
+
+    it('mixed numeric and string segments chain', () => {
+      expect(mergePath(['Outer', '1', 'inner'])).toBe("Outer[2]['inner']");
+    });
+  });
+
+  describe('JSON column', () => {
+    it('emits dotted backtick-quoted accessor', () => {
+      expect(mergePath(['BodyJson', 'service', 'name'], ['BodyJson'])).toBe(
+        'BodyJson.`service`.`name`',
+      );
+    });
+  });
+
+  describe('Map column (HDX-4369)', () => {
+    // Failing reproducer from the issue body: on a Map(String, String), a
+    // numeric-looking sub-key must NOT collapse into array-index syntax.
+    // ClickHouse rejects `LogAttributes[2]` against a Map column with
+    // "Illegal types of arguments: Map(String, String), UInt8 for function
+    // arrayElement". The fix adds a `mapColumns` parameter that forces the
+    // bracketed string-key form regardless of whether the key parses as a
+    // non-negative integer.
+    it('numeric sub-key on a Map renders as string subscript, not array index', () => {
+      const result = mergePath(['LogAttributes', '1'], [], ['LogAttributes']);
+      expect(result).not.toBe('LogAttributes[2]');
+      expect(result).not.toMatch(/\[\d+\]$/);
+      expect(result).toBe("LogAttributes['1']");
+    });
+
+    it('non-numeric Map sub-key keeps string subscript (unchanged)', () => {
+      expect(
+        mergePath(['LogAttributes', 'service.name'], [], ['LogAttributes']),
+      ).toBe("LogAttributes['service.name']");
+    });
+
+    it('multi-segment Map path chains string subscripts', () => {
+      expect(
+        mergePath(['LogAttributes', '1', 'foo'], [], ['LogAttributes']),
+      ).toBe("LogAttributes['1']['foo']");
+    });
+
+    it('Array column with numeric key still uses array-index syntax', () => {
+      // Inverse case: keep existing behavior for non-Map parents.
+      expect(mergePath(['SomeArray', '1'], [], ['LogAttributes'])).toBe(
+        'SomeArray[2]',
+      );
+    });
+
+    it('JSON column wins over Map column when both lists contain the key', () => {
+      // Caller can't currently configure the same column as both; the order
+      // is deterministic if they did.
+      expect(mergePath(['Body', '1'], ['Body'], ['Body'])).toBe('Body.`1`');
+    });
+  });
+
+  describe('SQL escaping of single quotes and backslashes', () => {
+    // Keys can contain user-controlled characters (Map sub-keys carry
+    // arbitrary text). An unescaped single quote produces malformed SQL like
+    // `Map['it's']`, which ClickHouse parses as the broken token sequence
+    // `Map['it']s']`. Backslash must escape first so the quote-escape
+    // backslash is not itself doubled.
+    it('escapes single quotes in Map sub-keys', () => {
+      expect(mergePath(['LogAttributes', "it's"], [], ['LogAttributes'])).toBe(
+        "LogAttributes['it\\'s']",
+      );
+    });
+
+    it('escapes backslashes in Map sub-keys', () => {
+      expect(
+        mergePath(['LogAttributes', 'back\\slash'], [], ['LogAttributes']),
+      ).toBe("LogAttributes['back\\\\slash']");
+    });
+
+    it('escapes a key containing both a backslash and a quote', () => {
+      expect(
+        mergePath(['LogAttributes', "a\\b'c"], [], ['LogAttributes']),
+      ).toBe("LogAttributes['a\\\\b\\'c']");
+    });
+
+    it('escapes single quotes in default-branch string subscripts', () => {
+      // The default Array / unknown column branch also takes string-key
+      // subscripts when the segment is non-numeric. Same escape applies.
+      expect(mergePath(['SomeColumn', "it's"])).toBe("SomeColumn['it\\'s']");
+    });
+
+    it('leaves numeric segments untouched in the default branch', () => {
+      // Numeric path collapses to bracketed integer index; escape is a
+      // no-op because Number.isInteger(asNumber) succeeds. Sanity check.
+      expect(mergePath(['SomeArray', '0'])).toBe('SomeArray[1]');
+    });
+  });
+});
+
 describe('getColorFromCSSToken', () => {
   afterEach(() => {
     jest.restoreAllMocks();

packages/app/src/components/DBRowDataPanel.tsx

@@ -200,6 +200,21 @@ export function getJSONColumnNames(meta: ResponseJSON['meta'] | undefined) {
   );
 }
 
+// Returns the names of Map-typed columns in the result metadata. Used by
+// `mergePath` to keep numeric-looking sub-keys on a Map(String, ...) from
+// collapsing into ClickHouse array-index syntax (`Map[2]`), which the
+// server rejects with
+// `Illegal types of arguments: Map(String, ...), UInt8 for function
+// arrayElement`. HDX-4369.
+export function getMapColumnNames(meta: ResponseJSON['meta'] | undefined) {
+  return (
+    meta
+      // Match both `Map(K, V)` and the bare `Map` (rare; defensive).
+      ?.filter(m => m.type === 'Map' || m.type.startsWith('Map('))
+      .map(m => m.name) ?? []
+  );
+}
+
 export function RowDataPanel({
   source,
   rowId,
@@ -222,11 +237,16 @@ export function RowDataPanel({
   }, [data]);
 
   const jsonColumns = getJSONColumnNames(data?.meta);
+  const mapColumns = getMapColumnNames(data?.meta);
 
   return (
     <div className="flex-grow-1 overflow-auto" data-testid={dataTestId}>
       <Box mx="md" my="sm">
-        <DBRowJsonViewer data={firstRow} jsonColumns={jsonColumns} />
+        <DBRowJsonViewer
+          data={firstRow}
+          jsonColumns={jsonColumns}
+          mapColumns={mapColumns}
+        />
       </Box>
     </div>
   );

packages/app/src/components/DBRowJsonViewer.test.tsx

@@ -292,5 +292,57 @@ describe('DBRowJsonViewer', () => {
         ),
       ).toBe("JSONExtractString(LogAttributes.`config`, 'host')");
     });
+
+    // HDX-4369. HyperJson promotes a Map sub-value that is itself a
+    // JSON-parseable string to `isInParsedJson=true` with
+    // parsedJsonRootPath=[MapCol, key] (see HyperJson.tsx:227-234). When that
+    // key is numeric, the inner `mergePath` used to emit `MapCol[N+1]` array
+    // syntax, which ClickHouse rejects with "Illegal types of arguments:
+    // Map(String, String), UInt8 for function arrayElement". Threading
+    // `mapColumns` keeps the Map[\'1\'] subscript.
+    it("emits Map['1'] for Map column with numeric sub-key holding parsed JSON", () => {
+      expect(
+        buildJSONExtractQuery(
+          ['LogAttributes', '1', 'foo'],
+          ['LogAttributes', '1'],
+          [], // jsonColumns
+          'JSONExtractString',
+          ['LogAttributes'], // mapColumns
+        ),
+      ).toBe("JSONExtractString(LogAttributes['1'], 'foo')");
+    });
+
+    it("emits Map['42'] for deeply nested Map column with numeric sub-key holding parsed JSON", () => {
+      expect(
+        buildJSONExtractQuery(
+          ['LogAttributes', '42', 'bar', 'baz'],
+          ['LogAttributes', '42'],
+          [],
+          'JSONExtractString',
+          ['LogAttributes'],
+        ),
+      ).toBe("JSONExtractString(LogAttributes['42'], 'bar', 'baz')");
+    });
+
+    it('keeps non-numeric Map sub-key unchanged when mapColumns is threaded', () => {
+      expect(
+        buildJSONExtractQuery(
+          ['LogAttributes', 'config', 'host'],
+          ['LogAttributes', 'config'],
+          [],
+          'JSONExtractString',
+          ['LogAttributes'],
+        ),
+      ).toBe("JSONExtractString(LogAttributes['config'], 'host')");
+    });
+
+    it('falls back to array index when mapColumns is empty (unchanged behavior)', () => {
+      // Without mapColumns, a numeric segment still gets the array-index
+      // treatment. This pins the pre-HDX-4369 default for the non-Map case
+      // (e.g. an Array(JSON) column whose element holds a parsed JSON value).
+      expect(
+        buildJSONExtractQuery(['SomeArray', '0', 'id'], ['SomeArray', '0']),
+      ).toBe("JSONExtractString(SomeArray[1], 'id')");
+    });
   });
 });

packages/app/src/components/DBRowJsonViewer.tsx

@@ -45,13 +45,20 @@ export function buildJSONExtractQuery(
   parsedJsonRootPath: string[],
   jsonColumns: string[] = [],
   jsonExtractFn: JSONExtractFn = 'JSONExtractString',
+  mapColumns: string[] = [],
 ): string | null {
   const nestedPath = keyPath.slice(parsedJsonRootPath.length);
   if (nestedPath.length === 0) {
     return null; // No nested path to extract
   }
 
-  const baseColumn = mergePath(parsedJsonRootPath, jsonColumns);
+  // `parsedJsonRootPath[0]` is the column the parsed-JSON view is anchored on.
+  // It can be a JSON column (auto-detected by ClickHouse JSON type) OR a Map
+  // column whose sub-value is a JSON-parseable string (HyperJson promotes those
+  // to `isInParsedJson=true`, see HyperJson.tsx:227). Thread `mapColumns` so a
+  // numeric-looking Map sub-key renders as `Map['1']` instead of the array
+  // `Map[2]`. See HDX-4369.
+  const baseColumn = mergePath(parsedJsonRootPath, jsonColumns, mapColumns);
   const jsonPathArgs = nestedPath.map(p => `'${p}'`).join(', ');
   return `${jsonExtractFn}(${baseColumn}, ${jsonPathArgs})`;
 }
@@ -333,9 +340,14 @@ function HyperJsonMenu({ rowData }: { rowData: any }) {
 export function DBRowJsonViewer({
   data,
   jsonColumns,
+  mapColumns,
 }: {
   data: any;
   jsonColumns?: string[];
+  // Map column names from the result-set metadata. Threaded into
+  // `mergePath` so numeric-looking sub-keys on a Map render as
+  // `Map['key']` instead of the array `Map[N+1]`. HDX-4369.
+  mapColumns?: string[];
 }) {
   const {
     onPropertyAddClick,
@@ -370,7 +382,7 @@ export function DBRowJsonViewer({
   const getLineActions = useCallback<GetLineActions>(
     ({ keyPath, value, isInParsedJson, parsedJsonRootPath }) => {
       const actions: LineAction[] = [];
-      const fieldPath = mergePath(keyPath, jsonColumns);
+      const fieldPath = mergePath(keyPath, jsonColumns, mapColumns);
 
       // Add to Filters action (strings only)
       // FIXME: TOTAL HACK To disallow adding timestamp to filters
@@ -398,6 +410,8 @@ export function DBRowJsonViewer({
                 keyPath,
                 parsedJsonRootPath,
                 jsonColumns,
+                'JSONExtractString',
+                mapColumns,
               );
               if (jsonQuery) {
                 filterFieldPath = jsonQuery;
@@ -442,6 +456,7 @@ export function DBRowJsonViewer({
                 parsedJsonRootPath,
                 jsonColumns,
                 jsonExtractFn,
+                mapColumns,
               );
 
               if (jsonQuery) {
@@ -485,6 +500,8 @@ export function DBRowJsonViewer({
                 keyPath,
                 parsedJsonRootPath,
                 jsonColumns,
+                'JSONExtractString',
+                mapColumns,
               );
               if (jsonQuery) {
                 chartFieldPath = jsonQuery;
@@ -512,6 +529,8 @@ export function DBRowJsonViewer({
             keyPath,
             parsedJsonRootPath,
             jsonColumns,
+            'JSONExtractString',
+            mapColumns,
           );
           if (jsonQuery) {
             columnFieldPath = jsonQuery;
@@ -621,6 +640,7 @@ export function DBRowJsonViewer({
       rowData,
       toggleColumn,
       jsonColumns,
+      mapColumns,
     ],
   );
 

packages/app/src/components/DBRowOverviewPanel.tsx

@@ -8,7 +8,11 @@ import { WithClause } from '@/hooks/useRowWhere';
 import { getEventBody } from '@/source';
 import { getHighlightedAttributesFromData } from '@/utils/highlightedAttributes';
 
-import { getJSONColumnNames, useRowData } from './DBRowDataPanel';
+import {
+  getJSONColumnNames,
+  getMapColumnNames,
+  useRowData,
+} from './DBRowDataPanel';
 import { DBRowJsonViewer } from './DBRowJsonViewer';
 import { RowSidePanelContext } from './DBRowSidePanel';
 import DBRowSidePanelHeader from './DBRowSidePanelHeader';
@@ -52,6 +56,7 @@ export function RowOverviewPanel({
   }, [source, data]);
 
   const jsonColumns = getJSONColumnNames(data?.meta);
+  const mapColumns = getMapColumnNames(data?.meta);
 
   const eventAttributesExpr =
     source.kind === SourceKind.Log || source.kind === SourceKind.Trace
@@ -282,6 +287,7 @@ export function RowOverviewPanel({
                 <DBRowJsonViewer
                   data={topLevelAttributes}
                   jsonColumns={jsonColumns}
+                  mapColumns={mapColumns}
                 />
               </Box>
             </Accordion.Panel>
@@ -300,6 +306,7 @@ export function RowOverviewPanel({
                 <DBRowJsonViewer
                   data={filteredEventAttributes}
                   jsonColumns={jsonColumns}
+                  mapColumns={mapColumns}
                 />
               </Box>
             </Accordion.Panel>