From ebafcb3616714b38fb4724fb4e8cf7687af69ee4 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 13 Oct 2026 16:58:11 +0000 Subject: [PATCH 1/2] fix(app): resolve row details with PK + _block_offset when only enable_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 --- .changeset/row-lookup-block-offset.md | 7 + packages/app/src/components/DBRowTable.tsx | 33 ++-- .../components/__tests__/DBRowTable.test.tsx | 161 +++++++++++++++++- 3 files changed, 189 insertions(+), 12 deletions(-) create mode 100644 .changeset/row-lookup-block-offset.md diff --git a/.changeset/row-lookup-block-offset.md b/.changeset/row-lookup-block-offset.md new file mode 100644 index 0000000000..b48b635d10 --- /dev/null +++ b/.changeset/row-lookup-block-offset.md @@ -0,0 +1,7 @@ +--- +'@hyperdx/app': patch +--- + +fix: resolve row details with primary key + `_block_offset` when a table only +persists `enable_block_offset_column`, instead of falling back to the slow +full-row lookup that required `enable_block_number_column` too diff --git a/packages/app/src/components/DBRowTable.tsx b/packages/app/src/components/DBRowTable.tsx index e25d11bc5e..dfdb095e58 100644 --- a/packages/app/src/components/DBRowTable.tsx +++ b/packages/app/src/components/DBRowTable.tsx @@ -1482,6 +1482,21 @@ export function appendSelectWithAdditionalKeys( } } +// Block columns are only stable row identifiers when the table persists them +// (the virtual versions are renumbered on merge). _block_offset + primary key +// is unique enough on its own: two rows would need identical key values (incl. +// a nanosecond timestamp) and the same offset within their insert blocks. So +// _block_number is used when persisted but not required, which lets tables +// with only enable_block_offset_column = 1 skip the slow full-row lookup. +export function getPersistedBlockColumns(engineFull: string): string[] { + if (!engineFull.includes('enable_block_offset_column = 1')) { + return []; + } + return engineFull.includes('enable_block_number_column = 1') + ? ['_block_number', '_block_offset'] + : ['_block_offset']; +} + function getSelectLength(select: SelectList): number { if (typeof select === 'string') { return select.split(',').filter(s => s.trim().length > 0).length; @@ -1520,16 +1535,16 @@ export function useConfigWithAdditionalSelect( } let extraKeys: string[] = []; + let hasBlockColumns = false; if (sourceId) { - const engineFull = tableMetadata?.engine_full ?? ''; - - const hasBlockColumns = - engineFull.includes('enable_block_number_column = 1') && - engineFull.includes('enable_block_offset_column = 1'); + const blockColumns = getPersistedBlockColumns( + tableMetadata?.engine_full ?? '', + ); - if (hasBlockColumns) { - extraKeys = ['_block_number', '_block_offset']; + if (blockColumns.length > 0) { + extraKeys = blockColumns; + hasBlockColumns = true; } else if (columns?.some(c => c.name === '__hdx_id')) { extraKeys = ['__hdx_id']; } @@ -1548,10 +1563,6 @@ export function useConfigWithAdditionalSelect( // column references like Timestamp, ServiceName) so the row WHERE clause // can use only these, avoiding expensive index loading on large columns // like Body. - const hasBlockColumns = - extraKeys.includes('_block_number') && - extraKeys.includes('_block_offset'); - const rowKeyColumns = hasBlockColumns ? new Set([ ...splitAndTrimWithBracket(partitionKey), diff --git a/packages/app/src/components/__tests__/DBRowTable.test.tsx b/packages/app/src/components/__tests__/DBRowTable.test.tsx index 408fc50b2f..765108d126 100644 --- a/packages/app/src/components/__tests__/DBRowTable.test.tsx +++ b/packages/app/src/components/__tests__/DBRowTable.test.tsx @@ -1,11 +1,14 @@ import { Provider } from 'jotai'; import type { ReactElement } from 'react'; -import { screen, waitFor, within } from '@testing-library/react'; +import type { BuilderChartConfigWithDateRange } from '@hyperdx/common-utils/dist/types'; +import { renderHook, screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { appendSelectWithAdditionalKeys, + getPersistedBlockColumns, RawLogTable, + useConfigWithAdditionalSelect, } from '@/components/DBRowTable'; import * as useChartConfigModule from '@/hooks/useChartConfig'; import { RowWhereResult } from '@/hooks/useRowWhere'; @@ -13,6 +16,22 @@ import { RowClickAction, UserPreferences } from '@/useUserPreferences'; const mockSetQueryState = jest.fn(); +// useConfigWithAdditionalSelect reads table metadata through react-query +// hooks that return a large UseQueryResult; the hook only reads `data`. +// Reading the handles back through `requireMock` keeps them typed as mocks +// without asserting a complete UseQueryResult. +jest.mock('@/hooks/useMetadata', () => ({ + ...jest.requireActual('@/hooks/useMetadata'), + useTableMetadata: jest.fn(), + useColumns: jest.fn(), +})); + +const { useTableMetadata: mockUseTableMetadata, useColumns: mockUseColumns } = + jest.requireMock<{ + useTableMetadata: jest.Mock; + useColumns: jest.Mock; + }>('@/hooks/useMetadata'); + // The inline expanded row reads the row query params through nuqs, which needs // a mounted Next router. Routing every setter through one mock lets tests assert // which param the expanded row's side panel button writes. @@ -774,3 +793,143 @@ describe('appendSelectWithAdditionalKeys', () => { }); }); }); + +describe('getPersistedBlockColumns', () => { + const engine = (settings: string) => + `MergeTree PARTITION BY toDate(Timestamp) ORDER BY (ServiceName, Timestamp) SETTINGS index_granularity = 8192${settings}`; + + it('returns both block columns when both are persisted', () => { + expect( + getPersistedBlockColumns( + engine( + ', enable_block_number_column = 1, enable_block_offset_column = 1', + ), + ), + ).toEqual(['_block_number', '_block_offset']); + }); + + it('returns only _block_offset when only enable_block_offset_column is set', () => { + expect( + getPersistedBlockColumns(engine(', enable_block_offset_column = 1')), + ).toEqual(['_block_offset']); + }); + + it('returns nothing when only enable_block_number_column is set', () => { + expect( + getPersistedBlockColumns(engine(', enable_block_number_column = 1')), + ).toEqual([]); + }); + + it('returns nothing when neither setting is present', () => { + expect(getPersistedBlockColumns(engine(''))).toEqual([]); + expect(getPersistedBlockColumns('')).toEqual([]); + }); +}); + +describe('useConfigWithAdditionalSelect', () => { + const config: BuilderChartConfigWithDateRange = { + connection: 'conn', + from: { databaseName: 'default', tableName: 'otel_logs' }, + select: 'Timestamp, ServiceName, Body', + where: '', + whereLanguage: 'sql', + timestampValueExpression: 'Timestamp', + dateRange: [new Date(0), new Date(1)], + }; + + const mockMetadata = ({ + engineFull, + columns = [], + }: { + engineFull: string; + columns?: { name: string; type: string }[]; + }) => { + mockUseTableMetadata.mockReturnValue({ + data: { + primary_key: 'toStartOfFiveMinutes(Timestamp), ServiceName, Timestamp', + partition_key: 'toDate(Timestamp)', + engine_full: engineFull, + }, + }); + mockUseColumns.mockReturnValue({ data: columns }); + }; + + afterEach(() => { + mockUseTableMetadata.mockReset(); + mockUseColumns.mockReset(); + }); + + it('restricts the row lookup to PK + _block_offset when only the offset column is persisted', () => { + mockMetadata({ + engineFull: + 'MergeTree PARTITION BY toDate(Timestamp) ORDER BY (toStartOfFiveMinutes(Timestamp), ServiceName, Timestamp) SETTINGS index_granularity = 8192, enable_block_offset_column = 1', + }); + + const { result } = renderHook(() => + useConfigWithAdditionalSelect(config, 'source-id'), + ); + + expect(result.current?.select).toBe( + 'Timestamp,ServiceName,Body,toDate(Timestamp),toStartOfFiveMinutes(Timestamp),_block_offset', + ); + expect(result.current?.additionalKeysLength).toBe(3); + expect(result.current?.rowKeyColumns).toEqual( + new Set([ + 'toDate(Timestamp)', + 'toStartOfFiveMinutes(Timestamp)', + 'ServiceName', + 'Timestamp', + '_block_offset', + ]), + ); + expect(result.current?.rowKeyColumns?.has('Body')).toBe(false); + }); + + it('uses both block columns when both are persisted', () => { + mockMetadata({ + engineFull: + 'MergeTree ORDER BY Timestamp SETTINGS enable_block_number_column = 1, enable_block_offset_column = 1', + }); + + const { result } = renderHook(() => + useConfigWithAdditionalSelect(config, 'source-id'), + ); + + expect(result.current?.select).toBe( + 'Timestamp,ServiceName,Body,toDate(Timestamp),toStartOfFiveMinutes(Timestamp),_block_number,_block_offset', + ); + expect(result.current?.rowKeyColumns?.has('_block_number')).toBe(true); + expect(result.current?.rowKeyColumns?.has('_block_offset')).toBe(true); + }); + + it('falls back to __hdx_id and the full row when no block column is persisted', () => { + mockMetadata({ + engineFull: + 'MergeTree ORDER BY Timestamp SETTINGS index_granularity = 8192', + columns: [{ name: '__hdx_id', type: 'String' }], + }); + + const { result } = renderHook(() => + useConfigWithAdditionalSelect(config, 'source-id'), + ); + + expect(result.current?.select).toBe( + 'Timestamp,ServiceName,Body,toDate(Timestamp),toStartOfFiveMinutes(Timestamp),__hdx_id', + ); + expect(result.current?.rowKeyColumns).toBeUndefined(); + }); + + it('ignores block columns for aggregate queries without a sourceId', () => { + mockMetadata({ + engineFull: + 'MergeTree ORDER BY Timestamp SETTINGS enable_block_offset_column = 1', + }); + + const { result } = renderHook(() => useConfigWithAdditionalSelect(config)); + + expect(result.current?.select).toBe( + 'Timestamp,ServiceName,Body,toDate(Timestamp),toStartOfFiveMinutes(Timestamp)', + ); + expect(result.current?.rowKeyColumns).toBeUndefined(); + }); +}); From 7dbc6fe361e3ca35038c7f2aba63acab41aede3d Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 13 Oct 2026 17:04:24 +0000 Subject: [PATCH 2/2] fix(app): also use PK + _block_number when only enable_block_number_column is set Co-authored-by: Aaron Knudtson --- .changeset/row-lookup-block-offset.md | 7 +++--- packages/app/src/components/DBRowTable.tsx | 22 ++++++++++--------- .../components/__tests__/DBRowTable.test.tsx | 21 ++++++++++++++++-- 3 files changed, 35 insertions(+), 15 deletions(-) diff --git a/.changeset/row-lookup-block-offset.md b/.changeset/row-lookup-block-offset.md index b48b635d10..5328e0257b 100644 --- a/.changeset/row-lookup-block-offset.md +++ b/.changeset/row-lookup-block-offset.md @@ -2,6 +2,7 @@ '@hyperdx/app': patch --- -fix: resolve row details with primary key + `_block_offset` when a table only -persists `enable_block_offset_column`, instead of falling back to the slow -full-row lookup that required `enable_block_number_column` too +fix: resolve row details with the primary key plus whichever of `_block_number` +/ `_block_offset` the table persists, instead of falling back to the slow +full-row lookup when only one of `enable_block_number_column` / +`enable_block_offset_column` is set diff --git a/packages/app/src/components/DBRowTable.tsx b/packages/app/src/components/DBRowTable.tsx index dfdb095e58..3e9a176803 100644 --- a/packages/app/src/components/DBRowTable.tsx +++ b/packages/app/src/components/DBRowTable.tsx @@ -1483,18 +1483,20 @@ export function appendSelectWithAdditionalKeys( } // Block columns are only stable row identifiers when the table persists them -// (the virtual versions are renumbered on merge). _block_offset + primary key -// is unique enough on its own: two rows would need identical key values (incl. -// a nanosecond timestamp) and the same offset within their insert blocks. So -// _block_number is used when persisted but not required, which lets tables -// with only enable_block_offset_column = 1 skip the slow full-row lookup. +// (the virtual versions are renumbered on merge). Either one combined with the +// primary key is unique enough: two rows would need identical key values +// (incl. a nanosecond timestamp) plus the same insert block or the same offset +// within their blocks. So a table that persists only one of them still skips +// the slow full-row lookup. export function getPersistedBlockColumns(engineFull: string): string[] { - if (!engineFull.includes('enable_block_offset_column = 1')) { - return []; + const columns: string[] = []; + if (engineFull.includes('enable_block_number_column = 1')) { + columns.push('_block_number'); } - return engineFull.includes('enable_block_number_column = 1') - ? ['_block_number', '_block_offset'] - : ['_block_offset']; + if (engineFull.includes('enable_block_offset_column = 1')) { + columns.push('_block_offset'); + } + return columns; } function getSelectLength(select: SelectList): number { diff --git a/packages/app/src/components/__tests__/DBRowTable.test.tsx b/packages/app/src/components/__tests__/DBRowTable.test.tsx index 765108d126..c4cf31d189 100644 --- a/packages/app/src/components/__tests__/DBRowTable.test.tsx +++ b/packages/app/src/components/__tests__/DBRowTable.test.tsx @@ -814,10 +814,10 @@ describe('getPersistedBlockColumns', () => { ).toEqual(['_block_offset']); }); - it('returns nothing when only enable_block_number_column is set', () => { + it('returns only _block_number when only enable_block_number_column is set', () => { expect( getPersistedBlockColumns(engine(', enable_block_number_column = 1')), - ).toEqual([]); + ).toEqual(['_block_number']); }); it('returns nothing when neither setting is present', () => { @@ -885,6 +885,23 @@ describe('useConfigWithAdditionalSelect', () => { expect(result.current?.rowKeyColumns?.has('Body')).toBe(false); }); + it('restricts the row lookup to PK + _block_number when only the number column is persisted', () => { + mockMetadata({ + engineFull: + 'MergeTree ORDER BY Timestamp SETTINGS enable_block_number_column = 1', + }); + + const { result } = renderHook(() => + useConfigWithAdditionalSelect(config, 'source-id'), + ); + + expect(result.current?.select).toBe( + 'Timestamp,ServiceName,Body,toDate(Timestamp),toStartOfFiveMinutes(Timestamp),_block_number', + ); + expect(result.current?.rowKeyColumns?.has('_block_number')).toBe(true); + expect(result.current?.rowKeyColumns?.has('Body')).toBe(false); + }); + it('uses both block columns when both are persisted', () => { mockMetadata({ engineFull: