Skip to content

Commit

Permalink
SQLAlchemy: Ignore SQL "FOR UPDATE" clause, CrateDB does not support it
Browse files Browse the repository at this point in the history
  • Loading branch information
amotl committed Sep 29, 2023
1 parent 471f026 commit c1ac679
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Unreleased
constraints
- DBAPI: Properly raise ``IntegrityError`` exceptions instead of
``ProgrammingError``, when CrateDB raises a ``DuplicateKeyException``.
- SQLAlchemy: Ignore SQL's ``FOR UPDATE`` clause. Thanks, @surister.

.. _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
Expand Down
7 changes: 7 additions & 0 deletions src/crate/client/sqlalchemy/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,3 +309,10 @@ def limit_clause(self, select, **kw):
Generate OFFSET / LIMIT clause, PostgreSQL-compatible.
"""
return PGCompiler.limit_clause(self, select, **kw)

def for_update_clause(self, select, **kw):
# CrateDB does not support the `INSERT ... FOR UPDATE` clause.
# See https://github.com/crate/crate-python/issues/577.
warnings.warn("CrateDB does not support the 'INSERT ... FOR UPDATE' clause, "
"it will be omitted when generating SQL statements.")
return ''
30 changes: 29 additions & 1 deletion src/crate/client/sqlalchemy/tests/compiler_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
from crate.testing.settings import crate_host


class SqlAlchemyCompilerTest(ParametrizedTestCase):
class SqlAlchemyCompilerTest(ParametrizedTestCase, ExtraAssertions):

def setUp(self):
self.crate_engine = sa.create_engine('crate://')
Expand Down Expand Up @@ -257,6 +257,34 @@ def test_insert_manyvalues(self):
mock.call(mock.ANY, 'INSERT INTO mytable (name) VALUES (?)', ('foo_4', ), None),
])

def test_for_update(self):
"""
Verify the `CrateCompiler.for_update_clause` method to
omit the clause, since CrateDB does not support it.
"""

with warnings.catch_warnings(record=True) as w:

# By default, warnings from a loop will only be emitted once.
# This scenario tests exactly this behaviour, to verify logs
# don't get flooded.
warnings.simplefilter("once")

selectable = self.mytable.select().with_for_update()
_ = str(selectable.compile(bind=self.crate_engine))

selectable = self.mytable.select().with_for_update()
statement = str(selectable.compile(bind=self.crate_engine))

# Verify SQL statement.
self.assertEqual(statement, "SELECT mytable.name, mytable.data \nFROM mytable")

# Verify if corresponding warning is emitted, once.
self.assertEqual(len(w), 1)
self.assertIsSubclass(w[-1].category, UserWarning)
self.assertIn("CrateDB does not support the 'INSERT ... FOR UPDATE' clause, "
"it will be omitted when generating SQL statements.", str(w[-1].message))


FakeCursor = MagicMock(name='FakeCursor', spec=Cursor)

Expand Down

0 comments on commit c1ac679

Please sign in to comment.