diff --git a/src/sqlalchemy_cratedb/compiler.py b/src/sqlalchemy_cratedb/compiler.py index 033c116..7a8335e 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 b75c54f..70a268a 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,30 @@ def process(value): return process +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 + + return process + + def result_processor(self, dialect, coltype): + def process(value): + if value is None: + return None + 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 04674da..dc4e53c 100644 --- a/tests/create_table_test.py +++ b/tests/create_table_test.py @@ -78,6 +78,37 @@ 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]`. 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_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_lower STRING, " + "\n\ttime_upper 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 da0ea4a..ebb55f8 100644 --- a/tests/datetime_test.py +++ b/tests/datetime_test.py @@ -105,6 +105,8 @@ 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) + time_upper = sa.Column(sa.TIME()) @pytest.fixture @@ -228,3 +230,37 @@ 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` 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. Both + spellings resolve to the same colspec, so both must round-trip. + + 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), + 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, 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)