ClickHouse / ClickHouse/clickhouse-js

Map(K, V) with duplicate keys silently loses entries (RowBinary readMap collapses pairs into a JS Map)

Open
#996 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
331
Forks
74
PR merge metrics
No merged PRs in 30d

Description

## Description

In ClickHouse, `Map(K, V)` is **not** a collection of unique-by-key pairs — [the docs](https://clickhouse.com/docs/sql-reference/data-types/map) state that "a map can contain two elements with the same key". The RowBinary wire format faithfully carries every pair (it is a flattened `Array(Tuple(K, V))` with a LEB128 count).

`@clickhouse/rowbinary` reads all of those pairs off the wire but then folds them into a JS `Map` via `out.set(key, ...)`, so pairs sharing a key are merged and only the last value survives. The data is read and then thrown away — there is no way for a caller to recover the duplicates. This affects every place a map appears: top level, map-in-map, map-in-array, and the compiled/monomorphized readers (`src/readers/compile.ts`, `src/readers/dynamic.ts`) which emit the same `Map.set` shape.

Relevant code: [`skills/clickhouse-js-node-rowbinary/src/readers/composite.ts:134-146`](https://github.com/ClickHouse/clickhouse-js/blob/main/skills/clickhouse-js-node-rowbinary/src/readers/composite.ts#L134).

Note the JSON-format path in `@clickhouse/client` has the same observable outcome, but for a different reason: the server emits `{"m":{"key":"X","key":"Y"}}` for `SELECT map('key','X','key','Y') FORMAT JSONEachRow`, and `JSON.parse` keeps the last duplicate key. That one is inherent to JSON; the RowBinary reader is the case where the client itself has the full data and discards it.

## ClickHouse server version

`26.7.3.19` (verified against a running server).

## Reproduction

Add to `skills/clickhouse-js-node-rowbinary/tests/` and run with `npx vitest run tests/MapDup.test.ts` from `skills/clickhouse-js-node-rowbinary`:

```ts
import { describe, expect, it } from "vitest";
import { query } from "./clickhouse.js";
import { readMap } from "../src/readers/composite.js";
import { Cursor } from "../src/readers/core.js";
import { readString } from "../src/readers/strings.js";

describe("Map with duplicate keys", () => {
it("wire carries both pairs, readMap collapses them", async () => {
const r = new Cursor(
await query(
"SELECT CAST(map('key', 'X', 'key', 'Y') AS Map(String, String)) FORMAT RowBinary",
),
);
const m = readMap(readString, readString)(r);

// Both pairs ARE on the wire and ARE consumed:
// 1 count byte + 2 * ((1 + 3) key + (1 + 1) value) = 13
expect(r.pos).toBe(13);

// ...but only one entry survives.
expect(m.size).toBe(2); // FAILS: got 1
});
});
```

Actual result:

```
AssertionError: expected 1 to be 2 // Object.is equality
❯ tests/MapDup.test.ts:23:20
```

`r.pos` is `13`, confirming both key/value pairs were decoded off the wire; the returned value is `Map(1) { 'key' => 'Y' }` — the `'X'` pair is silently dropped.

## Suggested fix

Not attempting a fix here, just pointing at the code. Options, roughly in increasing order of disruption:

1. Document the lossy behavior explicitly in the `readMap` JSDoc (`composite.ts:122-133`) and in `reader.md` — today the doc comment says a JS `Map` "keeps insertion order and accepts any key type", which reads as if nothing is lost.
2. Offer an opt-in pair-preserving reader, e.g. `readMapEntries(readKey, readValue): Reader>` returning the raw pair list, so callers who care about duplicates (or key ordering with duplicate keys) can use it. `readMap` would then be a thin wrapper over it. The compiled readers in `compile.ts` / `dynamic.ts` would need the same switch.
3. Change the default return type for `Map(K, V)` to an array of pairs in a future major — matches the server semantics but is a breaking change for every existing consumer.

## Link

Relayed from the equivalent report against the Java client: https://github.com/ClickHouse/clickhouse-java/issues/3047

Contributor guide

Open the contributing guide

Research direction

Start with the reproduction in skills/clickhouse-js-node-rowbinary/tests/ and inspect readMap in src/readers/composite.ts:134-146, then compare the compiled and dynamic readers in src/readers/compile.ts and src/readers/dynamic.ts. Review the readMap JSDoc and reader.md before choosing the intended duplicate-key behavior; done should include regression coverage for the affected map nesting and reader paths.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
databases
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.