ClickHouse / ClickHouse/clickhouse-js
Map(K, V) with duplicate keys silently loses entries (RowBinary readMap collapses pairs into a JS Map)
- 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
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