fix: selecting an at-prefixed JSON key no longer breaks row details

.changeset/fix-at-prefixed-json-key-row-details.md

@@ -0,0 +1,5 @@
+---
+'@hyperdx/common-utils': patch
+---
+
+fix: keep JSON path segments with special characters quoted in row detail lookups

packages/app/tests/e2e/components/SidePanelComponent.ts

@@ -329,6 +329,27 @@ export class SidePanelComponent {
       .click({ timeout: this.defaultTimeout });
   }
 
+  /**
+   * The "Column Values" (parsed) tab's content panel, which renders the full
+   * fetched row via the JSON viewer.
+   */
+  get columnValuesContent() {
+    return this.panelContainer.getByTestId('side-panel-tab-parsed');
+  }
+
+  /** Open the "Column Values" (parsed) tab. */
+  async openColumnValues() {
+    await this.clickTab('parsed');
+  }
+
+  /**
+   * A line in the "Column Values" tab matching `text` (substring). Used to
+   * assert a fetched row's value is shown.
+   */
+  columnValue(text: string) {
+    return this.columnValuesContent.getByText(text, { exact: false });
+  }
+
   /**
    * Close the side panel (if it has a close button)
    */

packages/app/tests/e2e/features/search/json-at-prefixed-key-row-details.spec.ts

@@ -0,0 +1,73 @@
+/**
+ * Selecting an at-prefixed JSON key breaks row details.
+ *
+ * When a displayed SELECT column is a JSON subcolumn whose key is not a bare
+ * identifier (e.g. `ResourceAttributesJSON.`@request_id``), ClickHouse reports
+ * the result column name without backticks (`ResourceAttributesJSON.@request_id`).
+ * The row side panel rebuilds a WHERE clause from the displayed columns to fetch
+ * the full row; before the fix it used that bare name verbatim, producing
+ * invalid SQL that matched no rows, so the panel came up empty.
+ *
+ * This drives the real UI path against a JSON-column schema: select the
+ * at-prefixed key, open the row, and confirm the detail panel is populated with
+ * the selected event's values.
+ */
+import { SearchPage } from '../../page-objects/SearchPage';
+import {
+  INTERESTING_FILTER_KEYS_JSON_AT_KEY,
+  INTERESTING_FILTER_KEYS_ROWS,
+} from '../../seed-clickhouse';
+import { expect, test } from '../../utils/base-test';
+import { INTERESTING_FILTER_KEYS_SOURCE_NAME } from '../../utils/constants';
+
+const QUERY_TIMEOUT = 20_000;
+
+const [ROW1] = INTERESTING_FILTER_KEYS_ROWS;
+
+test.describe(
+  'Search: at-prefixed JSON key row details',
+  { tag: ['@search', '@full-stack'] },
+  () => {
+    test('opening a row with an at-prefixed JSON key in SELECT shows the event', async ({
+      page,
+    }) => {
+      const searchPage = new SearchPage(page);
+      await searchPage.goto();
+      await searchPage.selectSource(INTERESTING_FILTER_KEYS_SOURCE_NAME);
+      await searchPage.timePicker.selectRelativeTime('Last 1 hour');
+
+      await test.step('isolate a single row', async () => {
+        await searchPage.performSearch(`ServiceName:"${ROW1.serviceName}"`);
+        await expect(searchPage.getTableError()).toHaveCount(0);
+        await expect(searchPage.table.getRows()).toHaveCount(1, {
+          timeout: QUERY_TIMEOUT,
+        });
+      });
+
+      await test.step('select the at-prefixed JSON key as a column', async () => {
+        await searchPage.setCustomSELECT(
+          `Timestamp, ServiceName, ResourceAttributesJSON.\`${INTERESTING_FILTER_KEYS_JSON_AT_KEY}\``,
+        );
+        await searchPage.submitEmptySearch();
+        await expect(searchPage.getTableError()).toHaveCount(0);
+        await expect(searchPage.table.getRows()).toHaveCount(1, {
+          timeout: QUERY_TIMEOUT,
+        });
+      });
+
+      await test.step('row details load the selected event', async () => {
+        await searchPage.table.clickFirstRow();
+        await expect(searchPage.sidePanel.container).toBeVisible();
+        // Before the fix the detail WHERE dropped the key's backticks and
+        // matched no rows, so the panel rendered empty with no error.
+        await expect(searchPage.sidePanel.errorState).toHaveCount(0);
+
+        await searchPage.sidePanel.openColumnValues();
+        await expect(
+          searchPage.sidePanel.columnValue(ROW1.jsonAtKeyValue),
+        ).toBeVisible({ timeout: QUERY_TIMEOUT });
+        await expect(searchPage.sidePanel.columnValue(ROW1.body)).toBeVisible();
+      });
+    });
+  },
+);

packages/app/tests/e2e/seed-clickhouse.ts

@@ -98,6 +98,10 @@ export const INTERESTING_FILTER_KEYS_ROWS = [
     serviceName: 'service1',
     resourceAttrValue: 'value1',
     jsonValue: 'value1',
+    // Top-level JSON key that is NOT a bare identifier (`@` prefix). Selecting
+    // it (`ResourceAttributesJSON.`@request_id``) and opening the row must keep
+    // the key backtick-quoted in the detail WHERE clause (HDX-5867).
+    jsonAtKeyValue: 'request-A',
     clusterName: 'cluster1',
     serviceNameHyphen: 'svc-one',
     mapHyphenValue: 'mapval1',
@@ -108,6 +112,7 @@ export const INTERESTING_FILTER_KEYS_ROWS = [
     serviceName: 'service2',
     resourceAttrValue: 'value2',
     jsonValue: 'value2',
+    jsonAtKeyValue: 'request-B',
     clusterName: 'cluster2',
     serviceNameHyphen: 'svc-two',
     mapHyphenValue: 'mapval2',
@@ -118,6 +123,7 @@ export const INTERESTING_FILTER_KEYS_ROWS = [
     serviceName: 'service3',
     resourceAttrValue: 'value3',
     jsonValue: 'value3',
+    jsonAtKeyValue: 'request-C',
     clusterName: 'cluster3',
     serviceNameHyphen: 'svc-three',
     mapHyphenValue: 'mapval3',
@@ -126,6 +132,10 @@ export const INTERESTING_FILTER_KEYS_ROWS = [
   },
 ] as const;
 
+// The at-prefixed JSON key seeded into `ResourceAttributesJSON`. Exported so the
+// row-details spec selects and asserts against the same key/values.
+export const INTERESTING_FILTER_KEYS_JSON_AT_KEY = '@request_id';
+
 // A single log whose Body is a JSON string with a flat, dotted key. It lives in
 // the default E2E Logs table (so it shares the E2E Logs source); the unique
 // ServiceName isolates it from other default-logs tests, which all scope by
@@ -793,7 +803,7 @@ function generateK8sEventLogs(
 function generateInterestingFilterKeyLogData(seedRef: number): string {
   return INTERESTING_FILTER_KEYS_ROWS.map((row, i) => {
     const timestampNs = (seedRef - (i + 1) * 1000) * 1000000;
-    const json = `{"key": {"subKey": {"subSubKey": "${row.jsonValue}"}}}`;
+    const json = `{"key": {"subKey": {"subSubKey": "${row.jsonValue}"}}, "${INTERESTING_FILTER_KEYS_JSON_AT_KEY}": "${row.jsonAtKeyValue}"}`;
     // JSON column with a hyphenated name AND hyphenated nested keys.
     const jsonHyphen = `{"key-1": {"key-2": "${row.jsonHyphenValue}"}}`;
     return (

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

@@ -4127,6 +4127,66 @@ describe('quoteResultColumnNameIfNeeded', () => {
       '`x-host-header`',
     );
   });
+
+  it('quotes non-identifier segments of a dotted JSON path', () => {
+    expect(quoteResultColumnNameIfNeeded('event.@request_id')).toBe(
+      'event.`@request_id`',
+    );
+    expect(quoteResultColumnNameIfNeeded('event.my key')).toBe(
+      'event.`my key`',
+    );
+    expect(quoteResultColumnNameIfNeeded('a.b.@c')).toBe('a.b.`@c`');
+  });
+
+  it('quotes the path but preserves a JSON type cast', () => {
+    // ClickHouse reports `event.foo.:String` as event.foo.:`String`.
+    expect(quoteResultColumnNameIfNeeded('event.foo.:`String`')).toBe(
+      'event.foo.:`String`',
+    );
+    expect(quoteResultColumnNameIfNeeded('event.@request_id.:`String`')).toBe(
+      'event.`@request_id`.:`String`',
+    );
+    expect(quoteResultColumnNameIfNeeded('event.@id.:`Decimal(10, 2)`')).toBe(
+      'event.`@id`.:`Decimal(10, 2)`',
+    );
+    expect(quoteResultColumnNameIfNeeded('event.@id.:`Int64`.null')).toBe(
+      'event.`@id`.:`Int64`.null',
+    );
+  });
+
+  it('quotes a key that starts with `:`', () => {
+    expect(quoteResultColumnNameIfNeeded('event.:k')).toBe('event.`:k`');
+  });
+
+  it('quotes a dotted-path segment that itself contains a colon', () => {
+    expect(quoteResultColumnNameIfNeeded('event.k8s:pod')).toBe(
+      'event.`k8s:pod`',
+    );
+  });
+
+  it('quotes an all-digit JSON key (unquoted it errors in ClickHouse)', () => {
+    // `event.200` unquoted is parsed as tuple-element access and rejected for a
+    // JSON column; backticks make it a key reference. Tuple positional access
+    // is reported by ClickHouse as `tupleElement(...)`, never a bare `t.1`.
+    expect(quoteResultColumnNameIfNeeded('event.200')).toBe('event.`200`');
+    expect(quoteResultColumnNameIfNeeded('tup.1')).toBe('tup.`1`');
+  });
+
+  it('preserves a sub-object accessor', () => {
+    // ClickHouse reports `event.^sub` as event.^`sub`.
+    expect(quoteResultColumnNameIfNeeded('event.^`sub`')).toBe('event.^`sub`');
+    expect(quoteResultColumnNameIfNeeded('event.@a.^`b.c`')).toBe(
+      'event.`@a`.^`b.c`',
+    );
+  });
+
+  it('quotes a key that starts with `^`', () => {
+    expect(quoteResultColumnNameIfNeeded('event.^k')).toBe('event.`^k`');
+  });
+
+  it('leaves a malformed path with an empty segment untouched', () => {
+    expect(quoteResultColumnNameIfNeeded('a..b')).toBe('a..b');
+  });
 });
 
 describe('parametric aggregate arguments are inlined as literals', () => {

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

@@ -190,19 +190,56 @@ export const quoteIdentifierIfNeeded = (identifier: string): string => {
     : quoteJsonPathSegment(unquoted);
 };
 
+// A single column or JSON path segment. No quoting/grouping characters, no `.` separator.
+const PLAIN_PATH_SEGMENT = /^[^`'"()[\],.]+/;
+
+// A JSON sub-object accessor or type cast. ClickHouse reports the operand
+// backticked (j.^`a`, j.a.:`Int64`).
+const OPERATOR_PATH_SEGMENT = /^[\^:]`(?:[^`]|``)*`/;
+
+// Split a name into `.`-separated path segments, or undefined when it isn't a
+// plain JSON path (function calls, literals, empty segments).
+const splitResultColumnPath = (name: string): string[] | undefined => {
+  const segments: string[] = [];
+  let rest = name;
+  while (true) {
+    const segment =
+      rest.match(OPERATOR_PATH_SEGMENT)?.[0] ??
+      rest.match(PLAIN_PATH_SEGMENT)?.[0];
+    if (segment == null) return undefined;
+    segments.push(segment);
+    rest = rest.slice(segment.length);
+    if (rest === '') return segments;
+    if (!rest.startsWith('.')) return undefined;
+    rest = rest.slice(1);
+  }
+};
+
+const quotePathSegment = (segment: string): string =>
+  OPERATOR_PATH_SEGMENT.test(segment)
+    ? segment
+    : quoteIdentifierIfNeeded(segment);
+
 /**
  * Quote a query result column name so it can be referenced in raw SQL.
  *
  * Most result column names do not need quoting:
  * - Bare identifiers (alphanumeric and underscore, starting with a letter or underscore)
  * - Numeric literals (e.g., `1`, `0x10`)
- * - Dotted column names (e.g., `my.col`)
  * - Function calls (e.g., `plus(a, b)`, `arrayElement(m, 'key')`)
+ *
+ * Dotted names are JSON/nested subcolumn paths (e.g. `event.@request_id`,
+ * which ClickHouse reports for the select `` event.`@request_id` ``). Each
+ * segment is quoted on its own. JSON type casts (eg. `` .:`String` ``) and
+ * `` ^`sub` `` sub-object accessors are preserved.
  */
-export const quoteResultColumnNameIfNeeded = (name: string): string =>
-  /^[^`'"()[\],.]+$/.test(name) && !Number.isFinite(Number(name))
-    ? quoteIdentifierIfNeeded(name)
-    : name;
+export const quoteResultColumnNameIfNeeded = (name: string): string => {
+  if (Number.isFinite(Number(name))) {
+    return name;
+  }
+  const segments = splitResultColumnPath(name);
+  return segments ? segments.map(quotePathSegment).join('.') : name;
+};
 
 const columnAppearsInMvSelect = (sql: string, columnName: string): boolean => {
   const escaped = columnName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');