From 3dae1a4e5f2e8569432d7a85399b0f4d4eedc99e Mon Sep 17 00:00:00 2001 From: Florin Date: Fri, 26 Jun 2026 17:30:37 +0200 Subject: [PATCH 1/4] Add support for `sa.Time` type by storing it as a `STRING` in ISO 8601 format --- src/sqlalchemy_cratedb/compiler.py | 9 +++++++++ src/sqlalchemy_cratedb/dialect.py | 23 +++++++++++++++++++++- tests/create_table_test.py | 24 +++++++++++++++++++++++ tests/datetime_test.py | 31 ++++++++++++++++++++++++++++++ 4 files changed, 86 insertions(+), 1 deletion(-) diff --git a/src/sqlalchemy_cratedb/compiler.py b/src/sqlalchemy_cratedb/compiler.py index 033c116d..b2de8457 100644 --- a/src/sqlalchemy_cratedb/compiler.py +++ b/src/sqlalchemy_cratedb/compiler.py @@ -246,6 +246,15 @@ def visit_datetime(self, type_, **kw): def visit_date(self, type_, **kw): return "TIMESTAMP" + def visit_time(self, type_, **kw): + """ + CrateDB has no storable `TIME` column type. Plain `TIME` does not exist, + and `TIME WITH TIME ZONE` (TIMETZ) is literal/cast-only ("does not support + storage"). So store the time-of-day as a `STRING`, holding the value in + ISO 8601 format (e.g. ``19:00:30.123456``). + """ + return "STRING" + def visit_ARRAY(self, type_, **kw): if type_.dimensions is not None and type_.dimensions > 1: raise NotImplementedError("CrateDB doesn't support multidimensional arrays") diff --git a/src/sqlalchemy_cratedb/dialect.py b/src/sqlalchemy_cratedb/dialect.py index b75c54f6..ccffbc59 100644 --- a/src/sqlalchemy_cratedb/dialect.py +++ b/src/sqlalchemy_cratedb/dialect.py @@ -21,7 +21,7 @@ import logging import warnings -from datetime import date, datetime +from datetime import date, datetime, time from sqlalchemy import types as sqltypes from sqlalchemy.engine import default, reflection @@ -167,10 +167,31 @@ def process(value): return process +class Time(sqltypes.Time): + def bind_processor(self, dialect): + def process(value): + if value is not None: + return value.isoformat() + return None + + return process + + def result_processor(self, dialect, coltype): + def process(value): + if value is None: + return None + if isinstance(value, time): + return value + return time.fromisoformat(value) + + return process + + colspecs = { sqltypes.Date: Date, sqltypes.DateTime: DateTime, sqltypes.TIMESTAMP: DateTime, + sqltypes.Time: Time, } if SA_VERSION >= SA_2_0: diff --git a/tests/create_table_test.py b/tests/create_table_test.py index 04674da1..13e8dd64 100644 --- a/tests/create_table_test.py +++ b/tests/create_table_test.py @@ -78,6 +78,30 @@ class User(self.Base): sa.util.immutabledict({}), ) + def test_table_time_type(self): + """ + `sa.Time` has no native CrateDB counterpart and is stored as `STRING`. + + Validates the fix for https://github.com/crate/sqlalchemy-cratedb/issues/206, + where `sa.Time` previously compiled to `TIME`, which CrateDB rejects with + `SQLParseException[Cannot find data type: time]`. + """ + + class Schedule(self.Base): + __tablename__ = "schedule" + name = sa.Column(sa.String, primary_key=True) + time_col = sa.Column(sa.Time) + + self.Base.metadata.create_all(bind=self.engine) + fake_cursor.execute.assert_called_with( + ( + "\nCREATE TABLE schedule (\n\tname STRING NOT NULL, " + "\n\ttime_col STRING, " + "\n\tPRIMARY KEY (name)\n)\n\n" + ), + sa.util.immutabledict({}), + ) + def test_column_obj(self): class DummyTable(self.Base): __tablename__ = "dummy" diff --git a/tests/datetime_test.py b/tests/datetime_test.py index da0ea4a0..95653431 100644 --- a/tests/datetime_test.py +++ b/tests/datetime_test.py @@ -105,6 +105,7 @@ class FooBar(Base): date = sa.Column(sa.Date) datetime_notz = sa.Column(DateTime(timezone=False)) datetime_tz = sa.Column(DateTime(timezone=True)) + time = sa.Column(sa.Time) @pytest.fixture @@ -228,3 +229,33 @@ def test_datetime_date(session): # Compare outcome. assert result["datetime_notz"] == dt.datetime(2009, 5, 13, 0, 0, 0) assert result["datetime_tz"] == dt.datetime(2009, 5, 13, 0, 0, 0) + + +@pytest.mark.skipif(SA_VERSION < SA_1_4, reason="Test case not supported on SQLAlchemy 1.3") +def test_time(session): + """ + An integration test for `sa.Time`. + + CrateDB has no native `TIME` type, so the dialect stores it as a `STRING` + in ISO 8601 format and parses it back into a `dt.time` object on read. + + Validates the fix for https://github.com/crate/sqlalchemy-cratedb/issues/206. + """ + + # insert + foo_item = FooBar( + name="foo", + time=dt.time(19, 0, 30, 123456), + ) + session.add(foo_item) + session.commit() + session.execute(sa.text("REFRESH TABLE foobar")) + + # query + result = ( + session.execute(sa.select(FooBar.name, FooBar.time)).mappings().first() + ) + + # compare + assert result["time"] == dt.time(19, 0, 30, 123456) + assert isinstance(result["time"], dt.time) From 2f00b1c46dc668d9488148f048cf05f7ddd1fa4f Mon Sep 17 00:00:00 2001 From: Florin Date: Mon, 29 Jun 2026 12:40:53 +0200 Subject: [PATCH 2/4] linter fix --- tests/datetime_test.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/datetime_test.py b/tests/datetime_test.py index 95653431..52701572 100644 --- a/tests/datetime_test.py +++ b/tests/datetime_test.py @@ -252,9 +252,7 @@ def test_time(session): session.execute(sa.text("REFRESH TABLE foobar")) # query - result = ( - session.execute(sa.select(FooBar.name, FooBar.time)).mappings().first() - ) + result = session.execute(sa.select(FooBar.name, FooBar.time)).mappings().first() # compare assert result["time"] == dt.time(19, 0, 30, 123456) From c425a61cd54cd95b0e60331e4935185662fda42c Mon Sep 17 00:00:00 2001 From: Florin Date: Tue, 30 Jun 2026 14:19:27 +0200 Subject: [PATCH 3/4] Add support for `sa.TIME` alongside `sa.Time`, ensuring consistent handling and round-trip behavior --- src/sqlalchemy_cratedb/compiler.py | 2 +- src/sqlalchemy_cratedb/dialect.py | 3 +-- tests/create_table_test.py | 13 ++++++++++--- tests/datetime_test.py | 15 ++++++++++++--- 4 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/sqlalchemy_cratedb/compiler.py b/src/sqlalchemy_cratedb/compiler.py index b2de8457..7a8335ea 100644 --- a/src/sqlalchemy_cratedb/compiler.py +++ b/src/sqlalchemy_cratedb/compiler.py @@ -246,7 +246,7 @@ def visit_datetime(self, type_, **kw): def visit_date(self, type_, **kw): return "TIMESTAMP" - def visit_time(self, type_, **kw): + def visit_TIME(self, type_, **kw): """ CrateDB has no storable `TIME` column type. Plain `TIME` does not exist, and `TIME WITH TIME ZONE` (TIMETZ) is literal/cast-only ("does not support diff --git a/src/sqlalchemy_cratedb/dialect.py b/src/sqlalchemy_cratedb/dialect.py index ccffbc59..70a268a7 100644 --- a/src/sqlalchemy_cratedb/dialect.py +++ b/src/sqlalchemy_cratedb/dialect.py @@ -171,6 +171,7 @@ class Time(sqltypes.Time): def bind_processor(self, dialect): def process(value): if value is not None: + assert isinstance(value, time) # noqa: S101 return value.isoformat() return None @@ -180,8 +181,6 @@ def result_processor(self, dialect, coltype): def process(value): if value is None: return None - if isinstance(value, time): - return value return time.fromisoformat(value) return process diff --git a/tests/create_table_test.py b/tests/create_table_test.py index 13e8dd64..dc4e53c8 100644 --- a/tests/create_table_test.py +++ b/tests/create_table_test.py @@ -84,19 +84,26 @@ def test_table_time_type(self): Validates the fix for https://github.com/crate/sqlalchemy-cratedb/issues/206, where `sa.Time` previously compiled to `TIME`, which CrateDB rejects with - `SQLParseException[Cannot find data type: time]`. + `SQLParseException[Cannot find data type: time]`. Both the generic + `sa.Time` and the SQL-standard capitalised `sa.TIME` must render `STRING`. """ class Schedule(self.Base): __tablename__ = "schedule" name = sa.Column(sa.String, primary_key=True) - time_col = sa.Column(sa.Time) + time_lower = sa.Column(sa.Time) + time_upper = sa.Column(sa.TIME()) + + # Exercise both spellings: `sa.TIME` is a subclass of `sa.Time`. + assert isinstance(Schedule.__table__.c.time_lower.type, sa.Time) + assert isinstance(Schedule.__table__.c.time_upper.type, sa.TIME) self.Base.metadata.create_all(bind=self.engine) fake_cursor.execute.assert_called_with( ( "\nCREATE TABLE schedule (\n\tname STRING NOT NULL, " - "\n\ttime_col STRING, " + "\n\ttime_lower STRING, " + "\n\ttime_upper STRING, " "\n\tPRIMARY KEY (name)\n)\n\n" ), sa.util.immutabledict({}), diff --git a/tests/datetime_test.py b/tests/datetime_test.py index 52701572..c80e606e 100644 --- a/tests/datetime_test.py +++ b/tests/datetime_test.py @@ -106,6 +106,7 @@ class FooBar(Base): datetime_notz = sa.Column(DateTime(timezone=False)) datetime_tz = sa.Column(DateTime(timezone=True)) time = sa.Column(sa.Time) + time_upper = sa.Column(sa.TIME()) @pytest.fixture @@ -234,10 +235,11 @@ def test_datetime_date(session): @pytest.mark.skipif(SA_VERSION < SA_1_4, reason="Test case not supported on SQLAlchemy 1.3") def test_time(session): """ - An integration test for `sa.Time`. + An integration test for `sa.Time` and the SQL-standard `sa.TIME`. CrateDB has no native `TIME` type, so the dialect stores it as a `STRING` - in ISO 8601 format and parses it back into a `dt.time` object on read. + in ISO 8601 format and parses it back into a `dt.time` object on read. Both + spellings resolve to the same colspec, so both must round-trip. Validates the fix for https://github.com/crate/sqlalchemy-cratedb/issues/206. """ @@ -246,14 +248,21 @@ def test_time(session): foo_item = FooBar( name="foo", time=dt.time(19, 0, 30, 123456), + time_upper=dt.time(19, 0, 30, 123456), ) session.add(foo_item) session.commit() session.execute(sa.text("REFRESH TABLE foobar")) # query - result = session.execute(sa.select(FooBar.name, FooBar.time)).mappings().first() + result = ( + session.execute(sa.select(FooBar.name, FooBar.time, FooBar.time_upper)) + .mappings() + .first() + ) # compare assert result["time"] == dt.time(19, 0, 30, 123456) assert isinstance(result["time"], dt.time) + assert result["time_upper"] == dt.time(19, 0, 30, 123456) + assert isinstance(result["time_upper"], dt.time) From f226112cb19a8759a676b02743e15994467fffe4 Mon Sep 17 00:00:00 2001 From: Florin Date: Tue, 30 Jun 2026 14:39:39 +0200 Subject: [PATCH 4/4] lint --- tests/datetime_test.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/datetime_test.py b/tests/datetime_test.py index c80e606e..ebb55f88 100644 --- a/tests/datetime_test.py +++ b/tests/datetime_test.py @@ -256,9 +256,7 @@ def test_time(session): # query result = ( - session.execute(sa.select(FooBar.name, FooBar.time, FooBar.time_upper)) - .mappings() - .first() + session.execute(sa.select(FooBar.name, FooBar.time, FooBar.time_upper)).mappings().first() ) # compare