fix: keep parsed-JSON "Add to Filters" keys as valid SQL

.changeset/fix-json-add-to-filters-sql.md

@@ -0,0 +1,5 @@
+---
+'@hyperdx/app': patch
+---
+
+Fix "Add to Filters" on a value inside parsed JSON from a String column (for example `Body`) building invalid SQL. The `JSONExtractString(...)` expression the JSON viewer produces is now passed through unchanged instead of being mis-parsed as a dot-form Map sub-key and mangled into a query ClickHouse rejects.

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

@@ -164,6 +164,21 @@ describe('DBRowJsonViewer', () => {
     );
   });
 
+  // HDX-4427: "Add to Filters" on a value inside parsed JSON from a String
+  // column must hand searchFilters the JSONExtract* expression, which is what
+  // gets serialized into the WHERE clause. Body here is a String column holding
+  // a JSON string with a dotted key, mirroring the play-clickstack repro.
+  it('adds a JSONExtractString filter for a value inside a parsed-JSON string column', () => {
+    renderComponent({ Body: JSON.stringify({ 'app.user.currency': 'USD' }) });
+
+    expandAndClickButton('Body', 'app.user.currency', 'Add to Filters');
+
+    expect(mockOnPropertyAddClick).toHaveBeenCalledWith(
+      "JSONExtractString(Body, 'app.user.currency')",
+      'USD',
+    );
+  });
+
   it('toggles columns with correct path formatting', () => {
     renderComponent(logData);
     clickLineButton('field1', 'Column');

packages/app/src/components/DBSearchPageFilters/jsonAddToFilter.pipeline.test.ts

@@ -0,0 +1,167 @@
+import {
+  type FilterState,
+  filtersToQuery,
+  isValidFilterCondition,
+} from '@hyperdx/common-utils/dist/filters';
+
+import { buildJSONExtractQuery } from '@/components/DBRowJsonViewer';
+import { escapeFilterStateKeys, parseQuery } from '@/searchFilters';
+
+import { cleanClickHouseExpression } from './utils';
+
+// End-to-end coverage for the "Add to Filters" -> WHERE-clause path, driven
+// through the real producer (buildJSONExtractQuery) and the real serialization
+// (escapeFilterStateKeys -> filtersToQuery). This is the regression net for
+// HDX-4427: a filter key that is a raw ClickHouse expression must serialize to
+// valid SQL and survive a parseQuery round-trip. Asserting validity (rather
+// than an exact string) catches this bug class for any JSON/Map key shape, not
+// just the one the user happened to hit.
+
+// Real top-level columns on a typical OTel logs table.
+const knownColumns = new Set(['Body', 'LogAttributes', 'ServiceName']);
+
+// buildJSONExtractQuery returns string | null; in every case here a nested path
+// exists, so null is a test-setup error rather than an expected branch.
+const jsonKey = (
+  keyPath: string[],
+  parsedJsonRootPath: string[],
+  jsonColumns: string[] = [],
+  jsonExtractFn:
+    | 'JSONExtractString'
+    | 'JSONExtractFloat'
+    | 'JSONExtractBool' = 'JSONExtractString',
+  mapColumns: string[] = [],
+): string => {
+  const out = buildJSONExtractQuery(
+    keyPath,
+    parsedJsonRootPath,
+    jsonColumns,
+    jsonExtractFn,
+    mapColumns,
+  );
+  if (out == null) {
+    throw new Error('buildJSONExtractQuery returned null for a nested path');
+  }
+  return out;
+};
+
+const runAddToFilter = (
+  key: string,
+  values: { included?: string[]; excluded?: string[] },
+) => {
+  const state: FilterState = {
+    [key]: {
+      included: new Set<string | boolean>(values.included ?? []),
+      excluded: new Set<string | boolean>(values.excluded ?? []),
+    },
+  };
+  // Mirror updateFilterQuery: escape keys, then serialize to the query.
+  return filtersToQuery(escapeFilterStateKeys(state, knownColumns));
+};
+
+describe('parsed-JSON "Add to Filters" -> valid SQL pipeline (HDX-4427)', () => {
+  const cases: {
+    desc: string;
+    key: string;
+    values: { included?: string[]; excluded?: string[] };
+  }[] = [
+    {
+      desc: 'flat dotted key in a String column (the reported play-clickstack case)',
+      key: jsonKey(['Body', 'app.user.currency'], ['Body']),
+      values: { included: ['USD'] },
+    },
+    {
+      desc: 'nested path in a String column',
+      key: jsonKey(['Body', 'app', 'user', 'id'], ['Body']),
+      values: { included: ['u-1'] },
+    },
+    {
+      desc: 'simple top-level key in a String column',
+      key: jsonKey(['Body', 'level'], ['Body']),
+      values: { included: ['error'], excluded: ['debug'] },
+    },
+    {
+      desc: 'numeric metric via JSONExtractFloat with a dotted path',
+      key: jsonKey(
+        ['Body', 'metrics.latency'],
+        ['Body'],
+        [],
+        'JSONExtractFloat',
+      ),
+      values: { included: ['200'] },
+    },
+    {
+      desc: 'boolean flag via JSONExtractBool with a dotted path',
+      key: jsonKey(['Body', 'flags.enabled'], ['Body'], [], 'JSONExtractBool'),
+      values: { included: ['true'] },
+    },
+    {
+      desc: 'Map sub-value holding JSON (bracketed base column)',
+      key: jsonKey(
+        ['LogAttributes', 'config', 'db.host'],
+        ['LogAttributes', 'config'],
+        [],
+        'JSONExtractString',
+        ['LogAttributes'],
+      ),
+      values: { included: ['localhost'] },
+    },
+    {
+      desc: 'value containing a single quote (SQL escaping)',
+      key: jsonKey(['Body', 'user.name'], ['Body']),
+      values: { included: ["O'Brien"] },
+    },
+  ];
+
+  it.each(cases)(
+    'emits only valid ClickHouse SQL for $desc',
+    ({ key, values }) => {
+      const query = runAddToFilter(key, values);
+
+      expect(query.length).toBeGreaterThan(0);
+      for (const filter of query) {
+        expect(filter.type).toBe('sql');
+        if (filter.type === 'sql') {
+          expect(isValidFilterCondition(filter.condition, 'sql')).toBe(true);
+        }
+      }
+    },
+  );
+
+  it('emits the corrected, valid condition for the reported case', () => {
+    const key = jsonKey(['Body', 'app.user.currency'], ['Body']);
+    expect(runAddToFilter(key, { included: ['USD'] })).toEqual([
+      {
+        type: 'sql',
+        condition: "JSONExtractString(Body, 'app.user.currency') IN ('USD')",
+      },
+    ]);
+  });
+
+  it('round-trips included and excluded values through parseQuery', () => {
+    const key = jsonKey(['Body', 'app.user.currency'], ['Body']);
+    const query = runAddToFilter(key, {
+      included: ['USD', 'EUR'],
+      excluded: ['JPY'],
+    });
+
+    const back = parseQuery(query).filters;
+    // parseQuery keys are the canonical (escaped) form; clean them the same way
+    // unescapeFilterStateKeys does before comparing to the in-memory key.
+    const restoredKeys = Object.keys(back).map(cleanClickHouseExpression);
+    expect(restoredKeys).toEqual([key]);
+
+    const entry = back[Object.keys(back)[0]];
+    expect(entry.included).toEqual(new Set(['USD', 'EUR']));
+    expect(entry.excluded).toEqual(new Set(['JPY']));
+  });
+
+  it('round-trips a value containing a single quote', () => {
+    const key = jsonKey(['Body', 'user.name'], ['Body']);
+    const back = parseQuery(
+      runAddToFilter(key, { included: ["O'Brien"] }),
+    ).filters;
+    const entry = back[Object.keys(back)[0]];
+    expect(entry.included).toEqual(new Set(["O'Brien"]));
+  });
+});

packages/app/src/components/DBSearchPageFilters/utils.test.ts

@@ -180,6 +180,34 @@ describe('toClickHouseKeyExpression', () => {
       "LogAttributes['42.foo']",
     );
   });
+
+  // HDX-4427: "Add to Filters" on a value inside parsed JSON from a String
+  // column builds a JSONExtract* function call as the filter key. These are
+  // already valid ClickHouse expressions and must pass through untouched.
+  // Previously the dot inside the quoted JSON path argument made
+  // parseMapFieldName treat the whole expression as a dot-form Map sub-key, and
+  // mergePath mangled it into invalid SQL like
+  // `JSONExtractString(Body, 'app['user.currency')']`.
+  describe('raw SQL function-call expression keys (parsed-JSON "Add to Filters")', () => {
+    it.each([
+      "JSONExtractString(Body, 'app.user.currency')",
+      "JSONExtractString(Body, 'app', 'user.currency')",
+      "JSONExtractString(Body, 'level')",
+      "JSONExtractFloat(Body, 'metrics.latency')",
+      "JSONExtractBool(Body, 'flags.enabled')",
+      "JSONExtractString(LogAttributes['weird.key.payload'], 'abc.def.jqk/abcd')",
+    ])('leaves the JSON-extract expression %s unchanged', key => {
+      expect(toClickHouseKeyExpression(key)).toBe(key);
+    });
+
+    // The guard generalizes the previous `startsWith('toString(')` special case,
+    // so a toString() wrapper with no bracket access still passes through.
+    it('leaves a toString() wrapper without bracket access unchanged', () => {
+      expect(toClickHouseKeyExpression('toString(Body)')).toBe(
+        'toString(Body)',
+      );
+    });
+  });
 });
 
 describe('toQuotedClickHouseKeyExpression', () => {
@@ -241,6 +269,18 @@ describe('toQuotedClickHouseKeyExpression', () => {
     ).toBe('`service-name`');
   });
 
+  // HDX-4427: the JSONExtract* key from a parsed-JSON "Add to Filters" reaches
+  // toQuotedClickHouseKeyExpression via escapeFilterStateKeys. It is not a known
+  // column and is already valid SQL, so it must pass through unquoted/unmangled.
+  it('leaves a JSON-extract function-call key unchanged', () => {
+    expect(
+      toQuotedClickHouseKeyExpression(
+        "JSONExtractString(Body, 'app.user.currency')",
+        new Set(['Body']),
+      ),
+    ).toBe("JSONExtractString(Body, 'app.user.currency')");
+  });
+
   describe('with knownColumns (schema-aware)', () => {
     it('quotes a flat column whose name contains dots as a single identifier', () => {
       const cols = new Set(['__hdx_materialized_k8s.cluster.name']);

packages/app/src/components/DBSearchPageFilters/utils.ts

@@ -142,12 +142,19 @@ export function getFilterStateEntry(
   );
 }
 
+// A key that begins with `identifier(` is a raw SQL function call (e.g.
+// `toString(...)`, `JSONExtractString(...)`), not a column name or a dot-form
+// Map sub-key, so it is already a valid ClickHouse expression.
+const isSqlFunctionCallExpression = (key: string): boolean =>
+  /^[A-Za-z_]\w*\(/.test(key);
+
 // Coerce a filterState key into a ClickHouse expression suitable for raw SQL.
 // A dot-form Map sub-key like `LogAttributes.host.name` is rewritten to bracket
 // form `LogAttributes['host.name']` via `mergePath` so the conversion stays
 // consistent with the keys produced by the facet-discovery path. Bracket form,
-// backtick-quoted JSON paths, `toString(...)` wrappers, and plain column names
-// are returned unchanged. Use this when handing a filterState key off to a SQL
+// backtick-quoted JSON paths, raw SQL function-call expressions
+// (`toString(...)`, `JSONExtractString(...)`), and plain column names are
+// returned unchanged. Use this when handing a filterState key off to a SQL
 // caller (e.g. "Load more" via metadata.getKeyValues), since `setFilterValue`
 // normalizes Map sub-keys to dot form which ClickHouse cannot resolve as map
 // access.
@@ -162,7 +169,12 @@ export function toClickHouseKeyExpression(key: string): string {
     key.includes("['") ||
     key.includes('["') ||
     key.includes('`') ||
-    key.startsWith('toString(')
+    // "Add to Filters" on a value inside parsed JSON from a String column builds
+    // a function-call key (e.g. JSONExtractString(Body, 'app.user.currency'));
+    // it must pass through untouched. Without this, parseMapFieldName splits on
+    // the dot inside the quoted argument and mergePath mangles it into the
+    // invalid `JSONExtractString(Body, 'app['user.currency')']`. HDX-4427.
+    isSqlFunctionCallExpression(key)
   ) {
     return key;
   }

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

@@ -62,6 +62,47 @@ export class SidePanelComponent {
     }
   }
 
+  /**
+   * In the parsed JSON view, expand a field whose value is a JSON string, then
+   * click the "Add to Filters" line action on a nested key. Mirrors the
+   * DBRowJsonViewer line actions: each row carries a hashed `*__line` CSS-module
+   * class, and the action is a `<button title="Add to Filters">` revealed only
+   * while the row is hovered.
+   */
+  async addParsedJsonFieldToFilter(parentField: string, nestedKey: string) {
+    await this.clickTab('parsed');
+
+    // The nested key only renders as its own line once the parent field (whose
+    // value is a JSON string) is expanded. An exact text match avoids matching
+    // the collapsed parent's raw-value preview, which contains the key inline.
+    const leaf = this.panelContainer.getByText(nestedKey, { exact: true });
+    if (
+      !(await leaf
+        .first()
+        .isVisible()
+        .catch(() => false))
+    ) {
+      // Click the parent's key (left side, away from the hover-only action
+      // menu) to expand it.
+      await this.panelContainer
+        .getByText(parentField, { exact: true })
+        .first()
+        .click();
+    }
+
+    await leaf.first().waitFor({ state: 'visible', timeout: 10_000 });
+    // Hover the line to mount its action menu, then click the titled button
+    // scoped to that line so the parent's menu is never the target.
+    await leaf.first().hover();
+    const leafLine = leaf
+      .first()
+      .locator('xpath=ancestor::*[contains(@class,"__line")][1]');
+    await leafLine
+      .getByTitle(/add to filters/i)
+      .first()
+      .click({ timeout: 10_000 });
+  }
+
   /**
    * Close the side panel (if it has a close button)
    */

packages/app/tests/e2e/features/search/json-add-to-filter.spec.ts

@@ -0,0 +1,59 @@
+import { SearchPage } from '../../page-objects/SearchPage';
+import { JSON_BODY_LOG } from '../../seed-clickhouse';
+import { expect, test } from '../../utils/base-test';
+import { DEFAULT_LOGS_SOURCE_NAME } from '../../utils/constants';
+
+// Re-running the search query after a filter change can take longer than
+// Playwright's 5s assertion default on slow CI runners.
+const QUERY_TIMEOUT = 20_000;
+
+// HDX-4427: "Add to Filters" on a value inside parsed JSON from a String column
+// (Body) builds a JSONExtractString(...) expression as the filter key. Before
+// the fix this serialized to invalid SQL and ClickHouse rejected the query, so
+// the whole search errored. This drives the real UI path end to end against
+// ClickHouse: add the filter from the row side panel, then confirm the query
+// runs without error and still returns the matching row.
+test.describe(
+  'Search: parsed-JSON "Add to Filters"',
+  { tag: ['@search', '@full-stack'] },
+  () => {
+    test('filtering on a nested JSON Body value runs without a ClickHouse error', async ({
+      page,
+    }) => {
+      const searchPage = new SearchPage(page);
+      await searchPage.goto();
+      await searchPage.selectSource(DEFAULT_LOGS_SOURCE_NAME);
+      await searchPage.timePicker.selectRelativeTime('Last 1 hour');
+
+      // Isolate the seeded JSON-body row so the side panel opens on it.
+      await searchPage.performSearch(
+        `ServiceName:"${JSON_BODY_LOG.serviceName}"`,
+      );
+      await expect(searchPage.getTableError()).toHaveCount(0);
+      await expect(searchPage.table.getRows()).toHaveCount(1, {
+        timeout: QUERY_TIMEOUT,
+      });
+
+      // Open the row, then add a filter on the nested JSON value from the
+      // parsed tab. This builds JSONExtractString(Body, 'app.user.currency').
+      await searchPage.table.clickFirstRow();
+      await searchPage.sidePanel.addParsedJsonFieldToFilter(
+        'Body',
+        JSON_BODY_LOG.jsonKey,
+      );
+
+      // The filter is built from the nested key as a JSONExtractString
+      // expression and persisted to the URL.
+      await expect(page).toHaveURL(/JSONExtractString/);
+      // The generated SQL must be valid: no table error and the matching row
+      // still comes back. Before the fix the mangled key produced invalid SQL
+      // that ClickHouse rejected, so the query errored and returned no rows.
+      await expect(searchPage.getTableError()).toHaveCount(0, {
+        timeout: QUERY_TIMEOUT,
+      });
+      await expect(searchPage.table.getRows()).toHaveCount(1, {
+        timeout: QUERY_TIMEOUT,
+      });
+    });
+  },
+);