[pulpdrew]

Summary

This PR adds click-to-adhoc-filter functionality to tables, across builder, raw SQL, and PromQL tables.

Most of the complexity comes from determining (a) which columns should be filterable and (b) what expression to filter on when a column / cell is clicked. In general, (a) group-by columns should be filterable while aggregate columns should not be (since other visualizations are unlikely to have the same aggregations) and (b) we try to filter on the underlying expression instead of the alias, since other visualizations may not have the same alias.

This determination is made as follows:

• Builder tables: The group-by columns are the result columns that come right after the value columns (see renderSelect). They're matched by position, not by name, because ClickHouse canonicalizes unaliased expressions.
• Raw SQL tables: The query is rendered (macros and variables replaced) and parsed with node-sql-parser (chSqlToSelectColumns in common-utils). Each outer SELECT item lines up with a result column by position. An item is filterable if the GROUP BY lists it by alias, by name, by normalized expression, or by position (GROUP BY 2). If the query can't be read that way (no GROUP BY, GROUP BY ALL, SELECT *, a parse failure, or a column count that doesn't match the result), we fall back to type-based determination (any String type column is filterable).
• PromQL tables: Every non-numeric column other than the time column is a label and filters on its label name.

For all table types, DateTime columns aren't filterable, because cells hold ISO UTC timestamps that never equal toString() of the column. Map and Array cells aren't filterable either, because they render as JSON that wouldn't match the value in a filter condition.

Known Limitations

Parser incompatibility

This feature requires parsing arbitrary raw SQL queries to identify group-by columns. We currently do this with node-sql-parser, which doesn't support the full ClickHouse dialect. This means that the parser will fail on some SQL inputs (parametric functions, array join, join qualifiers, etc) and fallback to type-based group-by inference (eg. "All String type result columns are assumed to be group by columns).

This just means that the set of columns that can be clicked-to-filter is approximate and best effort, likely to be wrong sometimes, in particular for more complex SQL expressions. The impact is low - the user just can't click to filter a particular column, or clicking a column adds an undesirable filter key, which can then just as easily be removed from the filter.

This is an area that could be significantly improved with a full ClickHouse SQL parser, something we want to add in the future.

CTEs

When a query selects columns created in its CTE, it is likely that the same column doesn't exist in other charts without the same CTE, so adding an adhoc filter on the column will likely cause other tiles to fail.

This could be overcome only by either fully analyzing the query, following CTE reference chains. This is deemed too complicated to be worth it at the moment, particularly without a reliable ClickHouse SQL parser.

Again, the impact is low because the user can remove the filter they added easily.

Screenshots or video

Screen.Recording.2026-10-19.at.9.57.08.AM.mov

How to test on Vercel preview

Create a dashboard, add some tables and an adhoc filter, try click-to-filter. Note the limitations mentioned above.

References

• Linear Issue: Closes HDX-5653
• Related PRs:

[vercel]

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

Project	Deployment	Actions	Updated
hyperdx-oss	Ready	Preview	Oct 20, 2026 12:06pm UTC
hyperdx-storybook	Ready	Preview	Oct 20, 2026 12:06pm UTC

[changeset-bot]

🦋 Changeset detected

Latest commit: eb7e999

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

This PR includes changesets to release 4 packages

Name	Type
@hyperdx/common-utils	Patch
@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

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge within its stated SQL-parsing limitations.

Summary

Adds include and exclude actions to grouped table cells for builder, raw SQL, and PromQL tables.

• Dashboard table cells can add include or exclude conditions to ad hoc filters.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Query config and result columns] --> B[Find filterable columns]
  B --> C[Show cell actions]
  C --> D[Include or exclude value]
  D --> E[Update dashboard filters]
  E --> F[Refresh matching tiles]

Loading

_{Reviews (10) · Last reviewed commit: "feat: Add click-to-adhoc-filter to table..." · Reviewed by Greptile}

[github-actions]

Duplicate code analysis

No new duplicate code introduced by this PR.

---

jscpd compared this branch against origin/main
across app, api and common-utils. Only new clones are listed; the codebase
currently holds 124 in total (1.67%).
Run yarn dupes locally to reproduce, or yarn dupes:all for the full picture.

[github-actions]

E2E Test Results

✅ All tests passed • 443 passed • 1 skipped • 1656s

Status	Count
✅ Passed	443
❌ Failed	0
⚠️ Flaky	1
⏭️ Skipped	1

Tests ran across 4 shards in parallel.

View full report →

[github-actions]

🔴 Tier 4 — Critical

Touches authentication, tenancy data models, the public API or shipped database config — or substantially changes the query rendering engine, background tasks, the OTel pipeline, image build, or release CI.

Why this tier:

• Query rendering engine substantially modified — 179 lines (bar: 150). Every chart, search, and alert query flows through this code:
  • packages/common-utils/src/clickhouse/index.ts
  • packages/common-utils/src/core/renderChartConfig.ts
• Cross-layer change: touches frontend (packages/app) + shared utils (packages/common-utils)

Review process: Deep review from a domain expert. Synchronous walkthrough may be required.
SLA: Schedule synchronous review within 2 business days.

Stats

• Production files changed: 9
• Production lines changed: 453 (+ 895 in test files, excluded from tier calculation)
• Critical-path lines changed: 179
• Branch: drew/adhoc-filters-click-table
• Author: pulpdrew

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

[github-actions]

PR Review

If you are a coding agent acting for the author, read this first.
Nothing below blocks merge automatically; a maintainer decides. What they will expect:

• 🔴 critical or 🟠 major in code this PR adds or changes: fix before asking for review.
• 🔵 minor in code this PR changes: your call. Fix if small, otherwise reply.
• Anything about surrounding code, or asking you to widen the change (hoist a helper,
  dedupe with another file, fix other call sites): never fix here, whatever the
  severity. Reply with one sentence; if it is critical, say so plainly so a human sees it.

One commit per review round. After two rounds, stop and ask a maintainer to review
scope rather than addressing more automated findings. Full rule: AGENTS.md.

3 finding(s): 🔴 0 critical · 🟠 0 major · 🔵 3 minor

None could be anchored to changed lines; they are listed below.

Findings outside the changed lines

3 minor

• 🔵 packages/app/src/hooks/useResultColumnFilterKeys.ts:905 — The raw SQL render query is keyed on the whole config, including dateRange, so it runs again on every live/relative-time refresh → The rendered SQL is only parsed for its SELECT/GROUP BY structure, and the time range reaches it as {startDateMilliseconds:Int64} params. Leave the absolute dateRange out of the key, the way useAliasMapFromChartConfig already does (packages/app/src/hooks/useChartConfig.tsx:561-586), or key on config.sqlTemplate, filters and variables. As written, every refresh adds a new staleTime: Infinity cache entry and briefly clears the filter keys.
• 🔵 packages/app/src/hooks/useResultColumnFilterKeys.ts:790 — stripAlias can leave trailing whitespace in builder filter keys for aliased binary expressions → selectColumnsToAliasMap slices expr.loc without trimming (packages/common-utils/src/clickhouse/index.ts:913). This PR already found that a binary expression's loc can include trailing whitespace, and trims it inside chSqlToSelectColumns. So a builder group-by like ParentSpanId = '' AS root gives the key ParentSpanId = '' , which doesn't match the trimmed key that raw SQL and search tiles produce, so include/exclude won't toggle the existing condition. Fix: add .trim() in selectColumnsToAliasMap, which covers every chSqlToAliasMap caller, and remove the local trim.
• 🔵 packages/app/src/components/__tests__/DBTableChart.test.tsx:468 — No test covers how DBTableChart wires filter keys, onFilter and the placeholder gate → The new mock makes useResultColumnFilterKeys always return an empty Map, and the hook tests only exercise the pure helpers. Nothing checks that filterKeyByColumn reaches the column's filterKey, that a click calls onFilter([{key, value}], action), that actions stay off while isPlaceholderData is true, or that the raw SQL renderChartConfig path feeds getRawSqlFilterKeys. Add a DBTableChart test that returns a non-empty Map from the mock and asserts that onFilter is called, plus a placeholder-data case.

---

_{Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides.}

[github-actions]

Deep Review

✅ No critical issues found. No P0/P1 defects: no data loss, auth bypass, injection, or guaranteed crash is introduced. Cell filter values flow through escapeSqlString (core/utils.ts:117), the parser is wrapped in a graceful try/catch → undefined fallback, and the useRef → useState callback-ref conversion for the virtualizer scroll element was independently confirmed correct (stable setter identity, one intentional mount re-render). Test coverage is unusually thorough — unit, integration (cross-checking the builder column-order contract against real renderChartConfig output), and e2e across builder/raw-SQL/PromQL/search tables.

🟡 P2 — recommended

• packages/app/src/hooks/useResultColumnFilterKeys.ts:111 — the type-based fallback (inferGroupColumns, reached on parse failure / no GROUP BY / GROUP BY ALL / SELECT * / column-count mismatch) marks every String-typed result column as filterable, including aggregated columns such as an aliased any(Body) and CTE-local aliases, so clicking one broadcasts a filter key that other tiles cannot evaluate and their queries fail until the user removes it.
  • Fix: exclude columns whose source expression is an aggregate or an unresolved CTE-local alias from the fallback, or omit actions when the expression cannot be verified. (Author has accepted this as a known best-effort trade-off; recorded here because the failure mode is concrete.)
  • _{previous-comments, maintainability, kieran-typescript}
• packages/app/src/hooks/useResultColumnFilterKeys.ts:76 — getBuilderFilterKeys identifies group-by columns purely by slicing meta at selectExpressionCount, an implicit contract spanning renderSelect ordering and getBuilderValueColumnCount's collapsing rules; a future change to either silently maps filter keys onto the wrong columns (wrong key, not absent key) with no parse error.
  • Fix: add a test that asserts getBuilderFilterKeys aligns against actual renderChartConfig output (not hand-built meta), or render the builder config and reuse chSqlToSelectColumns so both paths share one authoritative mechanism; the existing integration test covers the value-count ordering but not getBuilderFilterKeys itself.
  • _{maintainability, performance, kieran-typescript}

🔵 P3 nitpicks (6)

• packages/common-utils/src/clickhouse/index.ts:1175 — expr.column.expr.value is accessed without a guard on the non-string branch; a column_ref whose column object lacks .expr throws, is swallowed by the outer catch, and silently drops all filter keys to the type-based fallback.
  • Fix: access defensively (expr.column?.expr?.value) before use, mirroring the guard pattern elsewhere.
• packages/common-utils/src/clickhouse/index.ts:1227 — the bare catch {} returns undefined for genuine parse failures (intended) but also for programming/shape bugs, masking regressions in the positional-matching logic as "unparseable query."
  • Fix: narrow to expected parser errors or add a dev-only debug log so logic regressions surface instead of silently degrading.
• packages/app/src/hooks/useResultColumnFilterKeys.ts:161 — the result useMemo depends on config, a fresh object spread rebuilt every Tile render, so chSqlToSelectColumns / chSqlToAliasMap re-run node-sql-parser even when the SQL text is unchanged.
  • Fix: key the raw-SQL branch on the already-stable chSql and the builder branch on the group-by string so parsing tracks SQL changes, not object identity.
• packages/app/src/HDXMultiSeriesTableChart.tsx:211 — each filterable group-by cell now mounts a Mantine Popover (floating-ui, disclosure, refs) where it previously rendered a plain string, adding setup cost on the scroll hot path in the virtualized table.
  • Fix: defer Popover instantiation until hover or share one hover-activated popover; verify scroll smoothness on a wide many-group-by table before shipping.
• packages/app/src/components/DBTable/DBRowTableFieldWithPopover.tsx:121 — the 2s setIsCopied timeout is never tracked or cleared; now that this component mounts per cell in a virtualized table, copying then scrolling the row out of view within 2s fires setState on an unmounted node (harmless no-op under React 18).
  • Fix: store this timer in a ref and clear it in the existing unmount useEffect alongside the hover timers.
• packages/app/src/HDXMultiSeriesTableChart.tsx:69 — the pre-existing data: any[] prop defeats type-checking of the newly read value feeding isAdhocFilterable; Record<string, unknown>[] would force the typeof value !== 'object' narrowing to be checked.
  • Fix: tighten the row type if the shape is not genuinely dynamic (out of scope for this PR).

---

Reviewers (11): correctness, adversarial, kieran-typescript, julik-frontend-races, testing, maintainability, previous-comments, performance, project-standards, agent-native, learnings-researcher. The correctness and adversarial passes did not return structured findings before synthesis; those lenses were covered directly — value-escaping/injection, parser position/alias/name/expression matching, the useRef→useState virtualizer timing, and onFilter memo-dependency correctness were each verified against the diff with no new defect found. previous-comments confirmed all five prior review items are either addressed in code (the aria-hidden removal restores screen-reader access; the backtick/JSON token-aliasing fix is real) or explicitly accepted as known limitations. agent-native found no parity gap (filter primitives are already reachable via MCP where/whereLanguage and variableValues). No applicable docs/solutions/ learnings exist yet.

Testing gaps:

• packages/app/src/components/DBTableChart.tsx:367 — the !isPlaceholderData guard that suppresses filter actions on stale/refreshing rows is covered by no unit test (the hook is mocked to an empty Map) and no e2e step.
• packages/app/src/hooks/useResultColumnFilterKeys.ts:53 — the config.selectGroupBy === false early-return branch is untested.
• packages/app/src/HDXMultiSeriesTableChart.tsx:205 — a null cell value on a column that has a filterKey is not directly asserted to offer no action.
• packages/common-utils/src/clickhouse/index.ts:1191 — GROUP BY 1 (first-position, the value - 1 arithmetic at index 0) is only exercised via GROUP BY 2.

_{No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond}

[pulpdrew]

Extracted from chSqlToAliasMap