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

[alex-fedotyev]

Summary

mergePath in packages/app/src/utils.ts 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.

The function now accepts a mapColumns argument alongside jsonColumns. For Map-typed parents, all sub-keys render as string-key subscripts (Map['1']) regardless of whether the key looks numeric. Array columns keep their existing 1-based array-index behavior.

Confirmed impact

I traced the original report back to production session data. Two sessions in the 10-day window covered by HDX-4369 hit the exact ClickHouse error above. Both stack traces resolve to the ClickHouse query client in the chunked _app-*.js bundle, consistent with the bug class: an autocomplete / sidebar interaction on the search page produces a query that subscripts a Map column with a numeric token.

Reproduction patterns observed:

• Search view with no where clause set, user clicks an attribute in the filter sidebar; autocomplete fetch fires and ClickHouse rejects with the arrayElement type error.
• Search view filtered by TraceId = '...' with a Map-bracket field in the selected columns; same error.

Low absolute frequency in the sampled window, but the failure class matches a broader pattern of user-reported "grid expand keeps failing" symptoms, so the cheap fix is worth shipping. Full per-session detail (timestamps, user identities, session IDs, source IDs, exact URLs) is on the Linear ticket.

Adjacent failures in the same RUM window look superficially similar but are NOT addressed by this PR: two cases of subscripting a String column (e.g. SomeStringCol['...']) and one case of subscripting a JSON column. Different bug class, different fix; flagging for context.

What this PR changes

mergePath now accepts a third mapColumns argument. The three direct call sites thread it through:

• useAutoCompleteOptions derives mapColumns from the field list via the new deriveMapColumnsFromFields helper, matching on the canonical JSDataType.Map so wrapped Map types (LowCardinality(Map(...)), Nullable(Map(...))) are detected too.
• DBRowJsonViewer accepts an optional mapColumns prop, threaded by DBRowDataPanel and DBRowOverviewPanel from the result-set metadata via a new getMapColumnNames helper.
• DBSearchPageFilters uses a new useMapColumns hook (mirrors useJsonColumns).

The same-bug-class sibling in DBSearchPageFilters/utils.ts (toClickHouseKeyExpression, the "Load more" SQL path) is fixed in this PR by threading the parsed base name as mapColumns into mergePath. parseMapFieldName already proves the base column is a Map.

mergePath's Map and string-key branches now escape backslash and single-quote before interpolating into single-quoted SQL strings, so a Map sub-key like it's produces Map['it\'s'] instead of malformed SQL.

The deep-review pass on the previous revision flagged a residual leak through buildJSONExtractQuery. When a Map column sub-value is itself a JSON-parseable string, HyperJson promotes the child context to isInParsedJson=true with parsedJsonRootPath=[MapCol, key]. The inner mergePath call in buildJSONExtractQuery did not receive mapColumns, so a numeric sub-key fell through the default branch and re-introduced Map[N+1]. The latest commit threads mapColumns through buildJSONExtractQuery and its four call sites in DBRowJsonViewer (Add to Filters, Search, Chart, Toggle Column).

Explained test case for the reviewer

The shortest path from "this PR's claim" to "this PR's fix" is one test in packages/app/src/__tests__/utils.test.ts:

it("emits Map['1'] for a numeric segment under a Map column", () => {
  expect(mergePath(['LogAttributes', '1'], [], ['LogAttributes']))
    .toBe("LogAttributes['1']");
});

On main this test fails with LogAttributes[2]. That's the exact subscript that produced the ClickHouse error in the production sessions. After this PR the test passes because mergePath consults mapColumns (the third argument) and renders the string-key subscript instead of the array-index subscript.

The same shape applies to the other layers:

// DBSearchPageFilters/utils.test.ts: the "Load more" SQL path
expect(toClickHouseKeyExpression('LogAttributes.1', 'String'))
  .toBe("LogAttributes['1'] AS \"LogAttributes.1\"");

// DBRowJsonViewer.test.tsx: the JSON-extract path through a Map column
expect(buildJSONExtractQuery(
  ['LogAttributes', '1', 'foo'],
  ['LogAttributes', '1'],
  [], 'JSONExtractString',
  ['LogAttributes'],
)).toBe("JSONExtractString(LogAttributes['1'], 'foo')");

Each one is a regression test for a specific surface where a Map column meets a numeric-looking sub-key. If the threading drops at any call site, the corresponding test trips.

Test plan

• New tests in packages/app/src/__tests__/utils.test.ts covering:
  • The failing repro from the issue body: mergePath(['LogAttributes', '1'], [], ['LogAttributes']) returns LogAttributes['1'], not LogAttributes[2]
  • Non-numeric Map sub-key (unchanged behavior)
  • Multi-segment Map path
  • Array column with numeric key (inverse: unchanged behavior)
  • JSON-vs-Map precedence
  • Default (Array / unknown column) cases unchanged
  • SQL escaping for single quote, backslash, and both combined (Map and default branches)
• New tests in packages/app/src/components/DBSearchPageFilters/utils.test.ts covering:
  • Numeric-looking Map sub-key on the "Load more" path: LogAttributes.1 -> LogAttributes['1']
  • Multi-segment property path starting with a numeric segment
• New tests in packages/app/src/hooks/__tests__/useAutoCompleteOptions.test.tsx covering deriveMapColumnsFromFields:
  • Top-level Map columns
  • Wrapped Map types via jsType (LowCardinality(Map(...)), Nullable(Map(...)))
  • Nested fields excluded
  • Non-Map columns excluded
  • Undefined and empty inputs
• New tests in packages/app/src/components/DBRowJsonViewer.test.tsx covering buildJSONExtractQuery with mapColumns:
  • Numeric Map sub-key holding parsed JSON: emits JSONExtractString(LogAttributes['1'], 'foo')
  • Deeply nested numeric Map sub-key
  • Non-numeric Map sub-key (regression)
  • Empty mapColumns falls back to array-index (regression, covers Array(JSON))
• yarn jest for affected files (208 passed)
• make ci-lint clean
• yarn tsc --noEmit clean
• Reproduction confirmed against the production session data referenced in HDX-4369

Deferred follow-up

packages/api/src/controllers/ai.ts has three mergePath calls with the same bug class on the AI Summary / AI Assistant backend path. Fixing it crosses into a second package and would push this PR to Tier 3, so I split it out: #2367 (tracked as HDX-4402).

Fixes HDX-4369.

[changeset-bot]

🦋 Changeset detected

Latest commit: f4a1ac7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@hyperdx/app	Patch
@hyperdx/api	Patch
@hyperdx/otel-collector	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
hyperdx-oss	Ready	Preview, Comment	May 29, 2026 2:52pm
hyperdx-storybook	Error		May 29, 2026 2:52pm

[github-actions]

🔵 Tier 2 — Low Risk

Small, isolated change with no API route or data model modifications.

Why this tier:

• Standard feature/fix — introduces new logic or modifies core functionality

Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns.
SLA: Resolve within 4 business hours.

Stats

• Production files changed: 8
• Production lines changed: 203 (+ 304 in test files, excluded from tier calculation)
• Branch: alex/HDX-4369-mergepath-map-subscript
• Author: alex-fedotyev

To override this classification, remove the review/tier-2 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

[github-actions]

E2E Test Results

✅ All tests passed • 191 passed • 3 skipped • 1234s

Status	Count
✅ Passed	191
❌ Failed	0
⚠️ Flaky	4
⏭️ Skipped	3

Tests ran across 4 shards in parallel.

View full report →

[github-actions]

Deep Review

✅ No critical issues found. The Map-column branch in mergePath, the escapeSqlSingleQuoted helper, and the four threaded call sites all line up; tests pin each layer with exact-match assertions including SQL-escape edge cases.

🟡 P2 -- recommended

• packages/app/src/components/DBRowJsonViewer.tsx:62 -- buildJSONExtractQuery interpolates nestedPath segments (user-controlled log/trace attribute keys) into a single-quoted ClickHouse string literal with nestedPath.map(p => \'${p}'`)and never calls theescapeSqlSingleQuotedhelper the PR just introduced one line above, so a key containing'or` produces malformed or injectable SQL on the Add-to-Filters, Search, Chart, and Toggle Column actions. Pre-existing pattern, but the PR added the helper for exactly this class of issue and the function was already being modified.
  • Fix: Apply escapeSqlSingleQuoted to each nestedPath segment before single-quote wrapping and export it from utils.ts.
  • _{security, correctness}
• packages/app/src/hooks/useMetadata.tsx:133 -- New useMapColumns hook is uncovered: the JSDataType.Map filter, the four-field enabled guard, and the if (!tableConnection) return [] fallback are all load-bearing for the DBSearchPageFilters Map fix and a regression here would silently re-introduce the Map[N+1] SQL on the filter sidebar without breaking any test.
  • Fix: Add unit tests for useMapColumns covering the JSDataType.Map filter and the undefined-connection fallback (mirroring whatever shape future useJsonColumns coverage would take).
  • _testing

🔵 P3 nitpicks (5)

• packages/app/src/components/DBRowDataPanel.tsx:209 -- getMapColumnNames does a raw type-string match (type === 'Map' || type.startsWith('Map(')) and misses LowCardinality(Map(...)) / Nullable(Map(...)), while the sibling deriveMapColumnsFromFields uses canonical JSDataType.Map for exactly that reason. Matches the existing getJSONColumnNames precedent and ClickHouse rarely wraps Map at storage, so no demonstrated failure mode; the inconsistency between three Map-detection paths is the smell.
  • Fix: Route through convertCHDataTypeToJSType (or strip LowCardinality(...) / Nullable(...) before the prefix check) so all three helpers share one detection contract.
  • _{correctness, kieran-typescript}
• packages/app/src/components/__tests__/DBRowDataPanel.test.ts:16-23 -- The block comment above describe('getMapColumnNames') narrates a transient PR/merge state (OSS #2357, origin/main, HEAD, "which branch introduced the symbol") that becomes meaningless once the PR lands and the merge commit is normal history.
  • Fix: Trim to the regression rationale (HDX-4369, bare Map vs Map(K,V) matching) and match the terse style of the adjacent getJSONColumnNames describe.
  • _{kieran-typescript}
• packages/app/src/components/DBSearchPageFilters/utils.ts:159 -- toClickHouseKeyExpression unconditionally forces parsed.baseName into mapColumns, but the inline comment's claim that parseMapFieldName "guarantees" a Map base is a caller-convention assertion, not a code-enforced one; a JSON-column dot-form key without backticks (possible after a Lucene URL round-trip) emits Body['foo'] instead of the JSON accessor. Same string was emitted pre-PR for non-numeric keys, so not a regression.
  • Fix: Thread jsonColumns through toClickHouseKeyExpression and pass it to mergePath, or weaken the comment to a caller contract.
  • _correctness
• packages/app/src/components/DBRowDataPanel.tsx:248 -- Neither the panel test, the JSON-viewer test, nor DBSearchPageFilters.tsx:1242 has an assertion that the derived mapColumns is actually threaded through to DBRowJsonViewer / mergePath; the four prop-pass sites (DBRowDataPanel, DBRowOverviewPanel x2, DBSearchPageFilters) could silently drop the third argument without failing CI.
  • Fix: Add a render-tree or hook-level assertion that the threaded mapColumns value reaches the consumer (e.g., spy on mergePath or render with a Map-typed meta payload).
  • _testing
• packages/app/src/components/DBRowJsonViewer.tsx:42-49 -- buildJSONExtractQuery is now five positional parameters with three defaults, so three of the four updated call sites in this file pass 'JSONExtractString' as a literal solely to reach the trailing mapColumns argument.
  • Fix: Convert the trailing arguments to an options bag ({ jsonColumns, jsonExtractFn, mapColumns }) so call sites only pass what they override.
  • _{kieran-typescript}

---

Reviewers (4): correctness, security, kieran-typescript, testing.

Testing gaps:

• useMapColumns is fully untested.
• No render-tree test pins the mapColumns prop pass-through in DBRowDataPanel / DBRowOverviewPanel or the threading in DBSearchPageFilters.
• escapeSqlSingleQuoted is tested for ' and \ only; if buildJSONExtractQuery is rerouted through it, a key with both characters from a real attribute name would also need a test.

[alex-fedotyev]

Pushed a584d181 addressing the deep-review feedback. Summary of what's in this revision:

Same-bug-class sibling in toClickHouseKeyExpression: the "Load more" SQL path now threads parsed.baseName as mapColumns into mergePath. parseMapFieldName already proves the base is a Map, so this is a one-line addition. Two new test cases in DBSearchPageFilters/utils.test.ts cover numeric-looking Map sub-keys (LogAttributes.1 and LogAttributes.42.foo).

SQL escaping in mergePath: the Map and default-branch string-key arms now escape backslash and single-quote via a small escapeSqlSingleQuoted helper. Five new test cases in utils.test.ts cover it's, back\slash, both combined, the default branch, and the no-op sanity case on numeric segments.

useAutoCompleteOptions Map detection: switched from raw type-string matching (which missed LowCardinality(Map(...)) and Nullable(Map(...))) to the canonical f.jsType === JSDataType.Map. Extracted deriveMapColumnsFromFields as an exported helper so a regression here trips a unit test. Five new test cases cover wrapped Map types, nested-field exclusion, non-Map exclusion, and undefined/empty inputs.

buildJSONExtractQuery invariant: added a comment explaining why this mergePath call intentionally omits mapColumns (gated on isInParsedJson so the root is always a JSON column). No behavior change.

Deferred: the three mergePath calls in packages/api/src/controllers/ai.ts:116-132 are the same bug class on the AI backend path. Threading mapColumns there crosses into a second package and would push this PR to Tier 3, so I split it out as #2367.

Verification:

• yarn jest for affected files: 182 passed
• make ci-lint: clean
• yarn tsc --noEmit: clean
• Predicted tier: still Tier 2 (192 prod lines, 252 test lines, single-layer)

[alex-fedotyev]

Pushed f4a1ac74. Two things in this revision:

1. Closed the deep-review P2 on buildJSONExtractQuery. The reviewer flagged that HDX-4369 still reproduces through the parsed-JSON path: when a Map column has a sub-value that is itself a JSON-parseable string, HyperJson promotes the child to isInParsedJson=true with parsedJsonRootPath=[MapCol, key], and the inner mergePath call did not receive mapColumns. A numeric sub-key fell through to Map[N+1]. I confirmed the leak with a node repro:

Map[1] with JSON child:  JSONExtractString(LogAttributes[2], 'foo')      // broken
Map[42][bar][baz]:       JSONExtractString(LogAttributes[43], 'bar', ...) // broken
Map[config] (control):   JSONExtractString(LogAttributes['config'], 'foo') // ok, non-numeric key

buildJSONExtractQuery now takes a mapColumns parameter; the four call sites in DBRowJsonViewer (Add to Filters, Search, Chart, Toggle Column) thread it through. The stale comment that claimed the gate guaranteed a JSON column was rewritten to describe what parsedJsonRootPath[0] actually is.

Four new tests in DBRowJsonViewer.test.tsx pin the behavior: numeric Map sub-key, deeply nested numeric Map sub-key, non-numeric Map sub-key (regression), and empty mapColumns (regression for the Array(JSON) case).

2. PR body now carries the production-impact context. I traced HDX-4369 back to its production session data; the patterns match a sidebar-attribute click on the search view and a TraceId-filtered search with a Map-bracket field in the selected columns. Both stacks resolve to the ClickHouse query client in the chunked _app-*.js bundle, consistent with the bug class. The PR body now has the high-level impact summary, an "explained test case" walkthrough so the reviewer doesn't have to map test names to surfaces, and the deferred AI controller follow-up (#2367 / HDX-4402) is restated. Full per-session detail (timestamps, identities, session IDs, source IDs) is on the Linear ticket.

Verification on this revision:

• yarn jest on the five affected suites: 208 passed
• make ci-lint: clean
• yarn tsc --noEmit: clean
• Predicted tier: still Tier 2 (still single-layer, prod-line count unchanged)