From a320e2ebcd2e1e885789001e2cdb20c2a2d4bb80 Mon Sep 17 00:00:00 2001 From: Drew Davis Date: Mon, 24 Aug 2026 14:47:57 -0400 Subject: [PATCH 1/2] feat: Distribute exact-match lucene variable references --- .../lucene-variable-field-distribution.md | 6 + .../__tests__/variableCompletions.test.ts | 27 +- .../SQLEditor/variableCompletions.tsx | 35 ++- .../src/__tests__/queryParser.test.ts | 67 +++++ .../src/__tests__/variables.test.ts | 255 +++++++++++++++- packages/common-utils/src/macros.ts | 1 + packages/common-utils/src/queryParser.ts | 2 +- packages/common-utils/src/variables.ts | 275 +++++++++++++++--- 8 files changed, 602 insertions(+), 66 deletions(-) create mode 100644 .changeset/lucene-variable-field-distribution.md diff --git a/.changeset/lucene-variable-field-distribution.md b/.changeset/lucene-variable-field-distribution.md new file mode 100644 index 0000000000..568c498dc7 --- /dev/null +++ b/.changeset/lucene-variable-field-distribution.md @@ -0,0 +1,6 @@ +--- +'@hyperdx/app': patch +'@hyperdx/common-utils': patch +--- + +feat: Distribute exact-match lucene variable references diff --git a/packages/app/src/components/SQLEditor/__tests__/variableCompletions.test.ts b/packages/app/src/components/SQLEditor/__tests__/variableCompletions.test.ts index ba2e341a14..b4816e9303 100644 --- a/packages/app/src/components/SQLEditor/__tests__/variableCompletions.test.ts +++ b/packages/app/src/components/SQLEditor/__tests__/variableCompletions.test.ts @@ -95,13 +95,11 @@ describe('buildLuceneVariableSuggestions', () => { // No braced or explicit-format forms either — in a Lucene input the bare // reference already renders in the lucene format. expect(buildLuceneVariableSuggestions([SERVICE])).toEqual([ - { - value: '$service', - label: '$service', - description: - 'The selected values of service. Expands to: ("api" OR "web")', - }, + expect.objectContaining({ value: '$service', label: '$service' }), ]); + expect(buildLuceneVariableSuggestions([SERVICE])[0].description).toContain( + 'Expands to: ("api" OR "web")', + ); }); it('previews the empty selection as the term that drops out', () => { @@ -131,6 +129,14 @@ describe('expandLuceneVariablesForEnglishDisplay', () => { ); }); + it('expands a quoted reference to an exact match per value', () => { + // Quoting is how an author opts into exact matching, so the English + // summary has to show that shape rather than the substring one above. + expect(expand('ServiceName:"$service"', [SERVICE])).toBe( + '(ServiceName:"api" OR ServiceName:"web")', + ); + }); + it('leaves an unselected variable as written', () => { // `("")` reads as `'ServiceName' is ` once serialized to English, // which is worse than naming the placeholder that has no value yet. @@ -148,6 +154,15 @@ describe('expandLuceneVariablesForEnglishDisplay', () => { ).toBe('ServiceName:("api" OR "web") AND Env:$env'); }); + it('leaves a half-typed format as written', () => { + // `${service:l}` is a keystroke on the way to `${service:lucene}`, and + // expanding it throws. This runs on every keystroke, so it has to be + // survivable rather than take the input down with it. + expect(expand('ServiceName:${service:l}', [SERVICE])).toBe( + 'ServiceName:${service:l}', + ); + }); + it('leaves unknown references and the variable macros alone', () => { expect( expand('$nope AND $__filter(ServiceName, $service)', [SERVICE]), diff --git a/packages/app/src/components/SQLEditor/variableCompletions.tsx b/packages/app/src/components/SQLEditor/variableCompletions.tsx index b8661c7b76..6b4b297490 100644 --- a/packages/app/src/components/SQLEditor/variableCompletions.tsx +++ b/packages/app/src/components/SQLEditor/variableCompletions.tsx @@ -5,7 +5,7 @@ import { } from '@hyperdx/common-utils/dist/macros'; import { ChartVariable } from '@hyperdx/common-utils/dist/types'; import { - substituteVariables, + substituteWithContext, VARIABLE_FORMATS, VariableFormat, } from '@hyperdx/common-utils/dist/variables'; @@ -17,17 +17,22 @@ const VARIABLE_FORMAT_DESCRIPTIONS: Record = { sqlstring: "Quoted and comma-separated, escaped for SQL. e.g. 'a', 'b', 'c'", csv: 'Comma-separated and unquoted. Not SQL-escaped. e.g. a,b,c', regex: 'A regex alternation. Regex escaped. e.g. (a|b|c)', - lucene: 'An OR of quoted terms, for Lucene inputs. e.g. ("a" OR "b" OR "c")', + lucene: + 'An OR of quoted terms, for Lucene inputs. e.g. ("a" OR "b" OR "c"). Quote the reference (field:"$var") for exact-match behavior. Leave unquoted (field:$var) for substring matching.', }; -/** What `snippet` expands to against the variable's current selection. */ -function describeVariableExpansion( +/** What `snippet` expands to in SQL against the variable's current selection. */ +function describeSqlVariableExpansion( snippet: string, variable: ChartVariable, ): string | undefined { let expansion: string; try { - expansion = substituteVariables(snippet, [variable]); + expansion = substituteWithContext(snippet, { + variables: [variable], + defaultFormat: 'sqlstring', + inputLanguage: 'sql', + }); } catch { return undefined; } @@ -77,7 +82,7 @@ function referenceCompletions(variable: ChartVariable): SQLCompletion[] { /** A static description and an expansion preview given the current selection */ const help = (snippet: string, description: string) => { - const expansion = describeVariableExpansion(snippet, variable); + const expansion = describeSqlVariableExpansion(snippet, variable); return expansion ? completionInfo(description, expansion) : description; }; @@ -153,9 +158,10 @@ export type LuceneVariableSuggestion = { /** Expand references the way a Lucene expression is expanded at query time. */ const substituteLucene = (text: string, variables: ChartVariable[]) => - substituteVariables(text, variables, { + substituteWithContext(text, { + variables, defaultFormat: 'lucene', - disableMacros: true, + inputLanguage: 'lucene', }); /** @@ -174,7 +180,7 @@ export function buildLuceneVariableSuggestions( return { value: reference, label: reference, - description: `The selected values of ${variable.name}. Expands to: ${expansion}`, + description: `The selected values of ${variable.name}. Expands to: ${expansion} by default, or (Field:"value1" OR Field:"value1") when quoted like Field:"$${variable.name}".`, }; }); } @@ -188,6 +194,10 @@ export function buildLuceneVariableSuggestions( * `("")`, which the English serializer reads as `'field' is ` even * though that form filters nothing; leaving the reference as written is the * honest rendering of "no value chosen yet". + * + * Expansion can throw on a reference that is well-formed but not yet valid — + * `${name:l}` is a keystroke on the way to `${name:lucene}` — and this runs on + * every keystroke, so a failure falls back to the text as written. */ export function expandLuceneVariablesForEnglishDisplay( text: string, @@ -196,7 +206,12 @@ export function expandLuceneVariablesForEnglishDisplay( const selected = (variables ?? []).filter( variable => variable.values.length > 0, ); - return selected.length > 0 ? substituteLucene(text, selected) : text; + if (selected.length === 0) return text; + try { + return substituteLucene(text, selected); + } catch { + return text; + } } /** Context providing in-scope dashboard variables for descendant inputs. */ diff --git a/packages/common-utils/src/__tests__/queryParser.test.ts b/packages/common-utils/src/__tests__/queryParser.test.ts index f420a2cadc..c79b1ba431 100644 --- a/packages/common-utils/src/__tests__/queryParser.test.ts +++ b/packages/common-utils/src/__tests__/queryParser.test.ts @@ -177,6 +177,43 @@ describe('CustomSchemaSQLSerializerV2 - json', () => { sql: "(((ServiceName ILIKE '%foo bar baz%')))", english: '(ServiceName contains "foo bar baz")', }, + // The shapes the `lucene` variable format expands a field-scoped reference + // into. Distributing the field is what makes each value an exact match: + // the grouped `ServiceName:("a" OR "b")` above is a substring match. + { + lucene: '(ServiceName:"a" OR ServiceName:"b")', + sql: "(((ServiceName = 'a') OR (ServiceName = 'b')))", + english: "('ServiceName' is a OR 'ServiceName' is b)", + }, + { + lucene: '(ServiceName:"a")', + sql: "(((ServiceName = 'a')))", + english: "('ServiceName' is a)", + }, + { + // A Map key distributes to exact equality per value AND keeps the + // per-term index hint. Contrast the grouped form above, which is a + // substring match. + lucene: + '(LogAttributes.error.message:"a" OR LogAttributes.error.message:"b")', + sql: "(((`LogAttributes`['error.message'] = 'a' AND indexHint(mapContains(`LogAttributes`, 'error.message'))) OR (`LogAttributes`['error.message'] = 'b' AND indexHint(mapContains(`LogAttributes`, 'error.message')))))", + english: + "('LogAttributes.error.message' is a OR 'LogAttributes.error.message' is b)", + }, + { + // Same for a JSON path: exact equality per value, where the grouped form + // above compiles to ILIKE. + lucene: + '(ResourceAttributesJSON.error.message:"a" OR ResourceAttributesJSON.error.message:"b")', + sql: "(((toString(`ResourceAttributesJSON`.`error`.`message`) = 'a') OR (toString(`ResourceAttributesJSON`.`error`.`message`) = 'b')))", + english: + "('ResourceAttributesJSON.error.message' is a OR 'ResourceAttributesJSON.error.message' is b)", + }, + { + lucene: 'NOT (ServiceName:"a" OR ServiceName:"b")', + sql: "(NOT ((ServiceName = 'a') OR (ServiceName = 'b')))", + english: "NOT ('ServiceName' is a OR 'ServiceName' is b)", + }, { lucene: 'ServiceName:(abc def)', sql: "(((ServiceName ILIKE '%abc%') AND (ServiceName ILIKE '%def%')))", @@ -503,12 +540,42 @@ describe('CustomSchemaSQLSerializerV2 - json', () => { ['ServiceName:("")', '(((1=1)))'], ['("")', '(((1=1)))'], ['ServiceName:""', "((ServiceName = ''))"], + // A Map key drops out the same way a plain column does. + ['LogAttributes.error.message:("")', '(((1=1)))'], ])('renders the empty lucene term %s as %s', async (lucene, expected) => { expect(await new SearchQueryBuilder(lucene, serializer).build()).toBe( expected, ); }); + // Two empty-selection shapes that do NOT cleanly drop out. Pinned as-is so a + // fix shows up here as a deliberate change rather than a surprise; see the + // note on each. + it.each([ + [ + // A JSON path renders as a match-anything ILIKE rather than `1=1`. Whether + // this really matches every row depends on what `toString` yields for an + // absent path — if that is NULL, rows missing the attribute are filtered + // out while nothing is selected. + 'ResourceAttributesJSON.error.message:("")', + "(((toString(`ResourceAttributesJSON`.`error`.`message`) ILIKE '%%')))", + ], + [ + // `NOT (1=1)` matches NOTHING. A negated reference — `-ServiceName:$svc` + // or `-ServiceName:"$svc"` — therefore empties the tile until a value is + // selected, which is the opposite of the no-op the empty state intends. + '-ServiceName:("")', + '(NOT ((1=1)))', + ], + ])( + 'renders the empty lucene term %s as %s, which is not a no-op', + async (lucene, expected) => { + expect(await new SearchQueryBuilder(lucene, serializer).build()).toBe( + expected, + ); + }, + ); + it('correctly searches multi-column implicit field', async () => { const serializer = new CustomSchemaSQLSerializerV2({ metadata, diff --git a/packages/common-utils/src/__tests__/variables.test.ts b/packages/common-utils/src/__tests__/variables.test.ts index b5c57f81f2..2aff515c44 100644 --- a/packages/common-utils/src/__tests__/variables.test.ts +++ b/packages/common-utils/src/__tests__/variables.test.ts @@ -8,9 +8,10 @@ import { getVariableReferences, hasVariableMacro, substituteChartConfigVariables, - substituteVariables, substituteVariablesForLanguage, + substituteWithContext, validateVariableReferencesInTemplate, + type VariableContext, } from '@/variables'; const variable = ( @@ -19,6 +20,22 @@ const variable = ( expression?: string, ): ChartVariable => ({ name, values, expression }); +/** + * `substituteWithContext` with the SQL-ish defaults, so each case only spells + * out the part of the context it is exercising. + */ +const substituteVariables = ( + input: string, + variables: ChartVariable[], + overrides: Partial> = {}, +) => + substituteWithContext(input, { + variables, + defaultFormat: 'sqlstring', + inputLanguage: 'sql', + ...overrides, + }); + const SERVICE = variable('service', ['api', 'web'], 'ServiceName'); const EMPTY_SERVICE = variable('service', [], 'ServiceName'); @@ -159,6 +176,52 @@ describe('substituteVariables', () => { }); }); + describe('inputLanguage', () => { + // `defaultFormat` says how a reference's values are rendered; + // `inputLanguage` says what will parse the result. Only the latter turns on + // the Lucene handling, so the two stay independently settable. + it('renders lucene values without any lucene handling by default', () => { + // Values render as lucene terms, but the template is still treated as + // SQL: no quoted-reference rewrite, and the macros expand. + expect( + substituteVariables('ServiceName:"$service"', [SERVICE], { + defaultFormat: 'lucene', + }), + ).toBe('ServiceName:"("api" OR "web")"'); + expect( + substituteVariables('$__filter(ServiceName, $service)', [SERVICE], { + defaultFormat: 'lucene', + }), + ).toBe("(ServiceName IN ('api', 'web'))"); + }); + + it('turns on the lucene handling when the input is lucene', () => { + expect( + substituteVariables('ServiceName:"$service"', [SERVICE], { + defaultFormat: 'lucene', + inputLanguage: 'lucene', + }), + ).toBe('(ServiceName:"api" OR ServiceName:"web")'); + expect( + substituteVariables('$__filter(ServiceName, $service)', [SERVICE], { + defaultFormat: 'lucene', + inputLanguage: 'lucene', + }), + ).toBe('$__filter(ServiceName, $service)'); + }); + + it('leaves a reference asking for another format alone in lucene input', () => { + // The rewrite is per-reference: it only applies where the values would + // render as lucene terms in the first place. + expect( + substituteVariables('ServiceName:"${service:csv}"', [SERVICE], { + defaultFormat: 'lucene', + inputLanguage: 'lucene', + }), + ).toBe('ServiceName:"api,web"'); + }); + }); + describe('braced references', () => { it('substitutes ${name} with the default format', () => { expect(substituteVariables('${service}', [SERVICE])).toBe("'api', 'web'"); @@ -449,6 +512,194 @@ describe('substituteVariablesForLanguage', () => { ), ).toBe('ServiceName:("")'); }); + + describe('quoted references expand to exact matches', () => { + // Quoting a reference is how an author opts into matching each selected + // value exactly: the lucene→SQL layer compiles `field:"a"` as equality, + // where a group-internal `("a")` becomes a substring match. The distributed + // shapes below are pinned end-to-end in queryParser.test.ts. + const expand = (template: string, variables: ChartVariable[] = [SERVICE]) => + substituteVariablesForLanguage(template, variables, 'lucene'); + + it('is opt-in: quoting distributes the field, leaving it grouped does not', () => { + expect(expand('ServiceName:"$service"')).toBe( + '(ServiceName:"api" OR ServiceName:"web")', + ); + expect(expand('ServiceName:$service')).toBe( + 'ServiceName:("api" OR "web")', + ); + }); + + it('distributes over a single value', () => { + expect( + expand('ServiceName:"$service"', [variable('service', ['api'])]), + ).toBe('(ServiceName:"api")'); + }); + + it('distributes across the whitespace the grammar allows after the colon', () => { + // `ServiceName: "$service"` parses to the same AST as the unspaced form + // — same field, same term — so it has to expand the same way. + expect(expand('ServiceName: "$service"')).toBe( + '(ServiceName:"api" OR ServiceName:"web")', + ); + expect(expand('SeverityText:error AND ServiceName: "$service"')).toBe( + 'SeverityText:error AND (ServiceName:"api" OR ServiceName:"web")', + ); + }); + + it('distributes over a dotted field', () => { + expect(expand('LogAttributes.service:"$service"')).toBe( + '(LogAttributes.service:"api" OR LogAttributes.service:"web")', + ); + }); + + it('distributes over a Map key and a JSON path', () => { + // Both are just dotted fields to the grammar, so the field repeats + // verbatim per value. queryParser.test.ts pins what each compiles to — + // notably that a Map key keeps its `indexHint(mapContains(...))` on every + // distributed term. + expect(expand('LogAttributes.error.message:"$service"')).toBe( + '(LogAttributes.error.message:"api" OR LogAttributes.error.message:"web")', + ); + expect(expand('ResourceAttributesJSON.error.message:"$service"')).toBe( + '(ResourceAttributesJSON.error.message:"api" OR ResourceAttributesJSON.error.message:"web")', + ); + }); + + it('distributes over a Map key holding a value that needs escaping', () => { + expect( + expand('LogAttributes.k8s.pod.name:"$service"', [ + variable('service', ['pod-a"1', 'pod\\b']), + ]), + ).toBe( + '(LogAttributes.k8s.pod.name:"pod-a\\"1" OR LogAttributes.k8s.pod.name:"pod\\\\b")', + ); + }); + + it('keeps the no-op form for a Map key and a JSON path with no selection', () => { + expect( + expand('LogAttributes.error.message:"$service"', [EMPTY_SERVICE]), + ).toBe('LogAttributes.error.message:("")'); + expect( + expand('ResourceAttributesJSON.error.message:"$service"', [ + EMPTY_SERVICE, + ]), + ).toBe('ResourceAttributesJSON.error.message:("")'); + }); + + it('distributes over a field with an escaped colon', () => { + expect(expand('foo\\:bar:"$service"')).toBe( + '(foo\\:bar:"api" OR foo\\:bar:"web")', + ); + }); + + it('escapes each value through the distributed path', () => { + expect( + expand('ServiceName:"$service"', [variable('service', ['a"b'])]), + ).toBe('(ServiceName:"a\\"b")'); + }); + + it('distributes every quoted reference in a template', () => { + expect( + expand('ServiceName:"$service" AND Env:"$env"', [ + SERVICE, + variable('env', ['prod']), + ]), + ).toBe('(ServiceName:"api" OR ServiceName:"web") AND (Env:"prod")'); + }); + + it('keeps the surrounding parentheses of a wrapped reference', () => { + expect(expand('(ServiceName:"$service")')).toBe( + '((ServiceName:"api" OR ServiceName:"web"))', + ); + }); + + it('turns a `-` negated reference into NOT, which the grammar can parse', () => { + // The fork parses `-field:x` with the `-` inside the field name, and + // `-(...)` is not a shape the grammar accepts. + expect(expand('-ServiceName:"$service"')).toBe( + 'NOT (ServiceName:"api" OR ServiceName:"web")', + ); + expect(expand('SeverityText:error AND -ServiceName:"$service"')).toBe( + 'SeverityText:error AND NOT (ServiceName:"api" OR ServiceName:"web")', + ); + }); + + it('leaves a spelled-out NOT in the text', () => { + expect(expand('NOT ServiceName:"$service"')).toBe( + 'NOT (ServiceName:"api" OR ServiceName:"web")', + ); + }); + + it('keeps the grouped no-op form for an empty selection', () => { + // `field:("")` compiles to `1=1`; a distributed `field:""` would compare + // the column against the empty string instead. The quotes are consumed + // either way, so the quoted spelling is safe before anything is selected. + expect(expand('ServiceName:"$service"', [EMPTY_SERVICE])).toBe( + 'ServiceName:("")', + ); + expect(expand('-ServiceName:"$service"', [EMPTY_SERVICE])).toBe( + '-ServiceName:("")', + ); + expect(expand('ServiceName:$service', [EMPTY_SERVICE])).toBe( + 'ServiceName:("")', + ); + }); + + it('leaves a reference embedded in a longer phrase alone', () => { + // The phrase is more than the reference, so it is a phrase match rather + // than a selection. + expect(expand('ServiceName:"$service down"')).toBe( + 'ServiceName:"("api" OR "web") down"', + ); + }); + + it('leaves a bare reference grouped, since it has no field', () => { + expect(expand('$service')).toBe('("api" OR "web")'); + }); + + it('leaves an unfielded quoted reference alone', () => { + // Nothing to distribute the values over. + expect(expand('"$service"')).toBe('"("api" OR "web")"'); + }); + + it('leaves an already grouped reference alone', () => { + // The inner term parses with an implicit field, so there is no field on + // it to distribute. + expect(expand('ServiceName:("$service")')).toBe( + 'ServiceName:("("api" OR "web")")', + ); + }); + + it('leaves a reference that asked for another format alone', () => { + expect(expand('ServiceName:"${service:csv}"')).toBe( + 'ServiceName:"api,web"', + ); + }); + + it('leaves an unknown name verbatim', () => { + expect(expand('ServiceName:"$unknown"')).toBe('ServiceName:"$unknown"'); + }); + + it('falls back to the plain expansion when the template will not parse', () => { + // `http://` only parses after `encodeSpecialTokens`, which the renderer + // applies and this preprocessor does not. + expect(expand('Url:http://example.com AND ServiceName:"$service"')).toBe( + 'Url:http://example.com AND ServiceName:"("api" OR "web")"', + ); + }); + + it('never distributes over a field that is itself a reference', () => { + // `$field:"$service"` parses with the *placeholder* standing in for the + // field, so rewriting would splice that placeholder into the output. + expect( + expand('$field:"$service"', [ + variable('field', ['ServiceName']), + SERVICE, + ]), + ).toBe('("ServiceName"):"("api" OR "web")"'); + }); + }); }); describe('getVariableReferences', () => { @@ -1233,7 +1484,7 @@ describe('validateVariableReferencesInTemplate', () => { ).toEqual({ errors: [], warnings: [] }); }); - it('accepts a quoted reference: the lucene format quotes each value', () => { + it('accepts a quoted reference: quoting opts into exact matches', () => { expect( validate('ServiceName:"$service"', [SERVICE], { language: 'lucene' }), ).toEqual({ errors: [], warnings: [] }); diff --git a/packages/common-utils/src/macros.ts b/packages/common-utils/src/macros.ts index afb8a0dbc9..e48ae1c779 100644 --- a/packages/common-utils/src/macros.ts +++ b/packages/common-utils/src/macros.ts @@ -479,6 +479,7 @@ export function replaceMacros( const variableContext: VariableContext | undefined = variables && { variables, defaultFormat: 'sqlstring', + inputLanguage: 'sql', }; const macroNames = [ diff --git a/packages/common-utils/src/queryParser.ts b/packages/common-utils/src/queryParser.ts index d6c76ea314..1beb8e43f4 100644 --- a/packages/common-utils/src/queryParser.ts +++ b/packages/common-utils/src/queryParser.ts @@ -74,7 +74,7 @@ function normalizeChExpression(expr: string): string { return expr.replace(/\s+/g, '').replace(/`/g, ''); } -const IMPLICIT_FIELD = ''; +export const IMPLICIT_FIELD = ''; const RANGE_UNBOUNDED = '*'; // Type guards for lucene AST types diff --git a/packages/common-utils/src/variables.ts b/packages/common-utils/src/variables.ts index 555a934ff3..67a717239d 100644 --- a/packages/common-utils/src/variables.ts +++ b/packages/common-utils/src/variables.ts @@ -1,9 +1,12 @@ +import lucene from '@hyperdx/lucene'; + import { escapeSqlString, isQuoteEscapedByBackslash, splitAndTrimWithBracket, } from './core/utils'; import { MacroExpansionError, MalformedMacroArgsError } from './macroErrors'; +import { IMPLICIT_FIELD } from './queryParser'; import { ChartConfigWithOptDateRange, ChartVariable, @@ -392,13 +395,8 @@ export type VariableContext = { variables: ChartVariable[]; /** Format used by references that don't request one. */ defaultFormat: VariableFormat; - /** - * When true, `$__filter` and `$__conditionalAll` are left exactly as - * written. They expand to SQL predicates, so they have no meaning in a - * Lucene expression — expanding one there would splice SQL into a query - * that is about to be parsed as Lucene. - */ - disableMacros?: boolean; + /** The language whatever consumes the result will parse it as. */ + inputLanguage: SearchConditionLanguage; }; const sqlNoOp = (name: string) => @@ -561,35 +559,213 @@ export function expandVariableToken( ); } +/** Expand the given token without applying lucene-specific rewrites. */ +function expandTokenWithoutLuceneRewrites( + token: TemplateToken, + ctx: VariableContext, +): string { + if (token.kind === 'text') return token.text; + if (token.kind === 'macro') return token.raw; + return expandVariableToken(token, ctx); +} + +/** + * Renders Lucene syntax for a field being exact-matched against any of the given values. + * eg. `field:"value1" OR field:"value2" OR field:"value3"` + **/ +const formatDistributedLuceneValues = (field: string, values: string[]) => + `(${values + .map(value => `${field}:"${escapeLuceneValue(value)}"`) + .join(' OR ')})`; + +/** A non-text token's placeholder term and its span in the sentinel string. */ +type Sentinel = { + token: Exclude; + sentinel: string; + offset: number; +}; + +/** What a rewritten reference emits, and the span of text it replaces. */ +type LuceneRewrite = { start: number; end: number; text: string }; + +/** + * The values a variable reference renders when it renders in the lucene format. + * `undefined` if the token is not a lucene formatted variable reference or if + * the referenced variable is not found. + */ +function getLuceneFormattedValues( + token: TemplateToken, + ctx: VariableContext, +): string[] | undefined { + if (token.kind === 'text' || token.kind === 'macro') return undefined; + const requestedFormat = token.kind === 'braced' ? token.format : undefined; + if ((requestedFormat ?? ctx.defaultFormat) !== 'lucene') return undefined; + return ctx.variables.find(variable => variable.name === token.name)?.values; +} + +/** + * Index every `NodeTerm` in an AST by the offset of its term text, + * writing them to `termsByOffset`. + * */ +function indexLuceneTermsByOffset( + node: lucene.AST | lucene.Node | null | undefined, + termsByOffset: Map, +): void { + if (node == null) return; + + if ('termLocation' in node) { + termsByOffset.set( + // A quoted node's `termLocation` starts at the opening quote, + // so +1 to get offset of the actual term text. + node.termLocation.start.offset + (node.quoted ? 1 : 0), + node, + ); + return; + } + + if ('left' in node) { + indexLuceneTermsByOffset(node.left, termsByOffset); + if ('right' in node) indexLuceneTermsByOffset(node.right, termsByOffset); + } +} + /** - * Expand references, leaving each variable macro exactly as written — argument - * list and all, so the `$name` argument that names the variable survives too. + * Returns the lucene exact-match behavior for a quoted field reference, if the + * given sentinel is a quoted field reference in the given sentinel string. + * Otherwise returns undefined. * - * The macros are still *scanned* rather than left to the text branch, which is - * what makes "exactly as written" true: their arguments are never visited, so - * nothing inside one is substituted. A macro missing its closing paren is - * tolerated (`skip`), since this runs over expressions as they are typed. + * @param runStart is the offset the last rewrite ended at. */ -function substituteReferencesOnly(input: string, ctx: VariableContext): string { - return scanTemplateTokens(input, VARIABLE_MACRO_NAMES, { - onMalformed: 'skip', - }) - .map(token => { - if (token.kind === 'text') return token.text; - if (token.kind === 'macro') return token.raw; - return expandVariableToken(token, ctx); - }) - .join(''); +function rewriteQuotedVariableReference( + sentinel: Sentinel, + sentinelString: string, + termsByOffset: Map, + ctx: VariableContext, + runStart: number, +): LuceneRewrite | undefined { + // If the variable does not exist, then no rewrite is possible + const values = getLuceneFormattedValues(sentinel.token, ctx); + if (values == null) return undefined; + + // Find the Lucene AST node that corresponds to this token's sentinel + const node = termsByOffset.get(sentinel.offset); + + // If the sentinel is not being used in the form Field:"$var", apply no rewrite. + if ( + node == null || + node.term !== sentinel.sentinel || + !node.quoted || + node.field === IMPLICIT_FIELD || + node.fieldLocation == null + ) { + return undefined; + } + + // The offset in the sentinel string where the field name starts + const fieldStart = node.fieldLocation.start.offset; + + // If the field starts in a section that has been rewritten already, + // skip the rewrite to avoid overlapping rewrites. + if (fieldStart < runStart) return undefined; + + // Find the closing quote from the sentinel rather than from `termLocation.end`, + // which swallows whatever whitespace follows the term. + const sentinelEnd = sentinel.offset + sentinel.sentinel.length; + if (sentinelString.charAt(sentinelEnd) !== '"') return undefined; + + // `-field:x` is parsed with the `-` riding on `node.field`, and `-(…)` + // is not a shape the grammar accepts, so a negated reference has to come out + // as `NOT (…)` instead. + const negated = node.field.startsWith('-'); + const field = negated ? node.field.slice(1) : node.field; + if (field === '') return undefined; + + const span = { start: fieldStart, end: sentinelEnd + 1 }; + + // An empty selection stays the grouped no-op: `field:("")` compiles to `1=1`, + // where a distributed `field:""` would compare against the empty string + // instead. `node.field` already carries any `-`, and `-field:(…)` parses. + if (values.length === 0) return { ...span, text: `${node.field}:("")` }; + + return { + ...span, + text: `${negated ? 'NOT ' : ''}${formatDistributedLuceneValues(field, values)}`, + }; } -function substituteWithContext(input: string, ctx: VariableContext): string { - if (ctx.disableMacros) return substituteReferencesOnly(input, ctx); +/** + * Substitute rendered values for variable tokens, with lucene-aware rewrites + * that provide exact-match semantics for quoted Field:"$var" references. + */ +function substituteTokensWithLuceneRewrites( + tokens: TemplateToken[], + ctx: VariableContext, +): string { + // If no reference is rendered in the lucene format, the rewrite path is unnecessary. + const anyRewritable = tokens.some( + token => getLuceneFormattedValues(token, ctx) != null, + ); + if (!anyRewritable) { + return tokens + .map(token => expandTokenWithoutLuceneRewrites(token, ctx)) + .join(''); + } - return expandTemplate(input, { - macroNames: VARIABLE_MACRO_NAMES, - expandMacro: token => expandVariableToken(token, ctx), - expandReference: token => expandVariableToken(token, ctx), - }); + // Build the sentinel string, a string with all variable references replaced by + // unique placeholders, and record the locations of each of those placeholders. + // eg. Transform `field:"$service" AND $other` into `field:"__hdx_sentinel_0" AND __hdx_sentinel_1`. + let sentinelString = ''; + const sentinelLocations: Sentinel[] = []; + for (const token of tokens) { + if (token.kind === 'text') { + sentinelString += token.text; + continue; + } + const sentinel = `__hdx_sentinel_${sentinelLocations.length}`; + sentinelLocations.push({ token, sentinel, offset: sentinelString.length }); + sentinelString += sentinel; + } + + // Parse the sentinel string into a lucene AST + let ast: lucene.AST; + try { + ast = lucene.parse(sentinelString); + } catch { + // If the lucene is not valid, fall back to the non-rewrite path + return tokens + .map(token => expandTokenWithoutLuceneRewrites(token, ctx)) + .join(''); + } + + // Get the offset of every term's text in the sentinel string. + const termsByOffset = new Map(); + indexLuceneTermsByOffset(ast, termsByOffset); + + // Build the final output by replacing each sentinel with its rewritten expansion. + let rewrittenOutput = ''; + let runStart = 0; + for (const region of sentinelLocations) { + const exactMatchRewrite = rewriteQuotedVariableReference( + region, + sentinelString, + termsByOffset, + ctx, + runStart, + ); + if (exactMatchRewrite) { + const untouched = sentinelString.slice(runStart, exactMatchRewrite.start); + const rewrite = exactMatchRewrite.text; + rewrittenOutput += untouched + rewrite; + runStart = exactMatchRewrite.end; + } else { + const untouched = sentinelString.slice(runStart, region.offset); + const expanded = expandTokenWithoutLuceneRewrites(region.token, ctx); + rewrittenOutput += untouched + expanded; + runStart = region.offset + region.sentinel.length; + } + } + const remaining = sentinelString.slice(runStart); + return rewrittenOutput + remaining; } /** @@ -600,34 +776,39 @@ function substituteWithContext(input: string, ctx: VariableContext): string { * both sets in one pass. This entry point is for the surfaces that only carry * variables (chart-builder where/having, PromQL expressions). */ -export function substituteVariables( +export function substituteWithContext( input: string, - variables: ChartVariable[], - { - defaultFormat = 'sqlstring', - disableMacros, - }: { defaultFormat?: VariableFormat; disableMacros?: boolean } = {}, + ctx: VariableContext, ): string { - return substituteWithContext(input, { - variables, - defaultFormat, - disableMacros, + if (ctx.inputLanguage === 'lucene') { + return substituteTokensWithLuceneRewrites( + // Variable macros are not supported in lucene, but we still scan for them so that + // downstream expansion doesn't attempt to expand variables referenced in their args. + scanTemplateTokens(input, VARIABLE_MACRO_NAMES, { onMalformed: 'skip' }), + ctx, + ); + } + + return expandTemplate(input, { + macroNames: VARIABLE_MACRO_NAMES, + expandMacro: token => expandVariableToken(token, ctx), + expandReference: token => expandVariableToken(token, ctx), }); } /** - * Expand a template for the language its renderer will parse it as. - * A Lucene expression renders values in the `lucene` format and gets no macros. + * Expand a template for the language its renderer will parse it as, rendering + * values in the format that language reads. */ export function substituteVariablesForLanguage( input: string, variables: ChartVariable[], - language: SearchConditionLanguage, + inputLanguage: SearchConditionLanguage, ): string { - const isLucene = language === 'lucene'; - return substituteVariables(input, variables, { - defaultFormat: isLucene ? 'lucene' : 'sqlstring', - disableMacros: isLucene, + return substituteWithContext(input, { + variables, + defaultFormat: inputLanguage === 'lucene' ? 'lucene' : 'sqlstring', + inputLanguage, }); } From 39963bfd26e72ba3f566a1e5a7209486047fe788 Mon Sep 17 00:00:00 2001 From: Drew Davis Date: Mon, 24 Aug 2026 15:39:10 -0400 Subject: [PATCH 2/2] fix: Encode special values before lucene variable rewrite --- .../src/__tests__/queryParser.test.ts | 20 +++++ .../src/__tests__/variables.test.ts | 79 ++++++++++++++++++- packages/common-utils/src/queryParser.ts | 79 +++++++++++++++---- packages/common-utils/src/variables.ts | 33 ++++++-- 4 files changed, 185 insertions(+), 26 deletions(-) diff --git a/packages/common-utils/src/__tests__/queryParser.test.ts b/packages/common-utils/src/__tests__/queryParser.test.ts index c79b1ba431..c3f897b3db 100644 --- a/packages/common-utils/src/__tests__/queryParser.test.ts +++ b/packages/common-utils/src/__tests__/queryParser.test.ts @@ -3,6 +3,8 @@ import { ClickhouseClient } from '@/clickhouse/node'; import { getMetadata } from '@/core/metadata'; import { CustomSchemaSQLSerializerV2, + decodeSpecialTokensToSource, + encodeSpecialTokens, genEnglishExplanation, parseKvItemsCastExpression, parseKvItemsExpression, @@ -20,6 +22,24 @@ afterAll(() => { jest.restoreAllMocks(); }); +describe('special token encoding', () => { + it('decodeSpecialTokensToSource is a lossless inverse of encodeSpecialTokens', () => { + const queries = [ + 'Url:http://example.com', + 'Url:https://example.com/path', + 'Host:localhost:3000', + 'Body:path\\\\to\\\\file', + 'foo\\:bar:baz', + 'Url:http://localhost:8080 AND Body:a\\\\b AND foo\\:bar:x', + ]; + for (const query of queries) { + expect(decodeSpecialTokensToSource(encodeSpecialTokens(query))).toBe( + query, + ); + } + }); +}); + describe('CustomSchemaSQLSerializerV2 - json', () => { const metadata = getMetadata( new ClickhouseClient({ host: 'http://localhost:8123' }), diff --git a/packages/common-utils/src/__tests__/variables.test.ts b/packages/common-utils/src/__tests__/variables.test.ts index 2aff515c44..499e0e4c1d 100644 --- a/packages/common-utils/src/__tests__/variables.test.ts +++ b/packages/common-utils/src/__tests__/variables.test.ts @@ -682,10 +682,81 @@ describe('substituteVariablesForLanguage', () => { }); it('falls back to the plain expansion when the template will not parse', () => { - // `http://` only parses after `encodeSpecialTokens`, which the renderer - // applies and this preprocessor does not. - expect(expand('Url:http://example.com AND ServiceName:"$service"')).toBe( - 'Url:http://example.com AND ServiceName:"("api" OR "web")"', + expect(expand('(ServiceName:"$service"')).toBe( + '(ServiceName:"("api" OR "web")"', + ); + }); + + // `http://` and friends only parse after `encodeSpecialTokens`, which + // this preprocessor applies the same way the renderer does. The special + // text must round-trip verbatim wherever it sits relative to the + // reference. + it.each([ + [ + 'Url:http://example.com AND ServiceName:"$service"', + 'Url:http://example.com AND (ServiceName:"api" OR ServiceName:"web")', + ], + [ + 'ServiceName:"$service" AND Url:http://example.com', + '(ServiceName:"api" OR ServiceName:"web") AND Url:http://example.com', + ], + [ + 'http://example.com AND ServiceName:"$service"', + 'http://example.com AND (ServiceName:"api" OR ServiceName:"web")', + ], + [ + 'Url:https://a.example AND ServiceName:"$service" AND Referrer:https://b.example', + 'Url:https://a.example AND (ServiceName:"api" OR ServiceName:"web") AND Referrer:https://b.example', + ], + [ + 'Host:localhost:3000 AND ServiceName:"$service"', + 'Host:localhost:3000 AND (ServiceName:"api" OR ServiceName:"web")', + ], + [ + 'Url:http://localhost:8080 AND ServiceName:"$service"', + 'Url:http://localhost:8080 AND (ServiceName:"api" OR ServiceName:"web")', + ], + [ + 'Body:path\\\\to AND ServiceName:"$service"', + 'Body:path\\\\to AND (ServiceName:"api" OR ServiceName:"web")', + ], + [ + 'foo\\:bar:baz AND ServiceName:"$service"', + 'foo\\:bar:baz AND (ServiceName:"api" OR ServiceName:"web")', + ], + [ + 'Url:http://example.com AND foo\\:bar:"$service"', + 'Url:http://example.com AND (foo\\:bar:"api" OR foo\\:bar:"web")', + ], + [ + '-foo\\:bar:"$service" AND Url:https://example.com', + 'NOT (foo\\:bar:"api" OR foo\\:bar:"web") AND Url:https://example.com', + ], + [ + 'ServiceName:"$service" AND Url:http://example.com AND Env:"$env"', + '(ServiceName:"api" OR ServiceName:"web") AND Url:http://example.com AND (Env:"prod")', + ], + [ + 'Message:"see https://example.com/docs" AND ServiceName:"$service"', + 'Message:"see https://example.com/docs" AND (ServiceName:"api" OR ServiceName:"web")', + ], + [ + '(Url:http://a.example OR Url:https://b.example) AND ServiceName:"$service"', + '(Url:http://a.example OR Url:https://b.example) AND (ServiceName:"api" OR ServiceName:"web")', + ], + [ + 'ServiceName:"$service" AND Body:a\\\\b AND foo\\:bar:x AND Host:localhost:9000', + '(ServiceName:"api" OR ServiceName:"web") AND Body:a\\\\b AND foo\\:bar:x AND Host:localhost:9000', + ], + ])('distributes within `%s`', (template, expected) => { + expect(expand(template, [SERVICE, variable('env', ['prod'])])).toBe( + expected, + ); + }); + + it('keeps the no-op form for an escaped-colon field with no selection', () => { + expect(expand('foo\\:bar:"$service"', [EMPTY_SERVICE])).toBe( + 'foo\\:bar:("")', ); }); diff --git a/packages/common-utils/src/queryParser.ts b/packages/common-utils/src/queryParser.ts index 1beb8e43f4..61326a599a 100644 --- a/packages/common-utils/src/queryParser.ts +++ b/packages/common-utils/src/queryParser.ts @@ -29,22 +29,73 @@ import { UseTextIndex } from '@/types'; /** Max number of tokens to pass to hasAllTokens(), which supports up to 64 tokens as of ClickHouse v25.12. */ const HAS_ALL_TOKENS_CHUNK_SIZE = 50; -function encodeSpecialTokens(query: string): string { - return query - .replace(/\\\\/g, 'HDX_BACKSLASH_LITERAL') - .replace(/http:\/\//g, 'http_COLON_//') - .replace(/https:\/\//g, 'https_COLON_//') - .replace(/localhost:(\d{1,5})/g, 'localhost_COLON_$1') - .replace(/\\:/g, 'HDX_COLON'); +/** + * Sequences the lucene grammar can't parse, placeholder-encoded before + * parsing. `source` restores the original query spelling; `value` is the + * raw value the sequence represents (lucene escaping dropped). + */ +const SPECIAL_TOKEN_ENCODINGS = [ + { + encodePattern: /\\\\/g, + placeholder: 'HDX_BACKSLASH_LITERAL', + decodePattern: /HDX_BACKSLASH_LITERAL/g, + source: '\\\\', + value: '\\', + }, + { + encodePattern: /http:\/\//g, + placeholder: 'http_COLON_//', + decodePattern: /http_COLON_\/\//g, + source: 'http://', + value: 'http://', + }, + { + encodePattern: /https:\/\//g, + placeholder: 'https_COLON_//', + decodePattern: /https_COLON_\/\//g, + source: 'https://', + value: 'https://', + }, + { + encodePattern: /localhost:(\d{1,5})/g, + placeholder: 'localhost_COLON_$1', + decodePattern: /localhost_COLON_(\d{1,5})/g, + source: 'localhost:$1', + value: 'localhost:$1', + }, + { + encodePattern: /\\:/g, + placeholder: 'HDX_COLON', + decodePattern: /HDX_COLON/g, + source: '\\:', + value: ':', + }, +] as const; + +export function encodeSpecialTokens(query: string): string { + return SPECIAL_TOKEN_ENCODINGS.reduce( + (encoded, { encodePattern, placeholder }) => + encoded.replace(encodePattern, placeholder), + query, + ); } + +/** Decode placeholders (and `\"`) to the raw values the sequences represent. */ function decodeSpecialTokens(query: string): string { - return query - .replace(/\\"/g, '"') - .replace(/HDX_BACKSLASH_LITERAL/g, '\\') - .replace(/http_COLON_\/\//g, 'http://') - .replace(/https_COLON_\/\//g, 'https://') - .replace(/localhost_COLON_(\d{1,5})/g, 'localhost:$1') - .replace(/HDX_COLON/g, ':'); + return SPECIAL_TOKEN_ENCODINGS.reduce( + (decoded, { decodePattern, value }) => + decoded.replace(decodePattern, value), + query.replace(/\\"/g, '"'), + ); +} + +/** Lossless inverse of `encodeSpecialTokens`: placeholders back to their original query spelling. */ +export function decodeSpecialTokensToSource(query: string): string { + return SPECIAL_TOKEN_ENCODINGS.reduce( + (decoded, { decodePattern, source }) => + decoded.replace(decodePattern, source), + query, + ); } export function parse(query: string): lucene.AST { diff --git a/packages/common-utils/src/variables.ts b/packages/common-utils/src/variables.ts index 67a717239d..6ba73c61fe 100644 --- a/packages/common-utils/src/variables.ts +++ b/packages/common-utils/src/variables.ts @@ -6,7 +6,11 @@ import { splitAndTrimWithBracket, } from './core/utils'; import { MacroExpansionError, MalformedMacroArgsError } from './macroErrors'; -import { IMPLICIT_FIELD } from './queryParser'; +import { + decodeSpecialTokensToSource, + encodeSpecialTokens, + IMPLICIT_FIELD, +} from './queryParser'; import { ChartConfigWithOptDateRange, ChartVariable, @@ -677,15 +681,21 @@ function rewriteQuotedVariableReference( // is not a shape the grammar accepts, so a negated reference has to come out // as `NOT (…)` instead. const negated = node.field.startsWith('-'); - const field = negated ? node.field.slice(1) : node.field; + // The field is parsed from the encoded sentinel string, so restore the + // original spelling of any special sequences (e.g. an escaped colon). + const field = decodeSpecialTokensToSource( + negated ? node.field.slice(1) : node.field, + ); if (field === '') return undefined; const span = { start: fieldStart, end: sentinelEnd + 1 }; // An empty selection stays the grouped no-op: `field:("")` compiles to `1=1`, // where a distributed `field:""` would compare against the empty string - // instead. `node.field` already carries any `-`, and `-field:(…)` parses. - if (values.length === 0) return { ...span, text: `${node.field}:("")` }; + // instead. `-field:(…)` parses, so the `-` can stay on the field. + if (values.length === 0) { + return { ...span, text: `${negated ? '-' : ''}${field}:("")` }; + } return { ...span, @@ -714,11 +724,14 @@ function substituteTokensWithLuceneRewrites( // Build the sentinel string, a string with all variable references replaced by // unique placeholders, and record the locations of each of those placeholders. // eg. Transform `field:"$service" AND $other` into `field:"__hdx_sentinel_0" AND __hdx_sentinel_1`. + // Text is encoded the same way the renderer encodes before parsing, so the + // grammar accepts sequences like `http://`; all recorded offsets are in + // encoded space, and untouched output is decoded back to its source spelling. let sentinelString = ''; const sentinelLocations: Sentinel[] = []; for (const token of tokens) { if (token.kind === 'text') { - sentinelString += token.text; + sentinelString += encodeSpecialTokens(token.text); continue; } const sentinel = `__hdx_sentinel_${sentinelLocations.length}`; @@ -753,18 +766,22 @@ function substituteTokensWithLuceneRewrites( runStart, ); if (exactMatchRewrite) { - const untouched = sentinelString.slice(runStart, exactMatchRewrite.start); + const untouched = decodeSpecialTokensToSource( + sentinelString.slice(runStart, exactMatchRewrite.start), + ); const rewrite = exactMatchRewrite.text; rewrittenOutput += untouched + rewrite; runStart = exactMatchRewrite.end; } else { - const untouched = sentinelString.slice(runStart, region.offset); + const untouched = decodeSpecialTokensToSource( + sentinelString.slice(runStart, region.offset), + ); const expanded = expandTokenWithoutLuceneRewrites(region.token, ctx); rewrittenOutput += untouched + expanded; runStart = region.offset + region.sentinel.length; } } - const remaining = sentinelString.slice(runStart); + const remaining = decodeSpecialTokensToSource(sentinelString.slice(runStart)); return rewrittenOutput + remaining; }