fix optimization for bulk updates

- don't optimize if parameters are not supplied yet
- check the key type before binding parameters
- add test cases for bulk updates

This closes #224

Author: mxm

CHANGES.txt

@@ -5,6 +5,12 @@ Changes for crate
 Unreleased
 ==========
 
+2017/05/17 0.19.3
+=================
+
+ - Fix bulk updates which were broken due to query rewrites.
+
+
 2017/04/28 0.19.2
 =================
 

README.rst

@@ -76,7 +76,7 @@ Looking for more help?
 .. _contribution docs: CONTRIBUTING.rst
 .. _Crate.io: http://crate.io/
 .. _CrateDB: https://github.com/crate/crate
-.. _DB API 2.0: http://www.python.org/dev/peps/pep-0249/>
+.. _DB API 2.0: http://www.python.org/dev/peps/pep-0249/
 .. _developer docs: DEVELOP.rst
 .. _paid support: https://crate.io/pricing/
 .. _pip: https://pypi.python.org/pypi/pip

src/crate/client/sqlalchemy/compiler.py

@@ -44,9 +44,10 @@ def rewrite_update(clauseelement, multiparams, params):
     The update statement is only rewritten if an item of the MutableDict was
     changed.
     """
-
     newmultiparams = []
     _multiparams = multiparams[0]
+    if len(_multiparams) == 0:
+        return clauseelement, multiparams, params
     for _params in _multiparams:
         newparams = {}
         for key, val in _params.items():
@@ -316,7 +317,7 @@ def visit_update(self, update_stmt, **kw):
             set_clauses.append(clause)
 
         for k, v in update_stmt.parameters.items():
-            if '[' in k:
+            if isinstance(k, str) and '[' in k:
                 bindparam = sa.sql.bindparam(k, v)
                 set_clauses.append(k + ' = ' + self.process(bindparam))
 

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

@@ -21,6 +21,7 @@
 
 from __future__ import absolute_import
 from unittest import TestCase
+
 from crate.client.sqlalchemy.compiler import crate_before_execute
 
 import sqlalchemy as sa
@@ -55,3 +56,16 @@ def test_crate_update_rewritten(self):
         )
 
         assert hasattr(clauseelement, '_crate_specific') is True
+
+    def test_bulk_update_on_builtin_type(self):
+        """
+        The "before_execute" hook in the compiler doesn't get
+        access to the parameters in case of a bulk update. It
+        should not try to optimize any parameters.
+        """
+        data = ({},)
+        clauseelement, multiparams, params = crate_before_execute(
+            self.crate_engine, self.update, data, None
+        )
+
+        assert hasattr(clauseelement, '_crate_specific') is False

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

@@ -21,6 +21,8 @@
 
 from unittest import TestCase
 from datetime import datetime
+
+from crate.client.sqlalchemy.types import Object
 from mock import patch, MagicMock
 
 import sqlalchemy as sa
@@ -40,13 +42,14 @@ class SqlAlchemyUpdateTest(TestCase):
 
     def setUp(self):
         self.engine = sa.create_engine('crate://')
-        Base = declarative_base(bind=self.engine)
+        self.base = declarative_base(bind=self.engine)
 
-        class Character(Base):
+        class Character(self.base):
             __tablename__ = 'characters'
 
             name = sa.Column(sa.String, primary_key=True)
             age = sa.Column(sa.Integer)
+            obj = sa.Column(Object)
             ts = sa.Column(sa.DateTime, onupdate=datetime.utcnow)
 
         self.character = Character
@@ -79,3 +82,31 @@ def test_onupdate_is_triggered(self):
         self.assertTrue(isinstance(dt, datetime))
         self.assertTrue(dt > now)
         self.assertEqual('Arthur', args[2])
+
+    @patch('crate.client.connection.Cursor', FakeCursor)
+    def test_bulk_update(self):
+        """
+            Checks whether bulk updates work correctly
+            on native types and Crate types.
+        """
+        before_update_time = datetime.utcnow()
+
+        self.session.query(self.character).update({
+            # change everyone's name to Julia
+            self.character.name: 'Julia',
+            self.character.obj: {'favorite_book' : 'Romeo & Juliet'}
+        })
+
+        self.session.commit()
+
+        expected_stmt = ("UPDATE characters SET "
+                         "name = ?, obj = ?, ts = ?")
+        args, kwargs = fake_cursor.execute.call_args
+        stmt = args[0]
+        args = args[1]
+        self.assertEqual(expected_stmt, stmt)
+        self.assertEqual('Julia', args[0])
+        self.assertEqual({'favorite_book': 'Romeo & Juliet'}, args[1])
+        dt = datetime.strptime(args[2], '%Y-%m-%dT%H:%M:%S.%fZ')
+        self.assertTrue(isinstance(dt, datetime))
+        self.assertTrue(dt > before_update_time)