diff --git a/CHANGES.md b/CHANGES.md index 2284b026..22b2f830 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,11 @@ # Changelog ## Unreleased +- Types: Fixed `CLOB`, `NCHAR`, `NVARCHAR`, `DATETIME`, and `DATE` compiling to + type names CrateDB cannot parse. They now map to `STRING`, `CHAR`, `VARCHAR`, + and `TIMESTAMP` respectively, matching their generic lower-case counterparts +- BREAKING: Types: `cast(x, Date)` now renders `CAST(x AS DATE)` instead of + `CAST(x AS TIMESTAMP)`. - Types: Mapped `Numeric` and `DECIMAL` to CrateDB's `NUMERIC`, carrying the declared precision and scale into DDL and casts, where they previously compiled to `LONG` and `DOUBLE` and lost them. Declaring such a column diff --git a/src/sqlalchemy_cratedb/compiler.py b/src/sqlalchemy_cratedb/compiler.py index 3321fa03..3856dc68 100644 --- a/src/sqlalchemy_cratedb/compiler.py +++ b/src/sqlalchemy_cratedb/compiler.py @@ -238,6 +238,15 @@ def visit_unicode(self, type_, **kw): def visit_TEXT(self, type_, **kw): return "STRING" + def visit_CLOB(self, type_, **kw): + return "STRING" + + def visit_NCHAR(self, type_, **kw): + return self.visit_CHAR(type_, **kw) + + def visit_NVARCHAR(self, type_, **kw): + return self.visit_VARCHAR(type_, **kw) + def visit_DECIMAL(self, type_, **kw): return self.visit_NUMERIC(type_, **kw) @@ -264,11 +273,11 @@ def visit_INTEGER(self, type_, **kw): def visit_SMALLINT(self, type_, **kw): return "SHORT" - def visit_datetime(self, type_, **kw): + def visit_DATETIME(self, type_, **kw): return self.visit_TIMESTAMP(type_, **kw) - def visit_date(self, type_, **kw): - return "TIMESTAMP" + def visit_DATE(self, type_, **kw): + return "DATE" if kw.get("cast_target") else "TIMESTAMP" def visit_TIME(self, type_, **kw): """ @@ -327,6 +336,12 @@ def visit_JSONB(self, type_, **kw): class CrateCompiler(compiler.SQLCompiler): + def visit_typeclause(self, typeclause, **kw): + """ + Mark that a type is being rendered as a cast target. + """ + return super().visit_typeclause(typeclause, cast_target=True, **kw) + visit_on_conflict_do_update = PGCompiler.visit_on_conflict_do_update _on_conflict_target = PGCompiler._on_conflict_target diff --git a/tests/create_table_test.py b/tests/create_table_test.py index cddd89af..e0fc7b45 100644 --- a/tests/create_table_test.py +++ b/tests/create_table_test.py @@ -30,6 +30,7 @@ from unittest.mock import MagicMock, patch from crate.client.cursor import Cursor +from sqlalchemy.testing import AssertsCompiledSQL from sqlalchemy_cratedb import Geopoint, ObjectArray, ObjectType from sqlalchemy_cratedb.sa_version import SA_2_0, SA_VERSION @@ -40,9 +41,10 @@ @patch("crate.client.connection.Cursor", FakeCursor) -class SqlAlchemyCreateTableTest(TestCase): +class SqlAlchemyCreateTableTest(TestCase, AssertsCompiledSQL): def setUp(self): self.engine = sa.create_engine("crate://") + self.__dialect__ = self.engine.dialect self.Base = declarative_base() def test_table_basic_types(self): @@ -109,6 +111,40 @@ class Schedule(self.Base): sa.util.immutabledict({}), ) + def test_date_column_is_stored_as_timestamp(self): + """ + A `DATE` column is not storable, so it is emitted as `TIMESTAMP`. + """ + + class Appointment(self.Base): + __tablename__ = "appointment" + name = sa.Column(sa.String, primary_key=True) + day_lower = sa.Column(sa.Date) + day_upper = sa.Column(sa.DATE()) + days = sa.Column(sa.ARRAY(sa.DATE)) + + self.assert_compile( + sa.schema.CreateTable(Appointment.__table__), + "CREATE TABLE appointment (" + "name STRING NOT NULL, " + "day_lower TIMESTAMP, " + "day_upper TIMESTAMP, " + "days ARRAY(TIMESTAMP), " + "PRIMARY KEY (name))", + ) + + def test_date_cast_is_not_rewritten(self): + """ + `CAST(x AS DATE)` keeps its `DATE` target, unlike a `DATE` column. + """ + for type_, expected in ( + (sa.Date, "CAST(ts AS DATE)"), + (sa.DATE, "CAST(ts AS DATE)"), + (sa.ARRAY(sa.DATE), "CAST(ts AS ARRAY(DATE))"), + ): + with self.subTest(type_=type_): + self.assert_compile(sa.cast(sa.column("ts"), type_), expected) + def test_column_obj(self): class DummyTable(self.Base): __tablename__ = "dummy" diff --git a/tests/datetime_test.py b/tests/datetime_test.py index df994e7c..7d7a2f5e 100644 --- a/tests/datetime_test.py +++ b/tests/datetime_test.py @@ -232,6 +232,23 @@ def test_datetime_date(session): 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_date_cast_truncates(session): + """ + Casting to `sa.Date` drops the time of day, server-side. + """ + session.add(FooBar(name="cast", datetime_notz=INPUT_DATETIME_NOTZ)) + session.commit() + session.execute(sa.text("REFRESH TABLE foobar")) + + def hour_of(type_): + expression = sa.extract("hour", sa.cast(FooBar.datetime_notz, type_)) + return session.execute(sa.select(expression)).scalar() + + assert hour_of(sa.DATE) == 0 + assert hour_of(sa.DateTime) == INPUT_DATETIME_NOTZ.hour + + @pytest.mark.skipif(SA_VERSION < SA_1_4, reason="Test case not supported on SQLAlchemy 1.3") def test_datetime_tz_aware_read(session, cratedb_service): """ diff --git a/tests/test_type_compilation.py b/tests/test_type_compilation.py new file mode 100644 index 00000000..851c08c7 --- /dev/null +++ b/tests/test_type_compilation.py @@ -0,0 +1,105 @@ +# -*- coding: utf-8; -*- +# +# Licensed to CRATE Technology GmbH ("Crate") under one or more contributor +# license agreements. See the NOTICE file distributed with this work for +# additional information regarding copyright ownership. Crate licenses +# this file to you under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. You may +# obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# +# However, if you have executed another commercial license agreement +# with Crate these terms will supersede the license and you may use the +# software solely pursuant to the terms of the relevant commercial agreement. + +import pytest +import sqlalchemy as sa + +CORE_TYPE_FACTORIES = { + "BigInteger": lambda: sa.BigInteger, + "Boolean": lambda: sa.Boolean, + "Date": lambda: sa.Date, + "DateTime": lambda: sa.DateTime, + "Double": lambda: sa.Double, + "Enum": lambda: sa.Enum("a", "b", name="enum_a_b"), + "Float": lambda: sa.Float, + "Integer": lambda: sa.Integer, + "Interval": lambda: sa.Interval, + "JSON": lambda: sa.JSON, + "LargeBinary": lambda: sa.LargeBinary, + "Numeric": lambda: sa.Numeric(10, 2), + "PickleType": lambda: sa.PickleType, + "SmallInteger": lambda: sa.SmallInteger, + "String": lambda: sa.String(50), + "Text": lambda: sa.Text, + "Time": lambda: sa.Time, + "Unicode": lambda: sa.Unicode(50), + "UnicodeText": lambda: sa.UnicodeText, + "Uuid": lambda: sa.Uuid, + "ARRAY": lambda: sa.ARRAY(sa.Integer), + "BIGINT": lambda: sa.BIGINT, + "BINARY": lambda: sa.BINARY, + "BLOB": lambda: sa.BLOB, + "BOOLEAN": lambda: sa.BOOLEAN, + "CHAR": lambda: sa.CHAR(5), + "CLOB": lambda: sa.CLOB, + "DATE": lambda: sa.DATE, + "DATETIME": lambda: sa.DATETIME, + "DECIMAL": lambda: sa.DECIMAL(10, 2), + "DOUBLE": lambda: sa.DOUBLE, + "DOUBLE_PRECISION": lambda: sa.DOUBLE_PRECISION, + "FLOAT": lambda: sa.FLOAT, + "INTEGER": lambda: sa.INTEGER, + "NCHAR": lambda: sa.NCHAR(5), + "NUMERIC": lambda: sa.NUMERIC(10, 2), + "NVARCHAR": lambda: sa.NVARCHAR(50), + "REAL": lambda: sa.REAL, + "SMALLINT": lambda: sa.SMALLINT, + "TEXT": lambda: sa.TEXT, + "TIME": lambda: sa.TIME, + "TIMESTAMP": lambda: sa.TIMESTAMP, + "UUID": lambda: sa.UUID, + "VARBINARY": lambda: sa.VARBINARY(50), + "VARCHAR": lambda: sa.VARCHAR(50), +} + +CORE_TYPES = {name: factory() for name, factory in CORE_TYPE_FACTORIES.items() if hasattr(sa, name)} + + +KNOWN_UNSUPPORTED = { + "BINARY": "CrateDB has no binary data type", + "BLOB": "CrateDB has no binary data type", + "LargeBinary": "CrateDB has no binary data type", + "PickleType": "Builds on `sa.LargeBinary`", + "VARBINARY": "CrateDB has no binary data type", +} + + +def _cases(): + for name in sorted(CORE_TYPES): + marks = [] + if name in KNOWN_UNSUPPORTED: + marks.append(pytest.mark.xfail(strict=True, reason=KNOWN_UNSUPPORTED[name])) + yield pytest.param(name, marks=marks) + + +@pytest.mark.parametrize("name", list(_cases())) +def test_core_type_is_creatable(cratedb_service, name): + """CrateDB accepts the DDL the dialect generates for this type.""" + engine = cratedb_service.database.engine + table = sa.Table( + "type_compilation", + sa.MetaData(), + sa.Column("id", sa.Integer, primary_key=True), + sa.Column("d", CORE_TYPES[name]), + ) + table.drop(engine, checkfirst=True) + table.create(engine) + table.drop(engine)