Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

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

Aperta
  • Qualcosa non va?
#2,765 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 2 giorni

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
3/5
Tempo stimato
1-2 giorni
Idoneità per principianti
68/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Tranquilla
Stack tecnologico
clickhouse, typescript
Ambito
backend, databases, testing-qa

Direzione di ricerca

Il codice interessato si trova in packages/common-utils/src/queryParser.ts, in SQLSerializer.eq e CustomSchemaSQLSerializerV2.fieldSearch; inizia leggendo i tre punti di chiamata di ?? insieme ai rami di interpolazione corrispondenti. Estendi packages/common-utils/src/tests/queryParser.test.ts intorno al caso Map(String, Float64) con ricerche in map numeriche senza virgolette e negate, oltre a ricerche in map Bool. Il lavoro è completato quando i predicati generati non fanno più il doppio escaping dei pedici delle map e i test Jest pertinenti superano.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

external
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:502 — SQLSerializer.eq, Bool branch
  • packages/common-utils/src/queryParser.ts:1384 — CustomSchemaSQLSerializerV2.fieldSearch, Bool branch
  • packages/common-utils/src/queryParser.ts:1396 — CustomSchemaSQLSerializerV2.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.

Lingua principale
TypeScript
Stelle
10k
Fork
501
Merge medio
2g 3h
PR unite (30g)
152

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di hyperdxio/hyperdx

Tutte le issue di hyperdxio/hyperdx

Issue simili

Altre issue su TypeScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.