Conversation
Allow `Nullable(x)` to work in the binary driver for Numeric, Text, `UUID`, and `INET` columns. Add tests for all other currently supported columns. Raise exceptions for Enums, which cannot currently be `NULL` (but there should be a way to allow it like the http driver), as well as `LowCardinality(Nullable(x))`, which is allowed but currently unreachable. It's unreachable because the binary driver currently does not allow `NULL`s unless the column is explicitly `Nullable()` and `Nullable(LowCardinality(x))` is not supported by ClickHouse. But leave the exception, because we may want to eliminate that constraint and let `NULL`s go through, as ClickHouse will treat them as defaults. For example `NULL` for `Int8` becomes `0`, for `IPv6` becomes `::`. This is the behavior of the http driver, but we may want to remove it there, too, at least when the Postgres column has a `NOT NULL` constraint. Leave a long comment explaining this state of affairs While at it, improve the error message from that block, as it was confusing before. Add tests for all of the data types currently supported by the binary driver to both the binary and the http driver insertion tests. In the latter case, `NULL` enums work properly, and as explained above, default values are replaced for `NULL`s. The tests also verify array behavior, which currently disallows nested arrays. Sadly, `ARRAY[NULL]` fails on ClickHouse 23, so we need another expected output file.
serprex
approved these changes
Feb 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allow
Nullable(x)to work in the binary driver for Numeric, Text,UUID, andINETcolumns. Add tests for all other currently supported columns. Raise exceptions for Enums, which cannot currently beNULL(but there should be a way to allow it like the http driver), as well asLowCardinality(Nullable(x)), which is allowed but currently unreachable.It's unreachable because the binary driver currently does not allow
NULLs unless the column is explicitlyNullable()andNullable(LowCardinality(x))is not supported by ClickHouse.But leave the exception, because we may want to eliminate that constraint and let
NULLs go through, as ClickHouse will treat them as defaults. For exampleNULLforInt8becomes0, forIPv6becomes::. This is the behavior of the http driver, but we may want to remove it there, too, at least when the Postgres column has aNOT NULLconstraint. Leave a long comment explaining this state of affairsWhile at it, improve the error message from that block, as it was confusing before.
Add tests for all of the data types currently supported by the binary driver to both the binary and the http driver insertion tests. In the latter case,
NULLenums work properly, and as explained above, default values are replaced forNULLs. The tests also verify array behavior, which currently disallows nested arrays. Sadly,ARRAY[NULL]fails on ClickHouse 23, so we need another expected output file.Fixes #140.