Skip to content

Lucene search on a numeric- or Bool-valued Map column double-escapes the map subscript #2765

Description

@shuvamk

What happens

Searching a Map column whose value type is numeric or Bool generates SQL in which the map subscript is wrapped in an extra layer of backticks, with the inner ones doubled. ClickHouse then reads it as a single identifier literally named `Measures`['latency_ms'] rather than as a map subscript, so the predicate cannot resolve.

The tell is that the same search behaves differently depending only on quoting:

Search Generated predicate
Measures.latency_ms:250 Measures``['latency_ms']` = CAST('250', 'Float64') ``` ❌
Measures.latency_ms:"250" `Measures`['latency_ms'] = CAST('250', 'Float64')
Measures.latency_ms:>250 `Measures`['latency_ms'] > '250'
-Measures.latency_ms:250 Measures``['latency_ms']` != CAST('250', 'Float64') ``` ❌
Flags.cached:true Flags``['cached']` = 1 ``` ❌
Flags.cached:"true" Flags``['cached']` = 1 ``` ❌

For a Map(String, Bool) column both the quoted and unquoted forms are affected, so there is no working spelling of a Bool-map search. Map(String, String) columns are unaffected (Attributes.host:web1`Attributes`['host'] ILIKE '%web1%'), as are plain non-map columns.

Reproduced against main @ e73af381.

Cause

Three call sites pass the already-rendered column expression as a SqlString ?? placeholder, which applies escapeId to it a second time:

  • packages/common-utils/src/queryParser.ts:502SQLSerializer.eq, Bool branch
  • packages/common-utils/src/queryParser.ts:1384CustomSchemaSQLSerializerV2.fieldSearch, Bool branch
  • packages/common-utils/src/queryParser.ts:1396CustomSchemaSQLSerializerV2.fieldSearch, Number branch

getColumnForField() returns column already rendered — `Measures`['latency_ms'] for a map key, and the bare name for a plain column. Every other branch interpolates it raw: eq/Number (:514), gte (:597), lte, lt, gt, range (:731) and the final ILIKE (:1554). Those three are the odd ones out.

git log -L shows :514 used ${column} and :1396 used ?? from the initial common-utils commit (6ee29abe) — the paths look copy-pasted and then diverged, rather than deliberately different.

Why the tests don't catch it

queryParser.test.ts:2735 (falls back for Map(String, Float64) value type) uses the quoted form NumericAttributes.count:"42", which routes to the correct eq/Number path, and asserts only not.toContain('has(') / toContain('CAST'). No test uses an unquoted numeric-map term, and no test uses a Map(String, Bool) column at all.

Repro

Drop this in packages/common-utils/src/__tests__/ and run yarn jest --ci zzprobe:

import { ClickhouseClient } from '@/clickhouse/node';
import { getMetadata } from '@/core/metadata';
import { CustomSchemaSQLSerializerV2, SearchQueryBuilder } from '@/queryParser';

describe('probe', () => {
  function buildSerializer() {
    const metadata = getMetadata(new ClickhouseClient({ host: 'http://localhost:8123' }));
    metadata.getColumn = jest.fn().mockImplementation(async ({ column }) => {
      if (column === 'Measures') return { name: 'Measures', type: 'Map(String, Float64)' };
      if (column === 'Flags') return { name: 'Flags', type: 'Map(String, Bool)' };
      if (column === 'Body') return { name: 'Body', type: 'String' };
      return undefined;
    });
    metadata.getMaterializedColumnsLookupTable = jest.fn().mockImplementation(async () => new Map());
    metadata.getColumns = jest.fn().mockImplementation(async () => [
      { name: 'Measures', type: 'Map(String, Float64)', default_type: '', default_expression: '' },
      { name: 'Flags', type: 'Map(String, Bool)', default_type: '', default_expression: '' },
    ]);
    metadata.getSkipIndices = jest.fn().mockImplementation(async () => []);
    metadata.getSetting = jest.fn().mockImplementation(async () => '0');
    metadata.getServerVersion = jest.fn().mockImplementation(async () => [26, 5, 0, 0] as const);
    return new CustomSchemaSQLSerializerV2({
      metadata,
      databaseName: 'default',
      tableName: 'otel_logs',
      connectionId: 'test',
      implicitColumnExpression: 'Body',
    });
  }

  it.each([
    'Measures.latency_ms:250',
    'Measures.latency_ms:"250"',
    'Flags.cached:true',
  ])('probe %s', async q => {
    process.stdout.write(`\n${q}\n  ${await new SearchQueryBuilder(q, buildSerializer()).build()}\n`);
    expect(1).toBe(1);
  });
});

Two possible fixes — I'd rather you picked

I have a patch ready either way, but the choice is yours to make because they differ in an observable way:

A. Match the sibling branches — swap ?? for ${column} at the three sites. Smallest possible diff and makes all branches consistent. But for plain (non-map) columns those three branches currently get their backticks from escapeId, so this drops them: IsError:true would go from `IsError` = 1 to IsError = 1. That matches what the ILIKE, gt/lt and range branches already emit today (ServiceName ILIKE '%foo%', unquoted), so it is consistent — but it does remove quoting that is currently there, which would bite a column named e.g. order.

B. Escape only when it is a bare column name — keep escapeId for plain columns and interpolate raw only for a rendered map expression (mapKeyIndexExpression is already available in fieldSearch as the discriminator). Preserves today's quoting for plain columns; slightly larger diff, and leaves the serializer inconsistent about quoting overall.

There is also a C — have getColumnForField() return a uniformly-escaped expression and interpolate raw everywhere — which is the tidiest end state but a much wider change.

Happy to open a PR with whichever you prefer, with tests covering unquoted numeric-map, negated numeric-map, and Bool-map searches in both directions. Just say which.


Found while working on #2764. This report was AI-assisted, per the AI-Assisted Development section of CONTRIBUTING.md; every SQL string above was executed, not inferred.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions