ClickHouse / ClickHouse/clickhouse-cs
Reading SimpleAggregateFunction inside a Dynamic column throws NotImplementedException
- Dominant language
- C#
- Stars
- 94
- Forks
- 22
- Avg merge
- 11h 26m
- Merged PRs (30d)
- 22
Description
## Description
The server keeps `SimpleAggregateFunction(func, T)` as a concrete `Dynamic` subtype and encodes it in the [binary type encoding](https://clickhouse.com/docs/en/sql-reference/data-types/data-types-binary-encoding) as:
```
0x2E
```
`BinaryTypeDecoder.FromByteCode` dispatches `BinaryTypeIndex.SimpleAggregateFunction` (0x2E) to `DecodeSimpleAggregateFunction`, which is an unimplemented stub:
```csharp
// ClickHouse.Driver/Types/BinaryTypeDecoder.cs:300
private static SimpleAggregateFunctionType DecodeSimpleAggregateFunction(ExtendedBinaryReader reader)
{
throw new NotImplementedException("SimpleAggregateFunction decoding not implemented.");
}
```
So reading a `SimpleAggregateFunction` value out of a `Dynamic` column throws and the query fails. The same `FromByteCode` path is used by `VariantType`, `JsonType` and nested `Dynamic`, so those are affected too.
Note that a **top-level** `SimpleAggregateFunction` column works fine — that path goes through the textual type-name grammar (`SimpleAggregateFunctionType.Parse`), not the binary decoder. Only the binary-type-encoding path is broken.
Related: `DecodeAggregateFunction` (0x1E) is the same kind of stub at `BinaryTypeDecoder.cs:295`.
## ClickHouse server version
26.7.2.59 (verified against a running server)
## Reproduction
```csharp
using ClickHouse.Driver.Utility;
public class SafDynamicTests : AbstractConnectionTestFixture
{
[Test]
public async Task ShouldReadSimpleAggregateFunctionInDynamic()
{
using var reader = await connection.ExecuteReaderAsync(
"SELECT CAST(CAST(42, 'SimpleAggregateFunction(sum, UInt64)') AS Dynamic) AS d, 42::Int32 AS tail");
Assert.That(reader.Read(), Is.True);
Assert.That(reader.GetValue(0), Is.EqualTo((ulong)42));
Assert.That(reader.GetValue(1), Is.EqualTo(42));
}
[Test] // this one passes today (textual type-name path)
public async Task ShouldReadTopLevelSimpleAggregateFunction()
{
using var reader = await connection.ExecuteReaderAsync(
"SELECT CAST(42, 'SimpleAggregateFunction(sum, UInt64)') AS d, 42::Int32 AS tail");
Assert.That(reader.Read(), Is.True);
Assert.That(reader.GetValue(0), Is.EqualTo((ulong)42));
Assert.That(reader.GetValue(1), Is.EqualTo(42));
}
}
```
Expected: both tests pass, `d` reads as `42UL` and `tail` as `42`.
Actual: `ShouldReadTopLevelSimpleAggregateFunction` passes, `ShouldReadSimpleAggregateFunctionInDynamic` fails:
```
Failed ShouldReadSimpleAggregateFunctionInDynamic [17 ms]
System.NotImplementedException : SimpleAggregateFunction decoding not implemented.
at ClickHouse.Driver.Types.BinaryTypeDecoder.DecodeSimpleAggregateFunction(ExtendedBinaryReader reader) in ClickHouse.Driver/Types/BinaryTypeDecoder.cs:line 302
at ClickHouse.Driver.Types.BinaryTypeDecoder.FromByteCode(ExtendedBinaryReader reader, TypeSettings typeSettings) in ClickHouse.Driver/Types/BinaryTypeDecoder.cs:line 176
at ClickHouse.Driver.Types.DynamicType.Read(ExtendedBinaryReader reader) in ClickHouse.Driver/Types/DynamicType.cs:line 21
at ClickHouse.Driver.ADO.Readers.ClickHouseDataReader.Read() in ClickHouse.Driver/ADO/Readers/ClickHouseDataReader.cs:line 458
```
## Suggested fix
Implement `DecodeSimpleAggregateFunction` in `ClickHouse.Driver/Types/BinaryTypeDecoder.cs:300` to consume the full encoding and return a usable type:
1. `reader.ReadString()` — function name
2. `reader.Read7BitEncodedInt()` parameters, each a field-value encoding — must be consumed even when ignored
3. `reader.Read7BitEncodedInt()` arguments, each `FromByteCode(reader, typeSettings)`
Return `new SimpleAggregateFunctionType { AggregateFunction = name, UnderlyingType = arguments[0] }` (its `Read` already delegates to the underlying type). Consuming the whole encoding matters independently of the exception: if the tag were skipped without consuming the payload, the function name and argument encodings would be interpreted as row data and desynchronize the rest of the RowBinary stream. Note the signature needs `TypeSettings` threaded in to decode argument types.
`Nothing` (0x00) parameters aside, ClickHouse currently only emits parameterized `SimpleAggregateFunction` for a few functions, but the parameter section must still be skipped correctly.
## Link
Relayed from https://github.com/ClickHouse/clickhouse-java/issues/3005
Contributor guide
Research direction
Start in ClickHouse.Driver/Types/BinaryTypeDecoder.cs at DecodeSimpleAggregateFunction and inspect the FromByteCode path, then run the supplied ShouldReadSimpleAggregateFunctionInDynamic reproduction. Compare it with SimpleAggregateFunctionType.Parse and the top-level passing test; done means the Dynamic read returns 42UL and the following tail value remains readable.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 88/100