From a5ce650dc8817b1b6c0e554a8ef10446091372b8 Mon Sep 17 00:00:00 2001 From: vinzee Date: Wed, 14 Oct 2026 22:02:01 -0700 Subject: [PATCH 1/2] fix: v2 search drops rows after a sub-second startTime; make maxResults cap configurable A DateTime column holds second-truncated times, so comparing it to a millisecond start bound (TimestampTime >= 22:03:21.827) drops every row in that second even when the precise Timestamp column would match. Floor the start bound to the second for DateTime columns and keep it inclusive, mirroring the existing Date and toStartOf* handling. Add EXTERNAL_API_SEARCH_MAX_RESULTS (default 2000, the previous hardcoded cap) so deployments with bulk readers can raise the /api/v2/search page size. Invalid values fall back to the default, like EXTERNAL_API_RATE_LIMIT_MAX. --- .../v2-search-subsecond-start-max-results.md | 6 + packages/api/src/__tests__/config.test.ts | 39 ++++++ packages/api/src/config.ts | 11 ++ .../api/src/routers/external-api/v2/search.ts | 12 +- .../src/__tests__/renderChartConfig.test.ts | 127 +++++++++++++++++- .../src/core/renderChartConfig.ts | 74 ++++++++-- 6 files changed, 252 insertions(+), 17 deletions(-) create mode 100644 .changeset/v2-search-subsecond-start-max-results.md diff --git a/.changeset/v2-search-subsecond-start-max-results.md b/.changeset/v2-search-subsecond-start-max-results.md new file mode 100644 index 0000000000..3378d287d2 --- /dev/null +++ b/.changeset/v2-search-subsecond-start-max-results.md @@ -0,0 +1,6 @@ +--- +'@hyperdx/common-utils': patch +'@hyperdx/api': patch +--- + +fix: Floor the time filter start to the precision of a coarse timestamp column (DateTime, DateTime64 below ms) when a finer column also filters the range, so a sub-second start no longer drops the rest of that tick; make the external API v2 search `maxResults` cap configurable via `EXTERNAL_API_SEARCH_MAX_RESULTS` (defaults to 2000, the previous hardcoded value) diff --git a/packages/api/src/__tests__/config.test.ts b/packages/api/src/__tests__/config.test.ts index 88f17a9d71..c006077783 100644 --- a/packages/api/src/__tests__/config.test.ts +++ b/packages/api/src/__tests__/config.test.ts @@ -124,6 +124,45 @@ describe('config', () => { ); }); + describe('EXTERNAL_API_SEARCH_MAX_RESULTS', () => { + const ORIGINAL = process.env.EXTERNAL_API_SEARCH_MAX_RESULTS; + + afterEach(() => { + if (ORIGINAL === undefined) { + delete process.env.EXTERNAL_API_SEARCH_MAX_RESULTS; + } else { + process.env.EXTERNAL_API_SEARCH_MAX_RESULTS = ORIGINAL; + } + jest.resetModules(); + }); + + const loadMaxResults = () => { + let loaded!: typeof import('@/config'); + jest.isolateModules(() => { + loaded = jest.requireActual('@/config'); + }); + return loaded.EXTERNAL_API_SEARCH_MAX_RESULTS; + }; + + it('defaults to 2000 when unset', () => { + delete process.env.EXTERNAL_API_SEARCH_MAX_RESULTS; + expect(loadMaxResults()).toBe(2000); + }); + + it('accepts a valid positive integer', () => { + process.env.EXTERNAL_API_SEARCH_MAX_RESULTS = '10000'; + expect(loadMaxResults()).toBe(10_000); + }); + + it.each(['abc', '0', '50', '-5', '100rows', '', '0.5', 'Infinity'])( + 'falls back to 2000 for %p', + raw => { + process.env.EXTERNAL_API_SEARCH_MAX_RESULTS = raw; + expect(loadMaxResults()).toBe(2000); + }, + ); + }); + describe('EXPRESS_SESSION_SECRET', () => { const ORIGINAL_SECRET = process.env.EXPRESS_SESSION_SECRET; const ORIGINAL_LOCAL_APP_MODE = process.env.IS_LOCAL_APP_MODE; diff --git a/packages/api/src/config.ts b/packages/api/src/config.ts index fb920c9159..9ac7104ac9 100644 --- a/packages/api/src/config.ts +++ b/packages/api/src/config.ts @@ -76,6 +76,17 @@ export const EXTERNAL_API_RATE_LIMIT_MAX = (() => { return Number.isSafeInteger(parsed) && parsed > 0 ? parsed : 100; })(); +export const EXTERNAL_API_SEARCH_DEFAULT_MAX_RESULTS = 100; + +export const EXTERNAL_API_SEARCH_MAX_RESULTS = (() => { + const parsed = Number(env.EXTERNAL_API_SEARCH_MAX_RESULTS); + // A cap below the default would make requests without maxResults invalid. + return Number.isSafeInteger(parsed) && + parsed >= EXTERNAL_API_SEARCH_DEFAULT_MAX_RESULTS + ? parsed + : 2000; +})(); + // FOR CI ONLY export const CLICKHOUSE_HOST = env.CLICKHOUSE_HOST as string; export const CLICKHOUSE_USER = env.CLICKHOUSE_USER as string; diff --git a/packages/api/src/routers/external-api/v2/search.ts b/packages/api/src/routers/external-api/v2/search.ts index dfc98b1196..ede7ed6676 100644 --- a/packages/api/src/routers/external-api/v2/search.ts +++ b/packages/api/src/routers/external-api/v2/search.ts @@ -3,6 +3,10 @@ import express from 'express'; import { ObjectId } from 'mongodb'; import { z } from 'zod'; +import { + EXTERNAL_API_SEARCH_DEFAULT_MAX_RESULTS, + EXTERNAL_API_SEARCH_MAX_RESULTS, +} from '@/config'; import { parseTimeRange } from '@/mcp/tools/query/helpers'; import { getNonNullUserWithTeam } from '@/middleware/auth'; import { processRequestWithEnhancedErrors as validateRequest } from '@/utils/enhancedErrors'; @@ -223,10 +227,12 @@ const searchRequestSchema = z.object({ .number() .int() .min(1) - .max(2000) + .max(EXTERNAL_API_SEARCH_MAX_RESULTS) .optional() - .default(100) - .describe('Maximum rows to return (1-2000). Default: 100.'), + .default(EXTERNAL_API_SEARCH_DEFAULT_MAX_RESULTS) + .describe( + `Maximum rows to return (1-${EXTERNAL_API_SEARCH_MAX_RESULTS}). Default: ${EXTERNAL_API_SEARCH_DEFAULT_MAX_RESULTS}.`, + ), offset: z .number() .int() diff --git a/packages/common-utils/src/__tests__/renderChartConfig.test.ts b/packages/common-utils/src/__tests__/renderChartConfig.test.ts index 8e54b666cf..d210341aac 100644 --- a/packages/common-utils/src/__tests__/renderChartConfig.test.ts +++ b/packages/common-utils/src/__tests__/renderChartConfig.test.ts @@ -2650,7 +2650,9 @@ describe('renderChartConfig', () => { mockMetadata.getColumn.mockImplementation(async ({ column }) => column === 'date' ? ({ type: 'Date' } as ColumnMeta) - : ({ type: 'DateTime' } as ColumnMeta), + : column === 'TimestampTime' + ? ({ type: 'DateTime' } as ColumnMeta) + : ({ type: 'DateTime64(3)' } as ColumnMeta), ); }); @@ -2759,6 +2761,129 @@ describe('renderChartConfig', () => { ); }); + it.each([ + ['inclusive', true], + ['exclusive', false], + ])( + 'floors a sub-second %s start on a DateTime column and keeps it inclusive', + async (_, dateRangeStartInclusive) => { + const start = new Date('2026-10-13T22:03:21.827Z'); + const end = new Date('2026-10-13T22:05:00.500Z'); + + const actual = await timeFilterExpr({ + timestampValueExpression: 'TimestampTime, Timestamp', + dateRangeEndInclusive: false, + dateRangeStartInclusive, + dateRange: [start, end], + connectionId: 'test-connection', + databaseName: 'default', + tableName: 'target_table', + metadata: mockMetadata, + }); + + const precise = dateRangeStartInclusive ? '>=' : '>'; + expect(parameterizedQueryToSql(actual)).toBe( + `(TimestampTime >= fromUnixTimestamp64Milli(${new Date('2026-10-13T22:03:21Z').getTime()}) AND TimestampTime < fromUnixTimestamp64Milli(${end.getTime()}))AND(Timestamp ${precise} fromUnixTimestamp64Milli(${start.getTime()}) AND Timestamp < fromUnixTimestamp64Milli(${end.getTime()}))`, + ); + }, + ); + + it.each([ + ['inclusive', true, '>='], + ['exclusive', false, '>'], + ])( + 'leaves a %s start unfloored on a DateTime-only source so adjacent windows do not overlap', + async (_, dateRangeStartInclusive, op) => { + const start = new Date('2026-10-13T22:03:21.827Z'); + const end = new Date('2026-10-13T22:05:00Z'); + + const actual = await timeFilterExpr({ + timestampValueExpression: 'TimestampTime', + dateRangeEndInclusive: true, + dateRangeStartInclusive, + dateRange: [start, end], + connectionId: 'test-connection', + databaseName: 'default', + tableName: 'target_table', + metadata: mockMetadata, + }); + + expect(parameterizedQueryToSql(actual)).toBe( + `(TimestampTime ${op} fromUnixTimestamp64Milli(${start.getTime()}) AND TimestampTime <= fromUnixTimestamp64Milli(${end.getTime()}))`, + ); + }, + ); + + it.each([ + ['DateTime', 'toDateTime($__fromTime_ms)'], + ['DateTime64(1)', 'toDateTime64($__fromTime_ms, 1)'], + ])( + 'floors the start macro on a %s column in a raw SQL template', + async (type, expectedStart) => { + mockMetadata.getColumn.mockImplementation(async ({ column }) => + column === 'coarse' + ? ({ type } as ColumnMeta) + : ({ type: 'DateTime64(9)' } as ColumnMeta), + ); + + const actual = await timeFilterExpr({ + timestampValueExpression: 'coarse, Timestamp', + dateRangeEndInclusive: true, + dateRangeStartInclusive: true, + dateRange: [ + new Date('2026-10-13T22:03:21.827Z'), + new Date('2026-10-13T22:05:00Z'), + ], + isRenderingRawSqlTemplate: true, + connectionId: 'test-connection', + databaseName: 'default', + tableName: 'target_table', + metadata: mockMetadata, + }); + + expect(parameterizedQueryToSql(actual)).toBe( + `(coarse >= ${expectedStart} AND coarse <= $__toTime_ms)AND(Timestamp >= $__fromTime_ms AND Timestamp <= $__toTime_ms)`, + ); + }, + ); + + it.each([ + ['DateTime', '22:03:21.000'], + ["DateTime('UTC')", '22:03:21.000'], + ['Nullable(DateTime)', '22:03:21.000'], + ['DateTime64(0)', '22:03:21.000'], + ["DateTime64(0, 'UTC')", '22:03:21.000'], + ['DateTime64(1)', '22:03:21.800'], + ['DateTime64(2)', '22:03:21.820'], + ['DateTime64(3)', '22:03:21.827'], + ])( + 'floors the start bound of a %s column to %s when a finer column exists', + async (type, flooredStart) => { + mockMetadata.getColumn.mockImplementation(async ({ column }) => + column === 'coarse' + ? ({ type } as ColumnMeta) + : ({ type: 'DateTime64(9)' } as ColumnMeta), + ); + const start = new Date('2026-10-13T22:03:21.827Z'); + const end = new Date('2026-10-13T22:05:00Z'); + + const actual = await timeFilterExpr({ + timestampValueExpression: 'coarse, Timestamp', + dateRangeEndInclusive: true, + dateRangeStartInclusive: true, + dateRange: [start, end], + connectionId: 'test-connection', + databaseName: 'default', + tableName: 'target_table', + metadata: mockMetadata, + }); + + expect(parameterizedQueryToSql(actual)).toBe( + `(coarse >= fromUnixTimestamp64Milli(${new Date(`2026-10-13T${flooredStart}Z`).getTime()}) AND coarse <= fromUnixTimestamp64Milli(${end.getTime()}))AND(Timestamp >= fromUnixTimestamp64Milli(${start.getTime()}) AND Timestamp <= fromUnixTimestamp64Milli(${end.getTime()}))`, + ); + }, + ); + it.each(['Date32', 'Nullable(Date)', 'LowCardinality(Date)'])( 'wraps a %s column in toDate() and keeps the bounds inclusive', async type => { diff --git a/packages/common-utils/src/core/renderChartConfig.ts b/packages/common-utils/src/core/renderChartConfig.ts index 70dab887c4..db9835ea61 100644 --- a/packages/common-utils/src/core/renderChartConfig.ts +++ b/packages/common-utils/src/core/renderChartConfig.ts @@ -12,6 +12,7 @@ import { } from '@/core/histogram'; import { Metadata, unquoteIdentifier } from '@/core/metadata'; import { + classifyTimestampType, convertDateRangeToGranularityString, convertGranularityToSeconds, extractSettingsClauseFromEnd, @@ -951,7 +952,7 @@ export async function timeFilterExpr({ optimizedTimestampValueExpression, ); - const whereExprs = await Promise.all( + const columns = await Promise.all( valueExpressions.map(async expr => { const col = expr.trim(); @@ -982,23 +983,67 @@ export async function timeFilterExpr({ connectionId, }); - const unsafeTimestampValueExpression = { - UNSAFE_RAW_SQL: col, - }; - if (columnMeta == null && !skipColumnLookup) { console.warn( `Column ${col} not found in ${databaseName}.${tableName} while inferring type for time filter`, ); } + return { + col, + toStartOf, + isToDateExpr, + columnType: stripTypeWrappers(columnMeta?.type ?? ''), + }; + }), + ); + + const MS_PRECISION = 3; + const timePrecisionOf = (type: string) => { + const classified = classifyTimestampType(type); + return classified && classified.kind !== 'date' + ? classified.precision + : null; + }; + const hasPreciseColumn = columns.some( + ({ toStartOf, isToDateExpr, columnType }) => + !toStartOf && + !isToDateExpr && + (timePrecisionOf(columnType) ?? -1) >= MS_PRECISION, + ); + + const whereExprs = columns.map( + ({ col, toStartOf, isToDateExpr, columnType }) => { + const unsafeTimestampValueExpression = { + UNSAFE_RAW_SQL: col, + }; + + // A column coarser than the ms bound drops the rest of the start's tick, + // so floor to its precision. Only when a finer column keeps the exact + // bound; otherwise adjacent windows would share the floored rows. + const columnPrecision = timePrecisionOf(columnType); + const flooredPrecision = + hasPreciseColumn && + columnPrecision != null && + columnPrecision < MS_PRECISION + ? columnPrecision + : null; + const precisionStepMs = + flooredPrecision == null ? 1 : 10 ** (MS_PRECISION - flooredPrecision); + const columnStartTime = + Math.floor(startTime / precisionStepMs) * precisionStepMs; + const rawStartBound = isRenderingRawSqlTemplate ? includedDataInterval ? chSql`toStartOfInterval($__fromTime_ms, INTERVAL $__interval_s second) - INTERVAL $__interval_s second` - : chSql`$__fromTime_ms` + : flooredPrecision == null + ? chSql`$__fromTime_ms` + : flooredPrecision === 0 + ? chSql`toDateTime($__fromTime_ms)` + : chSql`toDateTime64($__fromTime_ms, ${{ UNSAFE_RAW_SQL: String(flooredPrecision) }})` : includedDataInterval - ? chSql`toStartOfInterval(fromUnixTimestamp64Milli(${{ Int64: startTime }}), INTERVAL ${includedDataInterval}) - INTERVAL ${includedDataInterval}` - : chSql`fromUnixTimestamp64Milli(${{ Int64: startTime }})`; + ? chSql`toStartOfInterval(fromUnixTimestamp64Milli(${{ Int64: columnStartTime }}), INTERVAL ${includedDataInterval}) - INTERVAL ${includedDataInterval}` + : chSql`fromUnixTimestamp64Milli(${{ Int64: columnStartTime }})`; const rawEndBound = isRenderingRawSqlTemplate ? includedDataInterval @@ -1016,13 +1061,16 @@ export async function timeFilterExpr({ ? chSql`${toStartOf.function}(${rawEndBound}${toStartOf.formattedRemainingArgs})` : rawEndBound; - const isDateType = - /^Date(?:32)?$/i.test(stripTypeWrappers(columnMeta?.type ?? '')) || - isToDateExpr; + const isDateType = /^Date(?:32)?$/i.test(columnType) || isToDateExpr; // toStartOf* and Date filters must stay inclusive — strict < on a rounded value drops a whole interval const startOp = - dateRangeStartInclusive || toStartOf || isDateType ? '>=' : '>'; + dateRangeStartInclusive || + toStartOf || + isDateType || + flooredPrecision != null + ? '>=' + : '>'; const endOp = dateRangeEndInclusive || toStartOf || isDateType ? '<=' : '<'; @@ -1031,7 +1079,7 @@ export async function timeFilterExpr({ } else { return chSql`(${unsafeTimestampValueExpression} ${startOp} ${startTimeCond} AND ${unsafeTimestampValueExpression} ${endOp} ${endTimeCond})`; } - }), + }, ); return concatChSql('AND', ...whereExprs); From 9fe3e367472e019b41e3562aabfea69b482d69dc Mon Sep 17 00:00:00 2001 From: Drew Davis Date: Fri, 16 Oct 2026 07:59:10 -0400 Subject: [PATCH 2/2] test: Add additional timeFilterExpr tests --- .../__tests__/queryChartConfig.int.test.ts | 75 +++++++++++++++++++ .../src/__tests__/renderChartConfig.test.ts | 58 ++++++++++++++ 2 files changed, 133 insertions(+) diff --git a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts index 427f7f6f76..bba523e136 100644 --- a/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts +++ b/packages/common-utils/src/__tests__/queryChartConfig.int.test.ts @@ -2924,4 +2924,79 @@ describe('queryChartConfig Integration Tests', () => { }); }); }); + + // Mirrors the OTel logs schema, where a DateTime column precedes the + // DateTime64(9) column in the timestamp expression for primary-key pruning. + describe('sub-second start on a DateTime + DateTime64 source', () => { + const LOGS_TABLE = 'logs_subsecond_start_int_test'; + const start = new Date('2026-10-13T22:03:21.827Z'); + const end = new Date('2026-10-13T22:05:00Z'); + + const searchConfig = ( + dateRange: [Date, Date], + dateRangeEndInclusive = true, + ): ChartConfigWithOptDateRange => ({ + connection: 'test-connection', + from: { databaseName: DATABASE, tableName: LOGS_TABLE }, + select: 'Body', + where: '', + whereLanguage: 'sql', + timestampValueExpression: 'TimestampTime, Timestamp', + orderBy: 'Timestamp ASC', + dateRange, + dateRangeEndInclusive, + }); + + const bodies = async (config: ChartConfigWithOptDateRange) => { + const result = await hdxClient.queryChartConfig({ + config, + metadata, + querySettings: undefined, + }); + return (result.data as Array<{ Body: string }>).map(r => r.Body); + }; + + beforeAll(async () => { + await client.command({ + query: `CREATE OR REPLACE TABLE ${DATABASE}.${LOGS_TABLE} ( + Timestamp DateTime64(9, 'UTC'), + TimestampTime DateTime('UTC') DEFAULT toDateTime(Timestamp), + Body String + ) + ENGINE = MergeTree + ORDER BY (TimestampTime, Timestamp)`, + }); + await client.insert({ + table: `${DATABASE}.${LOGS_TABLE}`, + values: [ + { Timestamp: '2026-10-13 22:03:21.500000000', Body: 'before-start' }, + { Timestamp: '2026-10-13 22:03:21.900000000', Body: 'same-second' }, + { Timestamp: '2026-10-13 22:03:22.100000000', Body: 'next-second' }, + ], + format: 'JSONEachRow', + }); + }); + + afterAll(async () => { + await client.command({ + query: `DROP TABLE IF EXISTS ${DATABASE}.${LOGS_TABLE}`, + }); + }); + + it('returns rows in the rest of the start second', async () => { + expect(await bodies(searchConfig([start, end]))).toEqual([ + 'same-second', + 'next-second', + ]); + }); + + it('does not return the same row from the preceding window', async () => { + const previousWindow = searchConfig( + [new Date('2026-10-13T22:00:00Z'), start], + false, + ); + + expect(await bodies(previousWindow)).toEqual(['before-start']); + }); + }); }); diff --git a/packages/common-utils/src/__tests__/renderChartConfig.test.ts b/packages/common-utils/src/__tests__/renderChartConfig.test.ts index d210341aac..8cb1559bee 100644 --- a/packages/common-utils/src/__tests__/renderChartConfig.test.ts +++ b/packages/common-utils/src/__tests__/renderChartConfig.test.ts @@ -2884,6 +2884,64 @@ describe('renderChartConfig', () => { }, ); + describe('sub-second start flooring edge cases', () => { + const start = new Date('2026-10-13T22:03:21.827Z'); + const flooredStart = new Date('2026-10-13T22:03:21Z'); + const end = new Date('2026-10-13T22:05:00Z'); + + const render = ( + timestampValueExpression: string, + includedDataInterval?: string, + ) => + timeFilterExpr({ + timestampValueExpression, + dateRangeEndInclusive: true, + dateRangeStartInclusive: false, + dateRange: [start, end], + connectionId: 'test-connection', + databaseName: 'default', + tableName: 'target_table', + metadata: mockMetadata, + includedDataInterval, + }).then(parameterizedQueryToSql); + + it('does not floor when the finer column type cannot be resolved', async () => { + mockMetadata.getColumn.mockImplementation(async ({ column }) => + column === 'TimestampTime' + ? ({ type: 'DateTime' } as ColumnMeta) + : undefined, + ); + + expect(await render('TimestampTime, Timestamp')).toBe( + `(TimestampTime > fromUnixTimestamp64Milli(${start.getTime()}) AND TimestampTime <= fromUnixTimestamp64Milli(${end.getTime()}))AND(Timestamp > fromUnixTimestamp64Milli(${start.getTime()}) AND Timestamp <= fromUnixTimestamp64Milli(${end.getTime()}))`, + ); + }); + + it('does not treat a toStartOf column as the finer column', async () => { + expect(await render('toStartOfMinute(Timestamp), TimestampTime')).toBe( + `(toStartOfMinute(Timestamp) >= toStartOfMinute(fromUnixTimestamp64Milli(${start.getTime()})) AND toStartOfMinute(Timestamp) <= toStartOfMinute(fromUnixTimestamp64Milli(${end.getTime()})))AND(TimestampTime > fromUnixTimestamp64Milli(${start.getTime()}) AND TimestampTime <= fromUnixTimestamp64Milli(${end.getTime()}))`, + ); + }); + + it('floors the start inside the includedDataInterval expansion', async () => { + expect(await render('TimestampTime, Timestamp', '1 minute')).toBe( + `(TimestampTime >= toStartOfInterval(fromUnixTimestamp64Milli(${flooredStart.getTime()}), INTERVAL 1 minute) - INTERVAL 1 minute AND TimestampTime <= toStartOfInterval(fromUnixTimestamp64Milli(${end.getTime()}), INTERVAL 1 minute) + INTERVAL 1 minute)AND(Timestamp > toStartOfInterval(fromUnixTimestamp64Milli(${start.getTime()}), INTERVAL 1 minute) - INTERVAL 1 minute AND Timestamp <= toStartOfInterval(fromUnixTimestamp64Milli(${end.getTime()}), INTERVAL 1 minute) + INTERVAL 1 minute)`, + ); + }); + + it('treats a Nullable(DateTime64(9)) column as the finer column', async () => { + mockMetadata.getColumn.mockImplementation(async ({ column }) => + column === 'TimestampTime' + ? ({ type: 'DateTime' } as ColumnMeta) + : ({ type: 'Nullable(DateTime64(9))' } as ColumnMeta), + ); + + expect(await render('TimestampTime, Timestamp')).toBe( + `(TimestampTime >= fromUnixTimestamp64Milli(${flooredStart.getTime()}) AND TimestampTime <= fromUnixTimestamp64Milli(${end.getTime()}))AND(Timestamp > fromUnixTimestamp64Milli(${start.getTime()}) AND Timestamp <= fromUnixTimestamp64Milli(${end.getTime()}))`, + ); + }); + }); + it.each(['Date32', 'Nullable(Date)', 'LowCardinality(Date)'])( 'wraps a %s column in toDate() and keeps the bounds inclusive', async type => {