fix(common-utils): honor open, exclusive and non-numeric range bounds

.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.

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' }),

packages/common-utils/src/queryParser.ts

@@ -68,6 +68,7 @@ function normalizeChExpression(expr: string): string {
 }
 
 const IMPLICIT_FIELD = '<implicit>';
+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<string>;
 }
 
@@ -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,
     );
   }