diff --git a/.changeset/json-path-range-search.md b/.changeset/json-path-range-search.md new file mode 100644 index 0000000000..f95316cfe8 --- /dev/null +++ b/.changeset/json-path-range-search.md @@ -0,0 +1,11 @@ +--- +'@hyperdx/common-utils': patch +'@hyperdx/api': patch +'@hyperdx/app': patch +--- + +fix: support Lucene range searches on JSON column paths + +A range such as `ResourceAttributesJSON.http.status:[400 TO 499]` rendered +`(( BETWEEN 400 AND 499))`, a ClickHouse syntax error. Ranges on a JSON path now +compare the path's numeric value, as `>`, `>=`, `<` and `<=` already do. diff --git a/packages/common-utils/src/__tests__/queryParser.test.ts b/packages/common-utils/src/__tests__/queryParser.test.ts index c3f897b3db..8cb109c546 100644 --- a/packages/common-utils/src/__tests__/queryParser.test.ts +++ b/packages/common-utils/src/__tests__/queryParser.test.ts @@ -169,6 +169,37 @@ describe('CustomSchemaSQLSerializerV2 - json', () => { ); }); + it('range', async () => { + const field = 'ResourceAttributesJSON.range.test'; + const numeric = `dynamicType(\`ResourceAttributesJSON\`.\`range\`.\`test\`) in ('Int8', 'Int16', 'Int32', 'Int64', 'Int128', 'Int256', 'UInt8', 'UInt16', 'UInt32', 'UInt64', 'UInt128', 'UInt256', 'Float32', 'Float64') and \`ResourceAttributesJSON\`.\`range\`.\`test\``; + expect(await serializer.range(field, '1', '5', false, {})).toBe( + `(${numeric} BETWEEN 1 AND 5)`, + ); + expect(await serializer.range(field, '1', '5', false, {}, 'none')).toBe( + `(${numeric} > 1 AND ${numeric} < 5)`, + ); + }); + + it('range from a Lucene JSON path, including negated and open bounds', async () => { + const numeric = `dynamicType(\`ResourceAttributesJSON\`.\`http\`.\`status\`) in ('Int8', 'Int16', 'Int32', 'Int64', 'Int128', 'Int256', 'UInt8', 'UInt16', 'UInt32', 'UInt64', 'UInt128', 'UInt256', 'Float32', 'Float64') and \`ResourceAttributesJSON\`.\`http\`.\`status\``; + const build = (lucene: string) => + new SearchQueryBuilder(lucene, serializer).build(); + expect(await build('ResourceAttributesJSON.http.status:[1 TO 5]')).toBe( + `((${numeric} BETWEEN 1 AND 5))`, + ); + // Negating must also keep rows whose value is not numeric, like the + // half-open form does. + expect(await build('-ResourceAttributesJSON.http.status:[1 TO 5]')).toBe( + `((NOT (${numeric} BETWEEN 1 AND 5)))`, + ); + expect(await build('-ResourceAttributesJSON.http.status:[* TO 5]')).toBe( + `((NOT (${numeric} <= 5)))`, + ); + expect(await build('ResourceAttributesJSON.http.status:[1 TO *]')).toBe( + `((${numeric} >= 1))`, + ); + }); + const testCases = [ { lucene: '"foo bar baz"', diff --git a/packages/common-utils/src/queryParser.ts b/packages/common-utils/src/queryParser.ts index 61326a599a..bc43c7ce04 100644 --- a/packages/common-utils/src/queryParser.ts +++ b/packages/common-utils/src/queryParser.ts @@ -791,8 +791,14 @@ export abstract class SQLSerializer implements Serializer { context: SerializerContext, inclusive: lucene.NodeRangedTerm['inclusive'] = 'both', ) { - const { column, found, mapKeyIndexExpression, isArray } = - await this.getColumnForField(field, context); + const { + column: rawColumn, + columnJSON, + found, + propertyType, + mapKeyIndexExpression, + isArray, + } = await this.getColumnForField(field, context); if (!found) { return this.NOT_FOUND_QUERY; } @@ -801,6 +807,10 @@ export abstract class SQLSerializer implements Serializer { 'range comparison is not supported for Array-type fields', ); } + // A JSON path has no plain column expression; compare its numeric form, + // as gt/gte/lt/lte do. + const column = + propertyType === JSDataType.JSON ? columnJSON?.number : rawColumn; const expressionPostfix = mapKeyIndexExpression && !isNegatedField ? ` AND ${mapKeyIndexExpression}` @@ -812,9 +822,22 @@ export abstract class SQLSerializer implements Serializer { return this.isNotNull(field, isNegatedField, context); } if (!isStartUnbounded && !isEndUnbounded && inclusive === 'both') { + const bounds = [ + this.attemptToParseNumber(start), + this.attemptToParseNumber(end), + ]; + // The JSON numeric form is `dynamicType(..) in (..) and path`, so a bare + // NOT BETWEEN would bind to the path only and drop non-numeric values. + // Negate the whole expression, as the half-open branch below does. + if (isNegatedField && propertyType === JSDataType.JSON) { + return SqlString.format( + `(NOT (${column} BETWEEN ? AND ?)${expressionPostfix})`, + bounds, + ); + } return SqlString.format( `(${column} ${isNegatedField ? 'NOT ' : ''}BETWEEN ? AND ?${expressionPostfix})`, - [this.attemptToParseNumber(start), this.attemptToParseNumber(end)], + bounds, ); } @@ -1207,12 +1230,7 @@ const KV_ITEMS_STRATEGIES = [ export function skipIndexMatches( idx: SkipIndexMetadata, expectedType: - | 'text' - | 'bloom_filter' - | 'minmax' - | 'tokenbf_v1' - | 'set' - | 'ngrambf_v1', + 'text' | 'bloom_filter' | 'minmax' | 'tokenbf_v1' | 'set' | 'ngrambf_v1', options?: { tokenizer?: string; },