fix(app): resolve row details with PK + whichever block column the table persists

.changeset/row-lookup-block-offset.md

@@ -0,0 +1,8 @@
+---
+'@hyperdx/app': patch
+---
+
+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

packages/app/src/components/DBRowTable.tsx

@@ -1482,6 +1482,23 @@ export function appendSelectWithAdditionalKeys(
   }
 }
 
+// Block columns are only stable row identifiers when the table persists them
+// (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[] {
+  const columns: string[] = [];
+  if (engineFull.includes('enable_block_number_column = 1')) {
+    columns.push('_block_number');
+  }
+  if (engineFull.includes('enable_block_offset_column = 1')) {
+    columns.push('_block_offset');
+  }
+  return columns;
+}
+
 function getSelectLength(select: SelectList): number {
   if (typeof select === 'string') {
     return select.split(',').filter(s => s.trim().length > 0).length;
@@ -1520,16 +1537,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 +1565,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),

packages/app/src/components/__tests__/DBRowTable.test.tsx

@@ -1,18 +1,37 @@
 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';
 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,160 @@ 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 only _block_number when only enable_block_number_column is set', () => {
+    expect(
+      getPersistedBlockColumns(engine(', enable_block_number_column = 1')),
+    ).toEqual(['_block_number']);
+  });
+
+  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('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:
+        '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();
+  });
+});