ClickHouse / ClickHouse/clickhouse-cs

JSON: reading a Dynamic/Variant-hinted path that holds an array throws InvalidOperationException; string values under a Dynamic hint are base64 under ReadStringsAsByteArrays

Open
#530 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
C#
Stars
94
Forks
22
Avg merge
11h 26m
Merged PRs (30d)
22

Description

### Describe the bug

Reading a `JSON` column that declares a typed path as `Dynamic` (or `Variant(...)`) fails hard when that path's value is an **array**: `ClickHouseDataReader.GetValue()` throws

```
System.InvalidOperationException: The element cannot be an object or array.
```

The whole row is unreadable — there is no way for the caller to get the value.

A second, related defect on the same code path: with `ReadStringsAsByteArrays=true`, a **string** value under a `Dynamic` hint comes back **base64-encoded** (`"dGV4dA=="` instead of `"text"`). This is the same corruption that #485 / PR #485 fixed for statically-typed string paths; the `Dynamic`/`Variant` case was left behind.

Both share one root cause (see below), which is why they are reported together.

### Steps to reproduce

1. `CREATE OR REPLACE TABLE t (data JSON(x Dynamic)) ENGINE = Memory`
2. `INSERT INTO t FORMAT JSONEachRow {"data": {"x": [1, 2, 3]}}`
3. `SELECT data FROM t` through the driver and call `reader.GetValue(0)` → throws.

### Expected behaviour

The driver should materialize the same document the server renders. The server is unambiguous:

```
SELECT toJSONString(data) FROM t
{"x":[1,2,3]}
```

Expected: `((JsonObject)reader.GetValue(0)).ToJsonString()` == `{"x":[1,2,3]}`, with `result["x"]` being a `JsonArray`. It is exactly what the driver already returns for the same value when the path is hinted `Array(Int64)` **or** left unhinted (a server-discovered dynamic path) — only an explicit `Dynamic`/`Variant` hint breaks.

For the second defect: `JSON(x Dynamic)` holding `"text"` should read back as `{"x":"text"}` under `ReadStringsAsByteArrays=true`, as `JSON(x String)` already does.

### Code example

```csharp
using var client = new ClickHouseClient("Host=localhost");
await client.ExecuteNonQueryAsync("CREATE OR REPLACE TABLE t (data JSON(x Dynamic)) ENGINE = Memory");
await client.ExecuteNonQueryAsync(@"INSERT INTO t FORMAT JSONEachRow {""data"": {""x"": [1, 2, 3]}}");

using var reader = await client.ExecuteReaderAsync("SELECT data FROM t");
reader.Read();
var result = (JsonObject)reader.GetValue(0); // throws InvalidOperationException
```

### Error log

```
System.InvalidOperationException: The element cannot be an object or array.
at System.Text.Json.Nodes.JsonValue.Create(JsonElement value, JsonNodeOptions? options)
at ClickHouse.Driver.Types.JsonType.ReadJsonValue(ExtendedBinaryReader reader, ClickHouseType type)
```

### Observed matrix

Verified against a live server (26.5.1.882) on current `main` (7b83764). `server` is `SELECT toJSONString(data)`; `driver` is `((JsonObject)reader.GetValue(0)).ToJsonString()`.

| Column type | Inserted value | server | driver |
|---|---|---|---|
| `JSON(x Dynamic)` | `[1, 2, 3]` | `{"x":[1,2,3]}` | **throws** |
| `JSON(x Dynamic)` | `[1, null, 3]` | `{"x":[1,null,3]}` | **throws** |
| `JSON(x Dynamic)` | `["a", "b"]` | `{"x":["a","b"]}` | **throws** |
| `JSON(x Dynamic)` | `[]` | `{"x":[]}` | **throws** |
| `JSON(x Dynamic)` | `[[1,2],[3]]` | `{"x":[[1,2],[3]]}` | **throws** |
| `JSON(x Variant(String, Array(Int64)))` | `[1, 2, 3]` | `{"x":[1,2,3]}` | **throws** |
| `JSON(x Dynamic)` | `"text"`, `ReadStringsAsByteArrays=true` | `{"x":"text"}` | `{"x":"dGV4dA=="}` |

Contrast cases that are already correct and must stay that way:

| Column type | Inserted value | driver |
|---|---|---|
| `JSON(x Array(Int64))` | `[1, 2, 3]` | `{"x":[1,2,3]}` (`JsonArray`) |
| `JSON` (unhinted) | `[1, 2, 3]` | `{"x":[1,2,3]}` (`JsonArray`) |
| `JSON` (unhinted) | `{"k":"v"}` | `{"x":{"k":"v"}}` |
| `JSON(x Map(String, Int64))` | `{"k":1}` | `{"x":{"k":1}}` |
| `JSON(x Dynamic)` | `{"k":"v"}` | `{"x":{"k":"v"}}` |
| `JSON(x Dynamic)` | `42` / `"text"` (default settings) | `{"x":42}` / `{"x":"text"}` |
| `JSON(x Variant(String, Array(Int64)))` | `"s"` | `{"x":"s"}` |
| `JSON(x String)` | `"text"`, `ReadStringsAsByteArrays=true` | `{"x":"text"}` |

### Root cause

`ClickHouse.Driver/Types/JsonType.cs`.

`ReadJsonNode` (line 312) dispatches on the **static** hinted ClickHouse type:

```csharp
var type = hintedType ?? BinaryTypeDecoder.FromByteCode(reader, TypeSettings);
return type switch
{
ArrayType at => ReadJsonArray(reader, at),
MapType mt => ReadJsonMap(reader, mt),
FixedStringType => ReadJsonFixedString(reader, type),
_ => ReadJsonValue(reader, type),
};
```

`DynamicType` and `VariantType` are containers whose concrete shape is only known **per value**, so they match no arm and fall into `ReadJsonValue(reader, type)`, where `type.Read(reader)` decodes the value opaquely (`DynamicType.Read` → `BinaryTypeDecoder.FromByteCode(reader, …).Read(reader)`; `VariantType.Read` → discriminator byte, then the selected alternative). Two consequences:

1. An array arrives as a CLR array, matches no arm of `ReadJsonValue`'s switch, and hits the default at line 416 — `JsonValue.Create(JsonSerializer.SerializeToElement(value))`. `JsonValue.Create(JsonElement)` [throws by contract](https://learn.microsoft.com/dotnet/api/system.text.json.nodes.jsonvalue.create) when the element's `ValueKind` is `Object` or `Array`. Hence the exception. (A subobject value survives only by accident: `JsonType.Read` returns a `JsonObject`, which is caught by the earlier `JsonObject jo => jo` arm.)
2. The `IsTextBacked(type)` guard at line 396 — the one added for #485 — is evaluated against the *static* type, which here is `Dynamic`/`Variant`, and `IsTextBacked` deliberately returns `false` for those (line 368-375: "their subtype is only known per value"). So a `byte[]` from a string under a `Dynamic` hint skips the decode arm and reaches the same default, which renders it base64.

The unhinted path is unaffected because `BinaryTypeDecoder.FromByteCode` has already resolved the concrete type before the `switch` runs.

### Suggested fix

Resolve the per-value concrete type *before* dispatching, then re-dispatch on it, so `Dynamic`/`Variant` reuse the existing `ArrayType`/`MapType`/`FixedString`/scalar arms instead of bypassing them:

- `DynamicType` → read the type header (`BinaryTypeDecoder.FromByteCode(reader, TypeSettings)`) and recurse `ReadJsonNode(reader, concreteType)`. This is the same thing `DynamicType.Read` does internally, just with the type handed to the JSON-aware dispatcher rather than to a plain `Read`.
- `VariantType` → read the discriminator byte; `0xFF` is the null discriminant (`VariantType.Read` maps it to `DBNull`), otherwise recurse with `UnderlyingTypes[discriminator]`.

Doing it at the dispatcher level fixes both symptoms at once (the array now goes to `ReadJsonArray`, the string now goes through a `StringType`/`FixedStringType` for which `IsTextBacked` is true) and also makes nested cases behave — e.g. `DBNull` elements inside such a container become JSON `null` via `ReadJsonValue`'s existing `DBNull → null` handling rather than serializing as `{}`.

Please keep the contrast rows above as regression coverage — in particular the unhinted-path and `JSON(x Array(...))`/`JSON(x Map(...))` cases, which must keep their current output, and `Array(UInt8)` under a `Dynamic` hint, which must **not** be text-decoded.

### Configuration

#### Environment
* Client version: current `main` (commit 7b83764)
* Language version: C# / .NET 10
* .NET version: 10.0.10
* OS: Ubuntu 24.04 (x64)

#### ClickHouse server
* ClickHouse Server version: 26.5.1.882
* ClickHouse Server non-default settings, if any: none
* `CREATE TABLE` statements for tables involved:
```sql
CREATE OR REPLACE TABLE t (data JSON(x Dynamic)) ENGINE = Memory;
CREATE OR REPLACE TABLE t2 (data JSON(x Variant(String, Array(Int64)))) ENGINE = Memory;
```
* Sample data: `{"x": [1, 2, 3]}` (see the matrix above for the full set)

---

Found by automated analysis of the JSON read path while working on #521 / PR #529 (which fixes the unrelated scalar-null case and does not touch this dispatch). Verified against a live ClickHouse server rather than by inspection; every row in the tables above was executed.

Contributor guide

Open the contributing guide

Research direction

Start in ClickHouse.Driver/Types/JsonType.cs at ReadJsonNode and inspect how DynamicType and VariantType are decoded before the existing ArrayType, MapType, FixedStringType, and scalar handling. Reproduce the JSON(x Dynamic) array and string cases from the issue, then verify that arrays and nested nulls materialize correctly, strings remain unencoded with ReadStringsAsByteArrays=true, and the listed contrast cases are unchanged.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
backend-api-design, databases
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.