Merge branch 'master' into ignore_for_update

CHANGES.txt

@@ -14,6 +14,10 @@ Unreleased
   certificate details are immanent, like no longer accepting the long
   deprecated ``commonName`` attribute. Instead, going forward, only the
   ``subjectAltName`` attribute will be used.
+- SQLAlchemy: Improve DDL compiler to ignore foreign key and uniqueness
+  constraints
+- DBAPI: Properly raise ``IntegrityError`` exceptions instead of
+  ``ProgrammingError``, when CrateDB raises a ``DuplicateKeyException``.
 
 .. _urllib3 v2.0 migration guide: https://urllib3.readthedocs.io/en/latest/v2-migration-guide.html
 .. _urllib3 v2.0 roadmap: https://urllib3.readthedocs.io/en/stable/v2-roadmap.html

src/crate/client/http.py

@@ -56,6 +56,7 @@
     BlobLocationNotFoundException,
     DigestNotFoundException,
     ProgrammingError,
+    IntegrityError,
 )
 
 
@@ -191,6 +192,18 @@ def _ex_to_message(ex):
 
 
 def _raise_for_status(response):
+    """
+    Properly raise `IntegrityError` exceptions for CrateDB's `DuplicateKeyException` errors.
+    """
+    try:
+        return _raise_for_status_real(response)
+    except ProgrammingError as ex:
+        if "DuplicateKeyException" in ex.message:
+            raise IntegrityError(ex.message, error_trace=ex.error_trace) from ex
+        raise
+
+
+def _raise_for_status_real(response):
     """ make sure that only crate.exceptions are raised that are defined in
     the DB-API specification """
     message = ''

src/crate/client/sqlalchemy/compiler.py

@@ -20,6 +20,7 @@
 # software solely pursuant to the terms of the relevant commercial agreement.
 
 import string
+import warnings
 from collections import defaultdict
 
 import sqlalchemy as sa
@@ -178,6 +179,22 @@ def post_create_table(self, table):
                 ', '.join(sorted(table_opts)))
         return special_options
 
+    def visit_foreign_key_constraint(self, constraint, **kw):
+        """
+        CrateDB does not support foreign key constraints.
+        """
+        warnings.warn("CrateDB does not support foreign key constraints, "
+                      "they will be omitted when generating DDL statements.")
+        return None
+
+    def visit_unique_constraint(self, constraint, **kw):
+        """
+        CrateDB does not support unique key constraints.
+        """
+        warnings.warn("CrateDB does not support unique constraints, "
+                      "they will be omitted when generating DDL statements.")
+        return None
+
 
 class CrateTypeCompiler(compiler.GenericTypeCompiler):
 

src/crate/client/sqlalchemy/tests/__init__.py

@@ -14,7 +14,7 @@
 from .connection_test import SqlAlchemyConnectionTest
 from .dict_test import SqlAlchemyDictTypeTest
 from .datetime_test import SqlAlchemyDateAndDateTimeTest
-from .compiler_test import SqlAlchemyCompilerTest
+from .compiler_test import SqlAlchemyCompilerTest, SqlAlchemyDDLCompilerTest
 from .update_test import SqlAlchemyUpdateTest
 from .match_test import SqlAlchemyMatchTest
 from .bulk_test import SqlAlchemyBulkTest
@@ -36,6 +36,7 @@ def test_suite_unit():
     tests.addTest(makeSuite(SqlAlchemyDictTypeTest))
     tests.addTest(makeSuite(SqlAlchemyDateAndDateTimeTest))
     tests.addTest(makeSuite(SqlAlchemyCompilerTest))
+    tests.addTest(makeSuite(SqlAlchemyDDLCompilerTest))
     tests.addTest(ParametrizedTestCase.parametrize(SqlAlchemyCompilerTest, param={"server_version_info": None}))
     tests.addTest(ParametrizedTestCase.parametrize(SqlAlchemyCompilerTest, param={"server_version_info": (4, 0, 12)}))
     tests.addTest(ParametrizedTestCase.parametrize(SqlAlchemyCompilerTest, param={"server_version_info": (4, 1, 10)}))

src/crate/client/sqlalchemy/tests/compiler_test.py

@@ -18,18 +18,30 @@
 # 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 warnings
 from textwrap import dedent
-from unittest import mock, skipIf
+from unittest import mock, skipIf, TestCase
+from unittest.mock import MagicMock, patch
 
+from crate.client.cursor import Cursor
 from crate.client.sqlalchemy.compiler import crate_before_execute
 
 import sqlalchemy as sa
 from sqlalchemy.sql import text, Update
 
+from crate.testing.util import ExtraAssertions
+
+try:
+    from sqlalchemy.orm import declarative_base
+except ImportError:
+    from sqlalchemy.ext.declarative import declarative_base
+
 from crate.client.sqlalchemy.sa_version import SA_VERSION, SA_1_4, SA_2_0
 from crate.client.sqlalchemy.types import ObjectType
 from crate.client.test_util import ParametrizedTestCase
 
+from crate.testing.settings import crate_host
+
 
 class SqlAlchemyCompilerTest(ParametrizedTestCase):
 
@@ -253,3 +265,150 @@ def test_for_update(self):
         selectable = self.mytable.select().with_for_update()
         statement = str(selectable.compile(bind=self.crate_engine))
         self.assertEqual(statement, "SELECT mytable.name, mytable.data \nFROM mytable")
+
+FakeCursor = MagicMock(name='FakeCursor', spec=Cursor)
+
+
+class CompilerTestCase(TestCase):
+    """
+    A base class for providing mocking infrastructure to validate the DDL compiler.
+    """
+
+    def setUp(self):
+        self.engine = sa.create_engine(f"crate://{crate_host}")
+        self.metadata = sa.MetaData(schema="testdrive")
+        self.session = sa.orm.Session(bind=self.engine)
+        self.setup_mock()
+
+    def setup_mock(self):
+        """
+        Set up a fake cursor, in order to intercept query execution.
+        """
+
+        self.fake_cursor = MagicMock(name="fake_cursor")
+        FakeCursor.return_value = self.fake_cursor
+
+        self.executed_statement = None
+        self.fake_cursor.execute = self.execute_wrapper
+
+    def execute_wrapper(self, query, *args, **kwargs):
+        """
+        Receive the SQL query expression, and store it.
+        """
+        self.executed_statement = query
+        return self.fake_cursor
+
+
+@patch('crate.client.connection.Cursor', FakeCursor)
+class SqlAlchemyDDLCompilerTest(CompilerTestCase, ExtraAssertions):
+    """
+    Verify a few scenarios regarding the DDL compiler.
+    """
+
+    def test_ddl_with_foreign_keys(self):
+        """
+        Verify the CrateDB dialect properly ignores foreign key constraints.
+        """
+
+        Base = declarative_base(metadata=self.metadata)
+
+        class RootStore(Base):
+            """The main store."""
+
+            __tablename__ = "root"
+
+            id = sa.Column(sa.Integer, primary_key=True)
+            name = sa.Column(sa.String)
+
+            items = sa.orm.relationship(
+                "ItemStore",
+                back_populates="root",
+                passive_deletes=True,
+            )
+
+        class ItemStore(Base):
+            """The auxiliary store."""
+
+            __tablename__ = "item"
+
+            id = sa.Column(sa.Integer, primary_key=True)
+            name = sa.Column(sa.String)
+            root_id = sa.Column(
+                sa.Integer,
+                sa.ForeignKey(
+                    f"{RootStore.__tablename__}.id",
+                    ondelete="CASCADE",
+                ),
+            )
+            root = sa.orm.relationship(RootStore, back_populates="items")
+
+        with warnings.catch_warnings(record=True) as w:
+
+            # Cause all warnings to always be triggered.
+            warnings.simplefilter("always")
+
+            # Verify SQL DDL statement.
+            self.metadata.create_all(self.engine, tables=[RootStore.__table__], checkfirst=False)
+            self.assertEqual(self.executed_statement, dedent("""
+                CREATE TABLE testdrive.root (
+                \tid INT NOT NULL, 
+                \tname STRING, 
+                \tPRIMARY KEY (id)
+                )
+    
+            """))  # noqa: W291, W293
+
+            # Verify SQL DDL statement.
+            self.metadata.create_all(self.engine, tables=[ItemStore.__table__], checkfirst=False)
+            self.assertEqual(self.executed_statement, dedent("""
+                CREATE TABLE testdrive.item (
+                \tid INT NOT NULL, 
+                \tname STRING, 
+                \troot_id INT, 
+                \tPRIMARY KEY (id)
+                )
+    
+            """))  # noqa: W291, W293
+
+        # Verify if corresponding warning is emitted.
+        self.assertEqual(len(w), 1)
+        self.assertIsSubclass(w[-1].category, UserWarning)
+        self.assertIn("CrateDB does not support foreign key constraints, "
+                      "they will be omitted when generating DDL statements.", str(w[-1].message))
+
+    def test_ddl_with_unique_key(self):
+        """
+        Verify the CrateDB dialect properly ignores unique key constraints.
+        """
+
+        Base = declarative_base(metadata=self.metadata)
+
+        class FooBar(Base):
+            """The entity."""
+
+            __tablename__ = "foobar"
+
+            id = sa.Column(sa.Integer, primary_key=True)
+            name = sa.Column(sa.String, unique=True)
+
+        with warnings.catch_warnings(record=True) as w:
+
+            # Cause all warnings to always be triggered.
+            warnings.simplefilter("always")
+
+            # Verify SQL DDL statement.
+            self.metadata.create_all(self.engine, tables=[FooBar.__table__], checkfirst=False)
+            self.assertEqual(self.executed_statement, dedent("""
+                CREATE TABLE testdrive.foobar (
+                \tid INT NOT NULL, 
+                \tname STRING, 
+                \tPRIMARY KEY (id)
+                )
+    
+            """))  # noqa: W291, W293
+
+        # Verify if corresponding warning is emitted.
+        self.assertEqual(len(w), 1)
+        self.assertIsSubclass(w[-1].category, UserWarning)
+        self.assertIn("CrateDB does not support unique constraints, "
+                      "they will be omitted when generating DDL statements.", str(w[-1].message))

src/crate/client/test_http.py

@@ -44,8 +44,7 @@
 from setuptools.ssl_support import find_ca_bundle
 
 from .http import Client, CrateJsonEncoder, _get_socket_opts, _remove_certs_for_non_https
-from .exceptions import ConnectionError, ProgrammingError
-
+from .exceptions import ConnectionError, ProgrammingError, IntegrityError
 
 REQUEST = 'crate.client.http.Server.request'
 CA_CERT_PATH = find_ca_bundle()
@@ -91,6 +90,17 @@ def bad_bulk_response():
     return r
 
 
+def duplicate_key_exception():
+    r = fake_response(409, 'Conflict')
+    r.data = json.dumps({
+        "error": {
+            "code": 4091,
+            "message": "DuplicateKeyException[A document with the same primary key exists already]"
+        }
+    }).encode()
+    return r
+
+
 def fail_sometimes(*args, **kwargs):
     if random.randint(1, 100) % 10 == 0:
         raise urllib3.exceptions.MaxRetryError(None, '/_sql', '')
@@ -302,6 +312,18 @@ def test_uuid_serialization(self, request):
         self.assertEqual(data['args'], [str(uid)])
         client.close()
 
+    @patch(REQUEST, fake_request(duplicate_key_exception()))
+    def test_duplicate_key_error(self):
+        """
+        Verify that an `IntegrityError` is raised on duplicate key errors,
+        instead of the more general `ProgrammingError`.
+        """
+        client = Client(servers="localhost:4200")
+        with self.assertRaises(IntegrityError) as cm:
+            client.sql('INSERT INTO testdrive (foo) VALUES (42)')
+        self.assertEqual(cm.exception.message,
+                         "DuplicateKeyException[A document with the same primary key exists already]")
+
 
 @patch(REQUEST, fail_sometimes)
 class ThreadSafeHttpClientTest(TestCase):