Repository navigation
fix(app): resolve row details with PK + whichever block column the table persists - #3364
Conversation
…e_block_offset_column is set Tables that persist _block_offset but not _block_number fell back to the full-row lookup, which filters on every selected column (incl. Body) and is far slower than the search itself. PK + _block_offset is unique enough with nanosecond timestamps, so require only the offset column and add _block_number when it is also persisted. Co-authored-by: Aaron Knudtson <knudtty@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 05048a6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
Duplicate code analysisNo new duplicate code introduced by this PR. jscpd compared this branch against |
 |
…olumn is set Co-authored-by: Aaron Knudtson <knudtty@users.noreply.github.com>
cursor
Bot
changed the title
E2E Test Results✅ All tests passed • 406 passed • 1 skipped • 1595s
Tests ran across 4 shards in parallel. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
github-actions
Bot
added
the
review/tier-2
🔵 Tier 2 — Low RiskSmall, isolated change with no API route or data model modifications. Why this tier:
Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns. Stats
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
PR Review
1 finding(s): 🔴 0 critical · 🟠 1 major · 🔵 0 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. |
Deep ReviewRelaxing the fast row-detail lookup to fire on either persisted block column is a sound performance fix, and the dedicated reviewer confirmed it achieves the HDX-5696 goal with no perf regression (sargability comes from the PK/partition predicates; 🟡 P2 — recommended
🔵 P3 nitpicks (1)
Reviewers (6): ce-correctness-reviewer, ce-testing-reviewer, ce-maintainability-reviewer, ce-performance-reviewer, ce-kieran-typescript-reviewer, ce-previous-comments-reviewer. Testing gaps:
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 |
…kup (#3566) ## Summary Clicking a search result could open the wrong row's details when the table persists only one of `_block_number` / `_block_offset`. - Since #3364 the side panel used a key-only lookup (primary key + partition + whichever block column exists) when at least one block column was persisted. One block column is not unique: `_block_offset` repeats across insert blocks, `_block_number` across the rows of one block, and the primary key often doesn't break the tie (the OTel exporter default keys on the second-precision `TimestampTime`). - Since #3530 the block columns are also detected from server-level `merge_tree_settings`, so on servers that enable only one of them, every table without explicit settings takes this path. - The key-only lookup now requires both block columns. A lone block column is still appended to the full-row lookup as a tiebreaker. - Tables that persist only one block column return to the full-row lookup and lose the #3364 speed-up. A `_part` + `_part_offset` lookup is the follow-up to restore it. ### How to reproduce **Starting point:** `yarn dev`. Set the server default on the dev ClickHouse and restart it: ```sh docker exec <ch-server> sh -c 'echo "<clickhouse><merge_tree><enable_block_offset_column>1</enable_block_offset_column></merge_tree></clickhouse>" > /etc/clickhouse-server/config.d/block_offset.xml' docker restart <ch-server> ``` Create an exporter-shaped table and insert three rows as separate inserts, two in the same second: ```sql CREATE TABLE default.repro_logs ( Timestamp DateTime64(9), TimestampTime DateTime DEFAULT toDateTime(Timestamp), ServiceName LowCardinality(String), SeverityText LowCardinality(String), Body String, LogAttributes Map(LowCardinality(String), String) ) ENGINE = MergeTree PARTITION BY toDate(TimestampTime) PRIMARY KEY (ServiceName, TimestampTime) ORDER BY (ServiceName, TimestampTime, Timestamp); INSERT INTO default.repro_logs (Timestamp, ServiceName, SeverityText, Body) VALUES ('2026-11-05 09:00:39.845', 'saruman', 'INFO', 'Response x_span_id=ESLMARS7L7'); INSERT INTO default.repro_logs (Timestamp, ServiceName, SeverityText, Body) VALUES ('2026-11-05 09:00:39.100', 'saruman', 'INFO', 'Request x_span_id=ESOPOZ7FCV'); INSERT INTO default.repro_logs (Timestamp, ServiceName, SeverityText, Body) VALUES ('2026-11-05 09:00:36.909', 'saruman', 'INFO', 'Request x_span_id=ESLMARS7L7'); ``` Add a log source for `default.repro_logs` (timestamp column `TimestampTime`), then search it for `"ESLMARS7L7"` over a range covering 2026-11-05 09:00 UTC. **On `main`:** 1. Click the `Response x_span_id=ESLMARS7L7` row a few times. 2. The side panel shows `Request x_span_id=ESOPOZ7FCV` about half the time. The query in `system.query_log` is `WHERE ServiceName='saruman' AND TimestampTime=... AND toDate(TimestampTime)=... AND _block_offset='0' LIMIT 1`. **On this branch:** 1. Click the same row. The panel always shows `Response x_span_id=ESLMARS7L7`; the query also filters on `Timestamp`, `SeverityText` and `Body`. ### How to test on Vercel preview N/A — needs a ClickHouse server default; the demo server's user cannot read `system.merge_tree_settings`. ### References - Related PRs: #3530, #3364, #2320, #2071
Summary
Opening log details on large text-log tables is much slower than the search itself (HDX-5696). The side panel already has a fast lookup that filters only on primary key + partition + block columns, but it was gated on the table persisting both
_block_numberand_block_offset. The affected table only setsenable_block_offset_column = 1, so every click fell back to the full-row WHERE, which filters on every selected column (includingBody) and triggers expensive index loading.This PR relaxes the gate: the fast path is taken when the table persists either block column, and the WHERE uses whichever of
_block_number/_block_offsetis available. Combined with the primary key, either one is unique enough in practice: two rows would need identical key values (including a nanosecond timestamp) and the same insert block or the same offset within their blocks. Tables that persist both columns keep the current behavior; tables with neither still fall back to__hdx_id/ the full row.Changes:
getPersistedBlockColumns(engineFull)inDBRowTable.tsxdecides which block columns to inject;useConfigWithAdditionalSelectuses it for both the extra SELECT keys androwKeyColumns.renderHookcoverage ofuseConfigWithAdditionalSelect(offset only, number only, both,__hdx_idfallback, nosourceId).@hyperdx/app.Known trade-off: rows with identical PK that also share the block number (or the offset) still collide and
LIMIT 1picks one; the thread on the Linear issue accepted that given nanosecond timestamps.Verified against ClickHouse 26.8 with tables that set only one of the two settings:
engine_fullexposes the setting as matched here, the persisted column survivesOPTIMIZE FINALunchanged, and the WHERE shape HyperDX now generates (PK expressions + the block column, noBody) returns the clicked row.How to test on Vercel preview
N/A — the demo tables persist both block columns, so the fast path was already taken there; this change only affects tables that persist one of them. To verify locally, create a table with
SETTINGS enable_block_offset_column = 1(orenable_block_number_column = 1) alone, add it as a log source, click a row and check the side-panel query filters on PK columns + that block column only.References
Linear Issue: HDX-5696