From 2461d168d9aac4aa62ed658984a4ecdbdcdd476e Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Wed, 25 Mar 2026 18:16:55 +0000 Subject: [PATCH 1/5] Fix: wrap nullable types in Nullable() for all column types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously only date/datetime types were wrapped in Nullable() when the JSON schema included "null". This caused nullable boolean (and other) columns from source databases to lose their NULL values — NULLs were silently converted to default values (false for bools, 0 for ints, etc). Now checks the JSON schema for "null" in the type array and wraps any non-primary-key column in Nullable() accordingly. --- target_clickhouse/connectors.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/target_clickhouse/connectors.py b/target_clickhouse/connectors.py index a2d7295..b86d9ed 100644 --- a/target_clickhouse/connectors.py +++ b/target_clickhouse/connectors.py @@ -128,6 +128,17 @@ def to_sql_type( ): sql_type = clickhouse_sqlalchemy_types.Nullable(sql_type) + # Wrap any type in Nullable if the JSON schema allows null values + # and it's not already Nullable and not a primary key. + schema_type = jsonschema_type.get("type", []) + if ( + isinstance(schema_type, list) + and "null" in schema_type + and not is_primary_key + and not isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable) + ): + sql_type = clickhouse_sqlalchemy_types.Nullable(sql_type) + return sql_type def create_empty_table( From 26c972438f9cce27b90a3263cbbc927659e4a82f Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Wed, 25 Mar 2026 18:32:49 +0000 Subject: [PATCH 2/5] Add unit tests for nullable type handling --- tests/test_nullable_types.py | 86 ++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 tests/test_nullable_types.py diff --git a/tests/test_nullable_types.py b/tests/test_nullable_types.py new file mode 100644 index 0000000..c84b27c --- /dev/null +++ b/tests/test_nullable_types.py @@ -0,0 +1,86 @@ +"""Tests for nullable type handling in to_sql_type.""" + +import sqlalchemy.types +from clickhouse_sqlalchemy import types as clickhouse_sqlalchemy_types +from unittest.mock import MagicMock + +from target_clickhouse.connectors import ClickhouseConnector + + +def _make_connector(): + """Create a ClickhouseConnector with mocked config.""" + connector = ClickhouseConnector.__new__(ClickhouseConnector) + connector.config = {} + connector.logger = MagicMock() + return connector + + +class TestNullableTypeMapping: + def test_nullable_bool_returns_nullable(self): + connector = _make_connector() + sql_type = connector.to_sql_type({"type": ["boolean", "null"]}) + assert isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable), ( + f"Expected Nullable, got {type(sql_type)}" + ) + + def test_non_nullable_bool_returns_plain_bool(self): + connector = _make_connector() + sql_type = connector.to_sql_type({"type": ["boolean"]}) + assert not isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable), ( + f"Expected non-Nullable, got {type(sql_type)}" + ) + + def test_nullable_integer_returns_nullable(self): + connector = _make_connector() + sql_type = connector.to_sql_type({"type": ["integer", "null"]}) + assert isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable), ( + f"Expected Nullable, got {type(sql_type)}" + ) + + def test_nullable_string_returns_nullable(self): + connector = _make_connector() + sql_type = connector.to_sql_type({"type": ["string", "null"]}) + assert isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable), ( + f"Expected Nullable, got {type(sql_type)}" + ) + + def test_nullable_number_returns_nullable(self): + connector = _make_connector() + sql_type = connector.to_sql_type({"type": ["number", "null"]}) + assert isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable), ( + f"Expected Nullable, got {type(sql_type)}" + ) + + def test_nullable_datetime_stays_nullable(self): + """Datetime was already wrapped in Nullable — should not double-wrap.""" + connector = _make_connector() + sql_type = connector.to_sql_type( + {"type": ["string", "null"], "format": "date-time"} + ) + assert isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable), ( + f"Expected Nullable, got {type(sql_type)}" + ) + + def test_primary_key_not_nullable(self): + connector = _make_connector() + sql_type = connector.to_sql_type( + {"type": ["boolean", "null"]}, is_primary_key=True + ) + assert not isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable), ( + f"Primary keys should not be Nullable, got {type(sql_type)}" + ) + + def test_non_nullable_integer_returns_int64(self): + connector = _make_connector() + sql_type = connector.to_sql_type({"type": ["integer"]}) + assert isinstance(sql_type, clickhouse_sqlalchemy_types.Int64), ( + f"Expected Int64, got {type(sql_type)}" + ) + + def test_string_type_not_list(self): + """Handle case where type is a string, not a list.""" + connector = _make_connector() + sql_type = connector.to_sql_type({"type": "boolean"}) + assert not isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable), ( + f"Expected non-Nullable for string type, got {type(sql_type)}" + ) From 2b40a98482723c1bddf0c752d52cd61a5dc71d59 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Wed, 25 Mar 2026 18:42:44 +0000 Subject: [PATCH 3/5] Fix test: use _config instead of config property --- tests/test_nullable_types.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_nullable_types.py b/tests/test_nullable_types.py index c84b27c..08b7ddb 100644 --- a/tests/test_nullable_types.py +++ b/tests/test_nullable_types.py @@ -2,15 +2,15 @@ import sqlalchemy.types from clickhouse_sqlalchemy import types as clickhouse_sqlalchemy_types -from unittest.mock import MagicMock +from unittest.mock import MagicMock, PropertyMock, patch from target_clickhouse.connectors import ClickhouseConnector def _make_connector(): - """Create a ClickhouseConnector with mocked config.""" + """Create a ClickhouseConnector without full initialization.""" connector = ClickhouseConnector.__new__(ClickhouseConnector) - connector.config = {} + connector._config = {} connector.logger = MagicMock() return connector From a3cf8e652efd6245312107a0531791582d028d90 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Wed, 25 Mar 2026 18:51:56 +0000 Subject: [PATCH 4/5] Fix tests: call to_sql_type as unbound method --- tests/test_nullable_types.py | 79 ++++++++++++------------------------ 1 file changed, 25 insertions(+), 54 deletions(-) diff --git a/tests/test_nullable_types.py b/tests/test_nullable_types.py index 08b7ddb..b6af7ea 100644 --- a/tests/test_nullable_types.py +++ b/tests/test_nullable_types.py @@ -1,86 +1,57 @@ """Tests for nullable type handling in to_sql_type.""" -import sqlalchemy.types from clickhouse_sqlalchemy import types as clickhouse_sqlalchemy_types -from unittest.mock import MagicMock, PropertyMock, patch +from singer_sdk import typing as th from target_clickhouse.connectors import ClickhouseConnector -def _make_connector(): - """Create a ClickhouseConnector without full initialization.""" - connector = ClickhouseConnector.__new__(ClickhouseConnector) - connector._config = {} - connector.logger = MagicMock() - return connector +def _to_sql_type(jsonschema_type, is_primary_key=False): + """Call to_sql_type as an unbound method — avoids full connector init.""" + return ClickhouseConnector.to_sql_type( + None, jsonschema_type, is_primary_key=is_primary_key + ) class TestNullableTypeMapping: def test_nullable_bool_returns_nullable(self): - connector = _make_connector() - sql_type = connector.to_sql_type({"type": ["boolean", "null"]}) - assert isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable), ( - f"Expected Nullable, got {type(sql_type)}" - ) + sql_type = _to_sql_type({"type": ["boolean", "null"]}) + assert isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable) def test_non_nullable_bool_returns_plain_bool(self): - connector = _make_connector() - sql_type = connector.to_sql_type({"type": ["boolean"]}) - assert not isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable), ( - f"Expected non-Nullable, got {type(sql_type)}" - ) + sql_type = _to_sql_type({"type": ["boolean"]}) + assert not isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable) def test_nullable_integer_returns_nullable(self): - connector = _make_connector() - sql_type = connector.to_sql_type({"type": ["integer", "null"]}) - assert isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable), ( - f"Expected Nullable, got {type(sql_type)}" - ) + sql_type = _to_sql_type({"type": ["integer", "null"]}) + assert isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable) def test_nullable_string_returns_nullable(self): - connector = _make_connector() - sql_type = connector.to_sql_type({"type": ["string", "null"]}) - assert isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable), ( - f"Expected Nullable, got {type(sql_type)}" - ) + sql_type = _to_sql_type({"type": ["string", "null"]}) + assert isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable) def test_nullable_number_returns_nullable(self): - connector = _make_connector() - sql_type = connector.to_sql_type({"type": ["number", "null"]}) - assert isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable), ( - f"Expected Nullable, got {type(sql_type)}" - ) + sql_type = _to_sql_type({"type": ["number", "null"]}) + assert isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable) def test_nullable_datetime_stays_nullable(self): - """Datetime was already wrapped in Nullable — should not double-wrap.""" - connector = _make_connector() - sql_type = connector.to_sql_type( + """Datetime was already wrapped — should not double-wrap.""" + sql_type = _to_sql_type( {"type": ["string", "null"], "format": "date-time"} ) - assert isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable), ( - f"Expected Nullable, got {type(sql_type)}" - ) + assert isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable) def test_primary_key_not_nullable(self): - connector = _make_connector() - sql_type = connector.to_sql_type( + sql_type = _to_sql_type( {"type": ["boolean", "null"]}, is_primary_key=True ) - assert not isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable), ( - f"Primary keys should not be Nullable, got {type(sql_type)}" - ) + assert not isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable) def test_non_nullable_integer_returns_int64(self): - connector = _make_connector() - sql_type = connector.to_sql_type({"type": ["integer"]}) - assert isinstance(sql_type, clickhouse_sqlalchemy_types.Int64), ( - f"Expected Int64, got {type(sql_type)}" - ) + sql_type = _to_sql_type({"type": ["integer"]}) + assert isinstance(sql_type, clickhouse_sqlalchemy_types.Int64) def test_string_type_not_list(self): """Handle case where type is a string, not a list.""" - connector = _make_connector() - sql_type = connector.to_sql_type({"type": "boolean"}) - assert not isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable), ( - f"Expected non-Nullable for string type, got {type(sql_type)}" - ) + sql_type = _to_sql_type({"type": "boolean"}) + assert not isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable) From ff24bdf3e4985bb48dbf5290f286333cef8f8b1c Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Wed, 25 Mar 2026 18:58:47 +0000 Subject: [PATCH 5/5] Fix lint: add docstrings, trailing commas, remove unused import --- tests/test_nullable_types.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/tests/test_nullable_types.py b/tests/test_nullable_types.py index b6af7ea..61c433f 100644 --- a/tests/test_nullable_types.py +++ b/tests/test_nullable_types.py @@ -1,7 +1,6 @@ """Tests for nullable type handling in to_sql_type.""" from clickhouse_sqlalchemy import types as clickhouse_sqlalchemy_types -from singer_sdk import typing as th from target_clickhouse.connectors import ClickhouseConnector @@ -9,45 +8,57 @@ def _to_sql_type(jsonschema_type, is_primary_key=False): """Call to_sql_type as an unbound method — avoids full connector init.""" return ClickhouseConnector.to_sql_type( - None, jsonschema_type, is_primary_key=is_primary_key + None, + jsonschema_type, + is_primary_key=is_primary_key, ) class TestNullableTypeMapping: + """Tests for nullable type wrapping in ClickhouseConnector.to_sql_type.""" + def test_nullable_bool_returns_nullable(self): + """Nullable bool schema should produce Nullable(Bool).""" sql_type = _to_sql_type({"type": ["boolean", "null"]}) assert isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable) def test_non_nullable_bool_returns_plain_bool(self): + """Non-nullable bool schema should not be wrapped.""" sql_type = _to_sql_type({"type": ["boolean"]}) assert not isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable) def test_nullable_integer_returns_nullable(self): + """Nullable integer schema should produce Nullable(Int64).""" sql_type = _to_sql_type({"type": ["integer", "null"]}) assert isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable) def test_nullable_string_returns_nullable(self): + """Nullable string schema should produce Nullable(String).""" sql_type = _to_sql_type({"type": ["string", "null"]}) assert isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable) def test_nullable_number_returns_nullable(self): + """Nullable number schema should produce Nullable(Float).""" sql_type = _to_sql_type({"type": ["number", "null"]}) assert isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable) def test_nullable_datetime_stays_nullable(self): """Datetime was already wrapped — should not double-wrap.""" sql_type = _to_sql_type( - {"type": ["string", "null"], "format": "date-time"} + {"type": ["string", "null"], "format": "date-time"}, ) assert isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable) def test_primary_key_not_nullable(self): + """Primary key columns should never be Nullable.""" sql_type = _to_sql_type( - {"type": ["boolean", "null"]}, is_primary_key=True + {"type": ["boolean", "null"]}, + is_primary_key=True, ) assert not isinstance(sql_type, clickhouse_sqlalchemy_types.Nullable) def test_non_nullable_integer_returns_int64(self): + """Non-nullable integer should produce Int64.""" sql_type = _to_sql_type({"type": ["integer"]}) assert isinstance(sql_type, clickhouse_sqlalchemy_types.Int64)