Two independent bugs with a common cause, filed together because the fix likely wants to be one decision. Both reproduce on main @ 02ce9d2, verified at runtime against a 25.10 server, and neither is covered by any existing test.
When ReadStringsAsByteArrays = true, StringType/FixedStringType return byte[] instead of string. Anything downstream that assumes string breaks — silently or loudly depending on the site.
Found while fixing the same class of bug in the JSON decoder (#485). Filing separately rather than widening that PR, since #2 below needs a design decision that shouldn't be bundled with a JSON fix.
1. GetSchema("Columns") throws InvalidCastException
ReadStringsAsByteArrays = true -> InvalidCastException: Unable to cast object of
type 'System.Byte[]' to type 'System.String'.
ReadStringsAsByteArrays = false -> 1 row(s), ProviderType=UInt8
Repro:
var settings = new ClickHouseClientSettings { /* host etc. */, ReadStringsAsByteArrays = true };
using var conn = new ClickHouseConnection(settings);
await conn.OpenAsync();
conn.GetSchema("Columns", new[] { "system", "one", null }); // throws
Utility/SchemaDescriber.cs:109:
var clickHouseType = TypeConverter.ParseClickHouseType((string)row["ProviderType"], TypeSettings.Default);
row["ProviderType"] = clickHouseType.ToString();
DescribeColumns runs a real query — SELECT database as Database, table as Table, name as Name, type as ProviderType, type as DataType FROM system.columns (SchemaDescriber.cs:85) — through ClickHouseDataAdapter.Fill. The reader is built with the connection's own TypeSettings (ADO/ClickHouseCommand.cs:210 → ClickHouseClient.cs:174), which carries the flag. All five projected columns are String, so every cell in the filled DataTable is a byte[] and the cast on line 109 fails.
There is a second failure hiding behind the first. The DataColumn CLR types come from GetFieldType → FrameworkType → byte[]. So fixing only the cast on 109 moves the exception to 110, where a string is written back into a byte[]-typed column (ArgumentException). Both need handling.
Suggested fix: have SchemaDescriber build its reader with readStringsAsByteArrays = false. This is the driver querying system.columns for its own metadata, not returning user data — that metadata is always text, and overriding at the source fixes the DataColumn types too rather than patching two call sites. (Utility/SchemaResolver.cs:92-95, the other internal query, reads header metadata only via WHERE 1=0 and never touches a value, so it is unaffected.)
ClickHouse.Driver.Tests/ADO/ConnectionTests.cs:314,322 already call GetSchema("Columns", …), but only with default settings.
2. GetString() returns the literal string "System.Byte[]"
ReadStringsAsByteArrays = true -> GetValue(0).GetType() = Byte[]
GetString(0) = "System.Byte[]"
ReadStringsAsByteArrays = false -> GetValue(0).GetType() = String
GetString(0) = "hello"
Repro: SELECT 'hello'::String, then reader.GetString(0).
ADO/Readers/ClickHouseDataReader.cs:188:
public override string GetString(int ordinal) => GetValue(ordinal)?.ToString();
object.ToString() on an array yields its type name. No exception, every row, and the result is plausible enough to travel a long way before anyone notices. This is the same shape as the JSON bug in #485 — a lenient catch-all converting a byte[] into convincing garbage.
The right semantic here is genuinely debatable, and it is not the same as the JSON case. #485 resolved its version by always UTF-8-decoding, justified by RFC 8259: a string inside a JSON document is text by definition. That reasoning does not transfer. GetString on a String column is precisely the case this flag exists for — the bytes may not be valid UTF-8, and lenient decoding would substitute U+FFFD for real data.
- (a) UTF-8 decode. Consistent with
GetString's existing leniency and with flag-off behaviour, making GetString flag-independent. But it silently transforms bytes the caller explicitly asked to receive raw.
- (b) Throw
InvalidCastException. Matches the reader's other typed accessors (GetByte/GetGuid/GetDecimal, :135-171), which already throw on a type mismatch. Tells the caller to use GetValue/GetFieldValue<byte[]>.
- (c) Strict decode (
UTF8Encoding(throwOnInvalidBytes: true)) — decode when it is unambiguously text, throw when it is not.
I lean (b), as the only option that never silently transforms data, and the flag is opt-in so anyone setting it knows they are handling bytes. Happy to implement whichever you prefer.
Related, same family
Formats/HttpParameterFormatter.cs:92-103 has a byte[] arm for FixedStringType but not for StringType, so a byte[] bound to a {x:String} placeholder formats as value.ToString() → "System.Byte[]". That is the write-path mirror of #2 and completes a read-then-filter round trip, so it probably wants the same decision.
Minor: Types/BinaryTypeDecoder.cs:330-340 DecodeCustomType ignores typeSettings and falls back to new StringType(), so it is a third flag construction site that does not honour the flag. No impact today (Custom types are the geo set, Float64 leaves only) — noting it for completeness.
Checked and clean
For whoever picks this up: the composite types are safe by construction. Array/Map/Tuple/Nested/Nullable/LowCardinality/Variant/Dynamic/QBit all derive element types from UnderlyingType.FrameworkType, which is already byte[] under the flag, so they stay internally consistent. Geo types have Float64 leaves only. POCO materialization fails fast with a descriptive InvalidOperationException from ValidateBinding before any row is materialized, so a byte[] never silently reaches a string setter. EnumType reads an integer and returns a name, never touching String decode.
I'm happy to open PRs for either or both — just let me know your preference on the #2 semantics first, since that determines the shape.
🤖 Generated with Claude Code
Two independent bugs with a common cause, filed together because the fix likely wants to be one decision. Both reproduce on
main@02ce9d2, verified at runtime against a 25.10 server, and neither is covered by any existing test.When
ReadStringsAsByteArrays = true,StringType/FixedStringTypereturnbyte[]instead ofstring. Anything downstream that assumesstringbreaks — silently or loudly depending on the site.Found while fixing the same class of bug in the JSON decoder (#485). Filing separately rather than widening that PR, since #2 below needs a design decision that shouldn't be bundled with a JSON fix.
1.
GetSchema("Columns")throwsInvalidCastExceptionRepro:
Utility/SchemaDescriber.cs:109:DescribeColumnsruns a real query —SELECT database as Database, table as Table, name as Name, type as ProviderType, type as DataType FROM system.columns(SchemaDescriber.cs:85) — throughClickHouseDataAdapter.Fill. The reader is built with the connection's ownTypeSettings(ADO/ClickHouseCommand.cs:210→ClickHouseClient.cs:174), which carries the flag. All five projected columns areString, so every cell in the filledDataTableis abyte[]and the cast on line 109 fails.There is a second failure hiding behind the first. The
DataColumnCLR types come fromGetFieldType→FrameworkType→byte[]. So fixing only the cast on 109 moves the exception to 110, where astringis written back into abyte[]-typed column (ArgumentException). Both need handling.Suggested fix: have
SchemaDescriberbuild its reader withreadStringsAsByteArrays = false. This is the driver queryingsystem.columnsfor its own metadata, not returning user data — that metadata is always text, and overriding at the source fixes theDataColumntypes too rather than patching two call sites. (Utility/SchemaResolver.cs:92-95, the other internal query, reads header metadata only viaWHERE 1=0and never touches a value, so it is unaffected.)ClickHouse.Driver.Tests/ADO/ConnectionTests.cs:314,322already callGetSchema("Columns", …), but only with default settings.2.
GetString()returns the literal string"System.Byte[]"Repro:
SELECT 'hello'::String, thenreader.GetString(0).ADO/Readers/ClickHouseDataReader.cs:188:object.ToString()on an array yields its type name. No exception, every row, and the result is plausible enough to travel a long way before anyone notices. This is the same shape as the JSON bug in #485 — a lenient catch-all converting abyte[]into convincing garbage.The right semantic here is genuinely debatable, and it is not the same as the JSON case. #485 resolved its version by always UTF-8-decoding, justified by RFC 8259: a string inside a JSON document is text by definition. That reasoning does not transfer.
GetStringon aStringcolumn is precisely the case this flag exists for — the bytes may not be valid UTF-8, and lenient decoding would substitute U+FFFD for real data.GetString's existing leniency and with flag-off behaviour, makingGetStringflag-independent. But it silently transforms bytes the caller explicitly asked to receive raw.InvalidCastException. Matches the reader's other typed accessors (GetByte/GetGuid/GetDecimal,:135-171), which already throw on a type mismatch. Tells the caller to useGetValue/GetFieldValue<byte[]>.UTF8Encoding(throwOnInvalidBytes: true)) — decode when it is unambiguously text, throw when it is not.I lean (b), as the only option that never silently transforms data, and the flag is opt-in so anyone setting it knows they are handling bytes. Happy to implement whichever you prefer.
Related, same family
Formats/HttpParameterFormatter.cs:92-103has abyte[]arm forFixedStringTypebut not forStringType, so abyte[]bound to a{x:String}placeholder formats asvalue.ToString()→"System.Byte[]". That is the write-path mirror of #2 and completes a read-then-filter round trip, so it probably wants the same decision.Minor:
Types/BinaryTypeDecoder.cs:330-340DecodeCustomTypeignorestypeSettingsand falls back tonew StringType(), so it is a third flag construction site that does not honour the flag. No impact today (Custom types are the geo set,Float64leaves only) — noting it for completeness.Checked and clean
For whoever picks this up: the composite types are safe by construction.
Array/Map/Tuple/Nested/Nullable/LowCardinality/Variant/Dynamic/QBitall derive element types fromUnderlyingType.FrameworkType, which is alreadybyte[]under the flag, so they stay internally consistent. Geo types haveFloat64leaves only. POCO materialization fails fast with a descriptiveInvalidOperationExceptionfromValidateBindingbefore any row is materialized, so abyte[]never silently reaches astringsetter.EnumTypereads an integer and returns a name, never touching String decode.I'm happy to open PRs for either or both — just let me know your preference on the #2 semantics first, since that determines the shape.
🤖 Generated with Claude Code