Fix #3370: row details overwrote the WHERE-clause id when the table has an id column by wanjinhao1 · Pull Request #3386 · hyperdxio/hyperdx

wanjinhao1 · 2026-10-16T12:05:14Z

Summary

In DBRowTable.tsx, the row-details object was built as { id: rowId, aliasWith: ..., ...row.original }. If the underlying table has an id column, the spread of row.original overwrote id with the raw column value, so expanding a row sent the raw value as the WHERE clause (e.g. WHERE (a5c1334f-9b43-...)), which ClickHouse parses as an invalid expression and fails with "Error loading row data".

The fix spreads row.original first and then sets id and aliasWith, so the WHERE clause identifier always wins — exactly the approach suggested in the issue by @karl-power.

Screenshots or video

Omitted — no visual UI change; the fix only corrects the query payload sent when expanding a row. (Bug symptom on main: "Error loading row data" banner; on this branch the row expands successfully.)

How to reproduce

Starting point: HyperDX OSS 2.41.0 (or main), a table whose id column is a string containing hyphens/periods, e.g. a5c1334f-9b43-44db-abc3-3412220272dd.0.0 (schema and details in #3370).

On main:

Search the table and expand a row.
Verify: "Error loading row data" is shown; the sent query contains WHERE (a5c1334f-9b43-44db-abc3-3412220272dd.0.0).

On this branch:

Same steps.
Verify: the row details load; the sent query keeps the proper WHERE clause on the internal row id.

How to test on Vercel preview

N/A — requires a custom table with a non-integer string id column, which the demo ClickHouse data does not provide; reproduction steps above cover the change.

References

Linear Issue: N/A
Related PRs: none
Issue: Fixes Error loading row data if id field is not an integer #3370

🤖 This change was implemented with AI assistance under the direction of the contributor (@wanjinhao1), who reviewed and validated the change. Formatting checked with Prettier.

…n row details The row details object spread row.original after setting id and aliasWith, so a table with an uid=1000(wan) gid=1000(wan) groups=1000(wan),4(adm),20(dialout),24(cdrom),27(sudo),30(dip),46(plugdev),100(users),114(lpadmin),995(input) column overwrote the WHERE clause id with the raw column value, producing invalid SQL when expanding a row. Spread the row first so id/aliasWith always win.

changeset-bot · 2026-10-16T12:05:20Z

🦋 Changeset detected

Latest commit: 2a074ff

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 · 2026-10-16T12:05:22Z

@wanjinhao1 is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

github-actions · 2026-10-16T12:05:30Z

Hi @wanjinhao1, thanks for the pull request!

Before we review code from a first-time contributor, a maintainer vouches for them, and you're not on our list yet. There's nothing else to file and this PR stays open — a maintainer will look at it, usually within a day or two, and vouch for you right here if it's a genuine attempt at the problem.

To make that quick, check the PR description has:

The issue it addresses (ideally one labelled Accepting PR) and why the change is needed
Steps to reproduce the problem and verify the fix, starting from https://play.hyperdx.io or yarn app:dev:local connected to the demo server
Before/after screenshots if anything in the UI changed

More detail in our contributing guide.

github-actions · 2026-10-16T12:07:53Z

                        canOpenSidePanel={canOpenSidePanel}
                      >
                        {renderRowDetails?.({
+                          ...row.original,


🔵 minor — No regression test pins that id/aliasWith override a same-named column in row details

Add a case to the existing inline-expand suite in packages/app/src/components/__tests__/DBRowTable.test.tsx (baseProps ~line 316). Use rows: [{ id: 'a5c1334f-...', col1: 'v' }] and a jest.fn() renderRowDetails, expand the row, then assert it was called with expect.objectContaining({ id: ROW_ID }). Without it, moving the spread back after id would bring #3370 back and every test would still pass.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

github-actions · 2026-10-16T12:07:54Z

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.

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

1 posted as inline comment(s) on the changed lines.

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

greptile-apps · 2026-10-16T12:08:56Z

Confidence Score: 4/5

[Medium impact] Fixes row detail panel ID handling in database table display.

The fix matches the row-details query’s needs, but the required regression test should be added before merging.

Findings

Spread order has no test ▶

Summary

Prevents raw row columns from overwriting the generated query fields used by expanded row details.

Expanded rows keep the internal id for their detail query.

wanjinhao1 explicitly omitted screenshots because the change only corrects the query payload; no screenshot finding is raised.

_{Reviews (1) · Last reviewed commit: "Add changeset for row-details id fix" · Reviewed by Greptile}

greptile-apps · 2026-10-16T12:09:00Z

+                          ...row.original,
                          id: rowId,
                          aliasWith:
                            row.original[INTERNAL_ROW_FIELDS.ALIAS_WITH],


Spread order has no test

This fix changes which id and aliasWith reach renderRowDetails, but adds no test. Existing expansion tests use rows without an id column, and their details callback ignores its argument, so they would pass with the old bug.

AGENTS.md requires tests alongside the implementation. Before merging, add a test that expands a row with a raw string id and checks that renderRowDetails receives the generated WHERE clause as id. Also check that computed aliasWith wins over a matching raw column.

Context Used: AGENTS.md (source)

github-actions · 2026-10-16T12:10:51Z

Deep Review

✅ No critical issues found. The one-line reorder is a correct fix: spreading row.original first and then setting id/aliasWith makes the internal WHERE-clause identifier win over any raw data column of the same name. Both consumers were verified — DBSqlRowTableWithSidebar reads r.id/r.aliasWith, and ClickhousePage reads only row.query — and the no-id-column path is unchanged. The explicit aliasWith line is not redundant, since row.original only carries the differently-named __hyperdx_alias_with key.

🟡 P2 -- recommended

packages/app/src/components/DBRowTable.tsx:1323 -- No regression test asserts the internal id wins over a raw id column, so reverting the key order would still pass the suite.
- Fix: In DBRowTable.test.tsx, render a row whose row.original contains an id key and assert the renderRowDetails spy receives an object where id equals the WHERE-clause rowId, not the raw column value.

🔵 P3 nitpicks (2)

packages/app/src/components/DBRowTable.tsx:1323 -- The ordering intent is non-obvious; a future edit could reintroduce the bug by moving the spread back below id/aliasWith.
- Fix: Add a short comment noting that row.original is spread first so the internal id/aliasWith override any identically named data column.
packages/app/src/components/DBRowTable.tsx:1322 -- The [key: string]: any index signature on the renderRowDetails parameter means TypeScript cannot verify the spread-override ordering, so correctness rests on manual key order (pre-existing).
- Fix: Optionally narrow the rest shape (e.g. Omit<Record<string, unknown>, 'id' | 'aliasWith'>) so override safety is type-checked rather than convention-only.

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

Testing gaps: No test exercises renderRowDetails when row.original contains an id key colliding with the internal rowId; the existing stub ignores its argument, so argument-shape regressions are invisible.

_{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}

github-actions Bot added external needs-vouch Author needs a maintainer to vouch for them labels Oct 16, 2026

Add changeset for row-details id fix

2a074ff

github-actions Bot reviewed Oct 16, 2026

View reviewed changes

greptile-apps Bot reviewed Oct 16, 2026

View reviewed changes

Navigation Menu

Repository navigation

Fix #3370: row details overwrote the WHERE-clause id when the table has an id column - #3386

Fix #3370: row details overwrote the WHERE-clause id when the table has an id column#3386
wanjinhao1 wants to merge 2 commits into
hyperdxio:mainfrom
wanjinhao1:agent/fix-3370-row-id-where

wanjinhao1 commented Oct 16, 2026

Uh oh!

changeset-bot Bot commented Oct 16, 2026 •

edited

Loading

Uh oh!

vercel Bot commented Oct 16, 2026

Uh oh!

github-actions Bot commented Oct 16, 2026

Uh oh!

github-actions Bot Oct 16, 2026

Uh oh!

github-actions Bot commented Oct 16, 2026

Uh oh!

greptile-apps Bot commented Oct 16, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot Oct 16, 2026

Uh oh!

github-actions Bot commented Oct 16, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

wanjinhao1 commented Oct 16, 2026

Summary

Screenshots or video

How to reproduce

How to test on Vercel preview

References

Uh oh!

changeset-bot Bot commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

vercel Bot commented Oct 16, 2026

Uh oh!

github-actions Bot commented Oct 16, 2026

Uh oh!

github-actions Bot Oct 16, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 16, 2026

PR Review

Uh oh!

greptile-apps Bot commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 4/5

Findings

Summary

Uh oh!

greptile-apps Bot Oct 16, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 16, 2026

Deep Review

🟡 P2 -- recommended

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

changeset-bot Bot commented Oct 16, 2026 •

edited

Loading

greptile-apps Bot commented Oct 16, 2026 •

edited

Loading