Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/sqlalchemy_cratedb/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
22 changes: 21 additions & 1 deletion src/sqlalchemy_cratedb/dialect.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Comment thread
florinutz marked this conversation as resolved.
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:
Expand Down
31 changes: 31 additions & 0 deletions tests/create_table_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
__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"
Expand Down
36 changes: 36 additions & 0 deletions tests/datetime_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)