diff --git a/.changeset/range-open-exclusive-bounds.md b/.changeset/range-open-exclusive-bounds.md new file mode 100644 index 0000000000..dac633503a --- /dev/null +++ b/.changeset/range-open-exclusive-bounds.md @@ -0,0 +1,12 @@ +--- +'@hyperdx/common-utils': patch +--- + +Honor open (`*`), exclusive (`{}`) and non-numeric bounds in Lucene ranges + +`Duration:[* TO 500]` compiled to `Duration BETWEEN '*' AND 500`, which +ClickHouse rejects with `TYPE_MISMATCH`. Exclusive and half-open ranges such as +`Duration:{100 TO 500}` were all serialized as an inclusive `BETWEEN`. Bounds +were parsed with `parseFloat`, so `Timestamp:[2024-01-01 TO 2024-06-01]` became +`BETWEEN 2024 AND 2024` and matched nothing. The plain-English explanation of a +search now marks excluded bounds too. diff --git a/packages/common-utils/src/__tests__/queryParser.test.ts b/packages/common-utils/src/__tests__/queryParser.test.ts index 69fe3370a2..618e236917 100644 --- a/packages/common-utils/src/__tests__/queryParser.test.ts +++ b/packages/common-utils/src/__tests__/queryParser.test.ts @@ -665,6 +665,136 @@ describe('CustomSchemaSQLSerializerV2 - json', () => { }); }); +describe('CustomSchemaSQLSerializerV2 - range bounds', () => { + const metadata = getMetadata( + new ClickhouseClient({ host: 'http://localhost:8123' }), + ); + metadata.getColumn = jest.fn().mockImplementation(async ({ column }) => { + if (column === 'Duration') { + return { name: 'Duration', type: 'UInt64' }; + } else if (column === 'Timestamp') { + return { name: 'Timestamp', type: 'DateTime64(9)' }; + } else if (column === 'ServiceName') { + return { name: 'ServiceName', type: 'String' }; + } else if (column === 'LogAttributes') { + return { name: 'LogAttributes', type: 'Map' }; + } + return undefined; + }); + metadata.getMaterializedColumnsLookupTable = jest + .fn() + .mockImplementation(async () => new Map()); + + const databaseName = 'testName'; + const tableName = 'testTable'; + const connectionId = 'testId'; + const serializer = new CustomSchemaSQLSerializerV2({ + metadata, + databaseName, + tableName, + connectionId, + implicitColumnExpression: 'Body', + }); + + const rangeCases = [ + { + lucene: 'Duration:[100 TO 500]', + sql: '((Duration BETWEEN 100 AND 500))', + }, + { + lucene: 'Duration:[* TO 500]', + sql: '((Duration <= 500))', + }, + { + lucene: 'Duration:[100 TO *]', + sql: '((Duration >= 100))', + }, + { + lucene: '-Duration:[* TO 500]', + sql: '((NOT (Duration <= 500)))', + }, + { + lucene: 'ServiceName:[* TO *]', + sql: '(notEmpty(ServiceName) = 1)', + }, + { + lucene: 'Duration:{100 TO 500}', + sql: '((Duration > 100 AND Duration < 500))', + }, + { + lucene: 'Duration:[100 TO 500}', + sql: '((Duration >= 100 AND Duration < 500))', + }, + { + lucene: 'Duration:{100 TO 500]', + sql: '((Duration > 100 AND Duration <= 500))', + }, + { + lucene: '-Duration:{100 TO 500}', + sql: '((NOT (Duration > 100 AND Duration < 500)))', + }, + { + lucene: 'Timestamp:[2024-01-01 TO 2024-06-01]', + sql: "((Timestamp BETWEEN '2024-01-01' AND '2024-06-01'))", + }, + { + lucene: 'Timestamp:{2024-01-01 TO 2024-06-01}', + sql: "((Timestamp > '2024-01-01' AND Timestamp < '2024-06-01'))", + }, + { + lucene: 'LogAttributes.duration_ms:{100 TO 500}', + sql: "((`LogAttributes`['duration_ms'] > 100 AND `LogAttributes`['duration_ms'] < 500 AND indexHint(mapContains(`LogAttributes`, 'duration_ms'))))", + }, + ]; + + it.each(rangeCases)( + 'converts "$lucene" to SQL "$sql"', + async ({ lucene, sql }) => { + const builder = new SearchQueryBuilder(lucene, serializer); + expect(await builder.build()).toBe(sql); + }, + ); + + const englishCases = [ + { + lucene: 'Duration:[100 TO 500]', + english: 'Duration is between 100 and 500', + }, + { + lucene: 'Duration:{100 TO 500}', + english: 'Duration is between 100 (exclusive) and 500 (exclusive)', + }, + { + lucene: 'Duration:[100 TO 500}', + english: 'Duration is between 100 and 500 (exclusive)', + }, + { + lucene: 'Duration:{100 TO 500]', + english: 'Duration is between 100 (exclusive) and 500', + }, + { + lucene: '-Duration:{100 TO 500}', + english: 'Duration is not between 100 (exclusive) and 500 (exclusive)', + }, + ]; + + it.each(englishCases)( + 'converts "$lucene" to english "$english"', + async ({ lucene, english }) => { + const actualEnglish = await genEnglishExplanation({ + query: lucene, + tableConnection: { + tableName, + databaseName, + connectionId, + }, + metadata, + }); + expect(actualEnglish).toBe(english); + }, + ); +}); + describe('CustomSchemaSQLSerializerV2 - bloom_filter tokens() indices', () => { const metadata = getMetadata( new ClickhouseClient({ host: 'http://localhost:8123' }), diff --git a/packages/common-utils/src/queryParser.ts b/packages/common-utils/src/queryParser.ts index 0a90c34f68..3ecff394b7 100644 --- a/packages/common-utils/src/queryParser.ts +++ b/packages/common-utils/src/queryParser.ts @@ -68,6 +68,7 @@ function normalizeChExpression(expr: string): string { } const IMPLICIT_FIELD = ''; +const RANGE_UNBOUNDED = '*'; // Type guards for lucene AST types function isNodeTerm(node: lucene.Node | lucene.AST): node is lucene.NodeTerm { @@ -197,6 +198,7 @@ interface Serializer { end: string, isNegatedField: boolean, context: SerializerContext, + inclusive?: lucene.NodeRangedTerm['inclusive'], ): Promise; } @@ -396,10 +398,20 @@ class EnglishSerializer implements Serializer { start: string, end: string, isNegatedField: boolean, + context: SerializerContext, + inclusive: lucene.NodeRangedTerm['inclusive'] = 'both', ) { + const startBound = + inclusive === 'both' || inclusive === 'left' + ? start + : `${start} (exclusive)`; + const endBound = + inclusive === 'both' || inclusive === 'right' + ? end + : `${end} (exclusive)`; return `${field} ${ isNegatedField ? 'is not' : 'is' - } between ${start} and ${end}`; + } between ${startBound} and ${endBound}`; } } @@ -685,7 +697,7 @@ export abstract class SQLSerializer implements Serializer { // TODO: Not sure if SQL really needs this or if it'll coerce itself private attemptToParseNumber(term: string): string | number { - const number = Number.parseFloat(term); + const number = Number(term); if (Number.isNaN(number)) { return term; } @@ -717,6 +729,7 @@ export abstract class SQLSerializer implements Serializer { end: string, isNegatedField: boolean, context: SerializerContext, + inclusive: lucene.NodeRangedTerm['inclusive'] = 'both', ) { const { column, found, mapKeyIndexExpression, isArray } = await this.getColumnForField(field, context); @@ -732,10 +745,41 @@ export abstract class SQLSerializer implements Serializer { mapKeyIndexExpression && !isNegatedField ? ` AND ${mapKeyIndexExpression}` : ''; - return SqlString.format( - `(${column} ${isNegatedField ? 'NOT ' : ''}BETWEEN ? AND ?${expressionPostfix})`, - [this.attemptToParseNumber(start), this.attemptToParseNumber(end)], - ); + + const isStartUnbounded = start === RANGE_UNBOUNDED; + const isEndUnbounded = end === RANGE_UNBOUNDED; + if (isStartUnbounded && isEndUnbounded) { + return this.isNotNull(field, isNegatedField, context); + } + if (!isStartUnbounded && !isEndUnbounded && inclusive === 'both') { + return SqlString.format( + `(${column} ${isNegatedField ? 'NOT ' : ''}BETWEEN ? AND ?${expressionPostfix})`, + [this.attemptToParseNumber(start), this.attemptToParseNumber(end)], + ); + } + + const bounds: string[] = []; + if (!isStartUnbounded) { + const operator = + inclusive === 'both' || inclusive === 'left' ? '>=' : '>'; + bounds.push( + SqlString.format(`${column} ${operator} ?`, [ + this.attemptToParseNumber(start), + ]), + ); + } + if (!isEndUnbounded) { + const operator = + inclusive === 'both' || inclusive === 'right' ? '<=' : '<'; + bounds.push( + SqlString.format(`${column} ${operator} ?`, [ + this.attemptToParseNumber(end), + ]), + ); + } + return isNegatedField + ? `(NOT (${bounds.join(' AND ')})${expressionPostfix})` + : `(${bounds.join(' AND ')}${expressionPostfix})`; } } @@ -1981,6 +2025,7 @@ async function nodeTerm( rangedTerm.term_max, isNegatedField, context, + rangedTerm.inclusive, ); }