From 73f6330a238cc53a2266806f2507dfd6babac7f3 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Mon, 30 Nov 2020 10:48:31 +0300 Subject: [PATCH 01/23] fix(dbapi): autocommit enabling fails if no transactions begun --- google/cloud/spanner_dbapi/connection.py | 8 +++- tests/unit/spanner_dbapi/test_connection.py | 48 +++++++++++++++++---- 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/google/cloud/spanner_dbapi/connection.py b/google/cloud/spanner_dbapi/connection.py index a397028287..981520d4e6 100644 --- a/google/cloud/spanner_dbapi/connection.py +++ b/google/cloud/spanner_dbapi/connection.py @@ -82,7 +82,13 @@ def autocommit(self, value): :type value: bool :param value: New autocommit mode state. """ - if value and not self._autocommit: + if ( + value + and not self._autocommit + and self._transaction + and not self._transaction.committed + and not self._transaction.rolled_back + ): self.commit() self._autocommit = value diff --git a/tests/unit/spanner_dbapi/test_connection.py b/tests/unit/spanner_dbapi/test_connection.py index 213eb24d84..159d505e90 100644 --- a/tests/unit/spanner_dbapi/test_connection.py +++ b/tests/unit/spanner_dbapi/test_connection.py @@ -51,25 +51,57 @@ def _make_connection(self): database = instance.database(self.DATABASE) return Connection(instance, database) - @unittest.skipIf(sys.version_info[0] < 3, "Python 2 patching is outdated") - def test_property_autocommit_setter(self): - from google.cloud.spanner_dbapi import Connection - - connection = Connection(self.INSTANCE, self.DATABASE) + def test_autocommit_setter_transaction_not_started(self): + connection = self._make_connection() with mock.patch( "google.cloud.spanner_dbapi.connection.Connection.commit" ) as mock_commit: connection.autocommit = True - mock_commit.assert_called_once_with() - self.assertEqual(connection._autocommit, True) + mock_commit.assert_not_called() + self.assertTrue(connection._autocommit) with mock.patch( "google.cloud.spanner_dbapi.connection.Connection.commit" ) as mock_commit: connection.autocommit = False mock_commit.assert_not_called() - self.assertEqual(connection._autocommit, False) + self.assertFalse(connection._autocommit) + + def test_autocommit_setter_transaction_started(self): + connection = self._make_connection() + + with mock.patch( + "google.cloud.spanner_dbapi.connection.Connection.commit" + ) as mock_commit: + connection._transaction = mock.Mock(committed=False, rolled_back=False) + + connection.autocommit = True + mock_commit.assert_called_once() + self.assertTrue(connection._autocommit) + + def test_autocommit_setter_transaction_started_commited_rolled_back(self): + connection = self._make_connection() + + with mock.patch( + "google.cloud.spanner_dbapi.connection.Connection.commit" + ) as mock_commit: + connection._transaction = mock.Mock(committed=True, rolled_back=False) + + connection.autocommit = True + mock_commit.assert_not_called() + self.assertTrue(connection._autocommit) + + connection.autocommit = False + + with mock.patch( + "google.cloud.spanner_dbapi.connection.Connection.commit" + ) as mock_commit: + connection._transaction = mock.Mock(committed=False, rolled_back=True) + + connection.autocommit = True + mock_commit.assert_not_called() + self.assertTrue(connection._autocommit) def test_property_database(self): from google.cloud.spanner_v1.database import Database From 632bc2012353c9d1796f5c015bd4d4b9713df392 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Mon, 30 Nov 2020 11:32:13 +0300 Subject: [PATCH 02/23] remove unused import --- tests/unit/spanner_dbapi/test_connection.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/spanner_dbapi/test_connection.py b/tests/unit/spanner_dbapi/test_connection.py index 159d505e90..c09451597e 100644 --- a/tests/unit/spanner_dbapi/test_connection.py +++ b/tests/unit/spanner_dbapi/test_connection.py @@ -15,7 +15,6 @@ """Cloud Spanner DB-API Connection class unit tests.""" import mock -import sys import unittest import warnings From 1f8e8646dda4beae6df72cf31b293e11e4f409ab Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Thu, 3 Dec 2020 12:01:46 +0300 Subject: [PATCH 03/23] don't calculate checksums in autocommit mode --- google/cloud/spanner_dbapi/cursor.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/google/cloud/spanner_dbapi/cursor.py b/google/cloud/spanner_dbapi/cursor.py index e2667f0599..d0142e7651 100644 --- a/google/cloud/spanner_dbapi/cursor.py +++ b/google/cloud/spanner_dbapi/cursor.py @@ -220,7 +220,8 @@ def fetchone(self): try: res = next(self) - self._checksum.consume_result(res) + if not self.connection.autocommit: + self._checksum.consume_result(res) return res except StopIteration: return @@ -237,7 +238,8 @@ def fetchall(self): res = [] try: for row in self: - self._checksum.consume_result(row) + if not self.connection.autocommit: + self._checksum.consume_result(row) res.append(row) except Aborted: self._connection.retry_transaction() @@ -265,7 +267,8 @@ def fetchmany(self, size=None): for i in range(size): try: res = next(self) - self._checksum.consume_result(res) + if not self.connection.autocommit: + self._checksum.consume_result(res) items.append(res) except StopIteration: break From fcc20aea17180db0e43d109dead996d0289295e1 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Thu, 3 Dec 2020 13:27:52 +0300 Subject: [PATCH 04/23] try using dummy WHERE clause --- google/cloud/spanner_dbapi/parse_utils.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/google/cloud/spanner_dbapi/parse_utils.py b/google/cloud/spanner_dbapi/parse_utils.py index 8848233d45..83f99af12d 100644 --- a/google/cloud/spanner_dbapi/parse_utils.py +++ b/google/cloud/spanner_dbapi/parse_utils.py @@ -533,9 +533,7 @@ def ensure_where_clause(sql): if any(isinstance(token, sqlparse.sql.Where) for token in sqlparse.parse(sql)[0]): return sql - raise ProgrammingError( - "Cloud Spanner requires a WHERE clause when executing DELETE or UPDATE query" - ) + return sql + " WHERE 1=1" def escape_name(name): From 73f91ef8097077386daec50ffa3cc5b5155526cc Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Thu, 3 Dec 2020 13:31:46 +0300 Subject: [PATCH 05/23] revert where clause --- google/cloud/spanner_dbapi/parse_utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/google/cloud/spanner_dbapi/parse_utils.py b/google/cloud/spanner_dbapi/parse_utils.py index 83f99af12d..8848233d45 100644 --- a/google/cloud/spanner_dbapi/parse_utils.py +++ b/google/cloud/spanner_dbapi/parse_utils.py @@ -533,7 +533,9 @@ def ensure_where_clause(sql): if any(isinstance(token, sqlparse.sql.Where) for token in sqlparse.parse(sql)[0]): return sql - return sql + " WHERE 1=1" + raise ProgrammingError( + "Cloud Spanner requires a WHERE clause when executing DELETE or UPDATE query" + ) def escape_name(name): From df9c35f8c7ad9d85272c9c49ee201fe7e4b2fc31 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Fri, 4 Dec 2020 10:33:44 +0300 Subject: [PATCH 06/23] unveil error --- google/cloud/spanner_dbapi/cursor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google/cloud/spanner_dbapi/cursor.py b/google/cloud/spanner_dbapi/cursor.py index d0142e7651..61c425a47e 100644 --- a/google/cloud/spanner_dbapi/cursor.py +++ b/google/cloud/spanner_dbapi/cursor.py @@ -192,8 +192,8 @@ def execute(self, sql, args=None): ) except (AlreadyExists, FailedPrecondition) as e: raise IntegrityError(e.details if hasattr(e, "details") else e) - except InvalidArgument as e: - raise ProgrammingError(e.details if hasattr(e, "details") else e) + # except InvalidArgument as e: + # raise ProgrammingError(e.details if hasattr(e, "details") else e) except InternalServerError as e: raise OperationalError(e.details if hasattr(e, "details") else e) From 5f04934ecd74ae789f45b5da69972c5001afa6a6 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Fri, 4 Dec 2020 12:10:29 +0300 Subject: [PATCH 07/23] fix where clauses --- google/cloud/spanner_dbapi/cursor.py | 7 +++++-- google/cloud/spanner_dbapi/parse_utils.py | 8 ++------ tests/unit/spanner_dbapi/test_cursor.py | 2 +- tests/unit/spanner_dbapi/test_parse_utils.py | 4 +--- 4 files changed, 9 insertions(+), 12 deletions(-) diff --git a/google/cloud/spanner_dbapi/cursor.py b/google/cloud/spanner_dbapi/cursor.py index 61c425a47e..ac22502916 100644 --- a/google/cloud/spanner_dbapi/cursor.py +++ b/google/cloud/spanner_dbapi/cursor.py @@ -171,6 +171,9 @@ def execute(self, sql, args=None): self.connection.run_prior_DDL_statements() if not self.connection.autocommit: + if classification == parse_utils.STMT_UPDATING: + sql = parse_utils.ensure_where_clause(sql) + sql, params = sql_pyformat_args_to_spanner(sql, args) statement = Statement( @@ -192,8 +195,8 @@ def execute(self, sql, args=None): ) except (AlreadyExists, FailedPrecondition) as e: raise IntegrityError(e.details if hasattr(e, "details") else e) - # except InvalidArgument as e: - # raise ProgrammingError(e.details if hasattr(e, "details") else e) + except InvalidArgument as e: + raise ProgrammingError(e.details if hasattr(e, "details") else e) except InternalServerError as e: raise OperationalError(e.details if hasattr(e, "details") else e) diff --git a/google/cloud/spanner_dbapi/parse_utils.py b/google/cloud/spanner_dbapi/parse_utils.py index 8848233d45..d3dd98dda6 100644 --- a/google/cloud/spanner_dbapi/parse_utils.py +++ b/google/cloud/spanner_dbapi/parse_utils.py @@ -523,19 +523,15 @@ def get_param_types(params): def ensure_where_clause(sql): """ Cloud Spanner requires a WHERE clause on UPDATE and DELETE statements. - Raise an error, if the given sql doesn't include it. + Add a dummy WHERE clause if non detected. :type sql: `str` :param sql: SQL code to check. - - :raises: :class:`ProgrammingError` if the given sql doesn't include a WHERE clause. """ if any(isinstance(token, sqlparse.sql.Where) for token in sqlparse.parse(sql)[0]): return sql - raise ProgrammingError( - "Cloud Spanner requires a WHERE clause when executing DELETE or UPDATE query" - ) + return sql + " WHERE 1=1" def escape_name(name): diff --git a/tests/unit/spanner_dbapi/test_cursor.py b/tests/unit/spanner_dbapi/test_cursor.py index 43fc077abe..ea9850e676 100644 --- a/tests/unit/spanner_dbapi/test_cursor.py +++ b/tests/unit/spanner_dbapi/test_cursor.py @@ -126,7 +126,7 @@ def test_execute_attribute_error(self): cursor = self._make_one(connection) with self.assertRaises(AttributeError): - cursor.execute(sql="") + cursor.execute(sql="SELECT 1") def test_execute_autocommit_off(self): from google.cloud.spanner_dbapi.utils import PeekIterator diff --git a/tests/unit/spanner_dbapi/test_parse_utils.py b/tests/unit/spanner_dbapi/test_parse_utils.py index 6d89a8a46a..3713ac11a8 100644 --- a/tests/unit/spanner_dbapi/test_parse_utils.py +++ b/tests/unit/spanner_dbapi/test_parse_utils.py @@ -391,7 +391,6 @@ def test_get_param_types_none(self): @unittest.skipIf(skip_condition, skip_message) def test_ensure_where_clause(self): - from google.cloud.spanner_dbapi.exceptions import ProgrammingError from google.cloud.spanner_dbapi.parse_utils import ensure_where_clause cases = ( @@ -409,8 +408,7 @@ def test_ensure_where_clause(self): for sql in err_cases: with self.subTest(sql=sql): - with self.assertRaises(ProgrammingError): - ensure_where_clause(sql) + self.assertEqual(ensure_where_clause(sql), sql + " WHERE 1=1") @unittest.skipIf(skip_condition, skip_message) def test_escape_name(self): From 6a0ff47858e0f81f1801de3a1bd9135218cfdffc Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Mon, 7 Dec 2020 16:38:26 +0300 Subject: [PATCH 08/23] add print --- google/cloud/spanner_dbapi/cursor.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/google/cloud/spanner_dbapi/cursor.py b/google/cloud/spanner_dbapi/cursor.py index ac22502916..74896be68f 100644 --- a/google/cloud/spanner_dbapi/cursor.py +++ b/google/cloud/spanner_dbapi/cursor.py @@ -156,6 +156,8 @@ def execute(self, sql, args=None): self._raise_if_closed() + print("my sql log: ", sql) + print(args) self._result_set = None # Classify whether this is a read-only SQL statement. From bb75c34b10e5c6c367ea239d51d74af4a3ca3325 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Tue, 8 Dec 2020 11:10:24 +0300 Subject: [PATCH 09/23] don't log --- google/cloud/spanner_dbapi/cursor.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/google/cloud/spanner_dbapi/cursor.py b/google/cloud/spanner_dbapi/cursor.py index 74896be68f..ac22502916 100644 --- a/google/cloud/spanner_dbapi/cursor.py +++ b/google/cloud/spanner_dbapi/cursor.py @@ -156,8 +156,6 @@ def execute(self, sql, args=None): self._raise_if_closed() - print("my sql log: ", sql) - print(args) self._result_set = None # Classify whether this is a read-only SQL statement. From 1527a0edca9197d62437af49dbc4d80e22c689a7 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Mon, 14 Dec 2020 14:47:36 +0300 Subject: [PATCH 10/23] print failed exceptions --- google/cloud/spanner_dbapi/cursor.py | 1 + 1 file changed, 1 insertion(+) diff --git a/google/cloud/spanner_dbapi/cursor.py b/google/cloud/spanner_dbapi/cursor.py index ac22502916..7d6d017209 100644 --- a/google/cloud/spanner_dbapi/cursor.py +++ b/google/cloud/spanner_dbapi/cursor.py @@ -194,6 +194,7 @@ def execute(self, sql, args=None): self._do_execute_update, sql, args or None ) except (AlreadyExists, FailedPrecondition) as e: + print(sql, args) raise IntegrityError(e.details if hasattr(e, "details") else e) except InvalidArgument as e: raise ProgrammingError(e.details if hasattr(e, "details") else e) From dd4e74e9c9a4c3fc850f6590eb4e8291dbb66424 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Mon, 14 Dec 2020 16:48:36 +0300 Subject: [PATCH 11/23] don't print --- google/cloud/spanner_dbapi/cursor.py | 1 - 1 file changed, 1 deletion(-) diff --git a/google/cloud/spanner_dbapi/cursor.py b/google/cloud/spanner_dbapi/cursor.py index cc8d6ddb83..d86a591aaf 100644 --- a/google/cloud/spanner_dbapi/cursor.py +++ b/google/cloud/spanner_dbapi/cursor.py @@ -195,7 +195,6 @@ def execute(self, sql, args=None): self._do_execute_update, sql, args or None ) except (AlreadyExists, FailedPrecondition) as e: - print(sql, args) raise IntegrityError(e.details if hasattr(e, "details") else e) except InvalidArgument as e: raise ProgrammingError(e.details if hasattr(e, "details") else e) From 092c334b0ee657261d50976b38e4783978533690 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Tue, 15 Dec 2020 12:08:24 +0300 Subject: [PATCH 12/23] separate insert statements --- google/cloud/spanner_dbapi/_helpers.py | 2 +- google/cloud/spanner_dbapi/connection.py | 19 +++++++++++++++++++ google/cloud/spanner_dbapi/cursor.py | 8 ++++++-- tests/unit/spanner_dbapi/test_connection.py | 12 ++++++------ tests/unit/spanner_dbapi/test_cursor.py | 4 ++-- 5 files changed, 34 insertions(+), 11 deletions(-) diff --git a/google/cloud/spanner_dbapi/_helpers.py b/google/cloud/spanner_dbapi/_helpers.py index 2fcdd59137..f7679d4420 100644 --- a/google/cloud/spanner_dbapi/_helpers.py +++ b/google/cloud/spanner_dbapi/_helpers.py @@ -59,7 +59,7 @@ def _execute_insert_heterogenous(transaction, sql_params_list): for sql, params in sql_params_list: sql, params = sql_pyformat_args_to_spanner(sql, params) param_types = get_param_types(params) - transaction.execute_update(sql, params=params, param_types=param_types) + return transaction.execute_update(sql, params=params, param_types=param_types) def _execute_insert_homogenous(transaction, parts): diff --git a/google/cloud/spanner_dbapi/connection.py b/google/cloud/spanner_dbapi/connection.py index 981520d4e6..f5ea5ed2c2 100644 --- a/google/cloud/spanner_dbapi/connection.py +++ b/google/cloud/spanner_dbapi/connection.py @@ -22,6 +22,9 @@ from google.cloud import spanner_v1 as spanner from google.cloud.spanner_v1.session import _get_retry_delay +from google.cloud.spanner_dbapi._helpers import _execute_insert_heterogenous +from google.cloud.spanner_dbapi._helpers import _execute_insert_homogenous +from google.cloud.spanner_dbapi._helpers import parse_insert from google.cloud.spanner_dbapi.checksum import _compare_checksums from google.cloud.spanner_dbapi.checksum import ResultsChecksum from google.cloud.spanner_dbapi.cursor import Cursor @@ -297,6 +300,22 @@ def run_statement(self, statement, retried=False): if not retried: self._statements.append(statement) + if statement.is_insert: + parts = parse_insert(statement.sql, statement.params) + + if parts.get("homogenous"): + return ( + _execute_insert_homogenous(transaction, parts), + ResultsChecksum() if retried else statement.checksum, + ) + else: + return ( + _execute_insert_heterogenous( + transaction, parts.get("sql_params_list"), + ), + ResultsChecksum() if retried else statement.checksum, + ) + return ( transaction.execute_sql( statement.sql, statement.params, param_types=statement.param_types, diff --git a/google/cloud/spanner_dbapi/cursor.py b/google/cloud/spanner_dbapi/cursor.py index d86a591aaf..0980480fd3 100644 --- a/google/cloud/spanner_dbapi/cursor.py +++ b/google/cloud/spanner_dbapi/cursor.py @@ -42,7 +42,7 @@ _UNSET_COUNT = -1 ColumnDetails = namedtuple("column_details", ["null_ok", "spanner_type"]) -Statement = namedtuple("Statement", "sql, params, param_types, checksum") +Statement = namedtuple("Statement", "sql, params, param_types, checksum, is_insert") class Cursor(object): @@ -178,7 +178,11 @@ def execute(self, sql, args=None): sql, params = sql_pyformat_args_to_spanner(sql, args) statement = Statement( - sql, params, get_param_types(params), ResultsChecksum(), + sql, + params, + get_param_types(params), + ResultsChecksum(), + classification == parse_utils.STMT_INSERT, ) (self._result_set, self._checksum,) = self.connection.run_statement( statement diff --git a/tests/unit/spanner_dbapi/test_connection.py b/tests/unit/spanner_dbapi/test_connection.py index c09451597e..2e2fccce5c 100644 --- a/tests/unit/spanner_dbapi/test_connection.py +++ b/tests/unit/spanner_dbapi/test_connection.py @@ -347,7 +347,7 @@ def test_run_statement_remember_statements(self): connection = self._make_connection() - statement = Statement(sql, params, param_types, ResultsChecksum(),) + statement = Statement(sql, params, param_types, ResultsChecksum(), False) with mock.patch( "google.cloud.spanner_dbapi.connection.Connection.transaction_checkout" ): @@ -369,7 +369,7 @@ def test_run_statement_dont_remember_retried_statements(self): connection = self._make_connection() - statement = Statement(sql, params, param_types, ResultsChecksum(),) + statement = Statement(sql, params, param_types, ResultsChecksum(), False) with mock.patch( "google.cloud.spanner_dbapi.connection.Connection.transaction_checkout" ): @@ -421,7 +421,7 @@ def test_retry_transaction(self): checksum.consume_result(row) retried_checkum = ResultsChecksum() - statement = Statement("SELECT 1", [], {}, checksum,) + statement = Statement("SELECT 1", [], {}, checksum, False) connection._statements.append(statement) with mock.patch( @@ -454,7 +454,7 @@ def test_retry_transaction_checksum_mismatch(self): checksum.consume_result(row) retried_checkum = ResultsChecksum() - statement = Statement("SELECT 1", [], {}, checksum,) + statement = Statement("SELECT 1", [], {}, checksum, False) connection._statements.append(statement) with mock.patch( @@ -484,7 +484,7 @@ def test_commit_retry_aborted_statements(self): cursor._checksum = ResultsChecksum() cursor._checksum.consume_result(row) - statement = Statement("SELECT 1", [], {}, cursor._checksum,) + statement = Statement("SELECT 1", [], {}, cursor._checksum, False) connection._statements.append(statement) connection._transaction = mock.Mock() @@ -538,7 +538,7 @@ def test_retry_aborted_retry(self): cursor._checksum = ResultsChecksum() cursor._checksum.consume_result(row) - statement = Statement("SELECT 1", [], {}, cursor._checksum,) + statement = Statement("SELECT 1", [], {}, cursor._checksum, False) connection._statements.append(statement) metadata_mock = mock.Mock() diff --git a/tests/unit/spanner_dbapi/test_cursor.py b/tests/unit/spanner_dbapi/test_cursor.py index e2e0ffcd9a..9f0510c4ab 100644 --- a/tests/unit/spanner_dbapi/test_cursor.py +++ b/tests/unit/spanner_dbapi/test_cursor.py @@ -531,7 +531,7 @@ def test_fetchone_retry_aborted_statements(self): cursor._checksum = ResultsChecksum() cursor._checksum.consume_result(row) - statement = Statement("SELECT 1", [], {}, cursor._checksum,) + statement = Statement("SELECT 1", [], {}, cursor._checksum, False) connection._statements.append(statement) with mock.patch( @@ -570,7 +570,7 @@ def test_fetchone_retry_aborted_statements_checksums_mismatch(self): cursor._checksum = ResultsChecksum() cursor._checksum.consume_result(row) - statement = Statement("SELECT 1", [], {}, cursor._checksum,) + statement = Statement("SELECT 1", [], {}, cursor._checksum, False) connection._statements.append(statement) with mock.patch( From 5d997fddba339b73dab9de78611c84c1b9cd2663 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Tue, 15 Dec 2020 12:24:31 +0300 Subject: [PATCH 13/23] don't return --- google/cloud/spanner_dbapi/_helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/cloud/spanner_dbapi/_helpers.py b/google/cloud/spanner_dbapi/_helpers.py index f7679d4420..2fcdd59137 100644 --- a/google/cloud/spanner_dbapi/_helpers.py +++ b/google/cloud/spanner_dbapi/_helpers.py @@ -59,7 +59,7 @@ def _execute_insert_heterogenous(transaction, sql_params_list): for sql, params in sql_params_list: sql, params = sql_pyformat_args_to_spanner(sql, params) param_types = get_param_types(params) - return transaction.execute_update(sql, params=params, param_types=param_types) + transaction.execute_update(sql, params=params, param_types=param_types) def _execute_insert_homogenous(transaction, parts): From 9e07ec83adac59dcf12e7d4e8af92dc86aca2cee Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Tue, 15 Dec 2020 12:41:23 +0300 Subject: [PATCH 14/23] re-run --- google/cloud/spanner_dbapi/connection.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/google/cloud/spanner_dbapi/connection.py b/google/cloud/spanner_dbapi/connection.py index f5ea5ed2c2..a210d77579 100644 --- a/google/cloud/spanner_dbapi/connection.py +++ b/google/cloud/spanner_dbapi/connection.py @@ -304,15 +304,17 @@ def run_statement(self, statement, retried=False): parts = parse_insert(statement.sql, statement.params) if parts.get("homogenous"): + _execute_insert_homogenous(transaction, parts) return ( - _execute_insert_homogenous(transaction, parts), + iter(()), ResultsChecksum() if retried else statement.checksum, ) else: + _execute_insert_heterogenous( + transaction, parts.get("sql_params_list"), + ) return ( - _execute_insert_heterogenous( - transaction, parts.get("sql_params_list"), - ), + iter(()), ResultsChecksum() if retried else statement.checksum, ) From 914930febb78712e053beda30de86be56a80ff41 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Tue, 15 Dec 2020 13:05:49 +0300 Subject: [PATCH 15/23] don't pyformat insert args --- google/cloud/spanner_dbapi/cursor.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/google/cloud/spanner_dbapi/cursor.py b/google/cloud/spanner_dbapi/cursor.py index 0980480fd3..80af6b6d3c 100644 --- a/google/cloud/spanner_dbapi/cursor.py +++ b/google/cloud/spanner_dbapi/cursor.py @@ -175,7 +175,8 @@ def execute(self, sql, args=None): if classification == parse_utils.STMT_UPDATING: sql = parse_utils.ensure_where_clause(sql) - sql, params = sql_pyformat_args_to_spanner(sql, args) + if classification != parse_utils.STMT_INSERT: + sql, params = sql_pyformat_args_to_spanner(sql, args) statement = Statement( sql, From 824d3ff2e2115838d9e0b9c4a893d4c9167001fd Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Tue, 15 Dec 2020 13:08:13 +0300 Subject: [PATCH 16/23] args --- google/cloud/spanner_dbapi/cursor.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/google/cloud/spanner_dbapi/cursor.py b/google/cloud/spanner_dbapi/cursor.py index 80af6b6d3c..eb8305daa0 100644 --- a/google/cloud/spanner_dbapi/cursor.py +++ b/google/cloud/spanner_dbapi/cursor.py @@ -176,12 +176,12 @@ def execute(self, sql, args=None): sql = parse_utils.ensure_where_clause(sql) if classification != parse_utils.STMT_INSERT: - sql, params = sql_pyformat_args_to_spanner(sql, args) + sql, args = sql_pyformat_args_to_spanner(sql, args) statement = Statement( sql, - params, - get_param_types(params), + args, + get_param_types(args), ResultsChecksum(), classification == parse_utils.STMT_INSERT, ) From 18e815f768ead5240d431d2dc42d86b1f2001f2e Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Tue, 15 Dec 2020 13:14:17 +0300 Subject: [PATCH 17/23] re-run --- google/cloud/spanner_dbapi/cursor.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/google/cloud/spanner_dbapi/cursor.py b/google/cloud/spanner_dbapi/cursor.py index eb8305daa0..047fb50874 100644 --- a/google/cloud/spanner_dbapi/cursor.py +++ b/google/cloud/spanner_dbapi/cursor.py @@ -181,7 +181,9 @@ def execute(self, sql, args=None): statement = Statement( sql, args, - get_param_types(args), + {} + if classification != parse_utils.STMT_INSERT + else get_param_types(args), ResultsChecksum(), classification == parse_utils.STMT_INSERT, ) From 7ddd0b58ace5a5ae52d3d5c48894a0946cd89c83 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Tue, 15 Dec 2020 13:21:41 +0300 Subject: [PATCH 18/23] fix --- google/cloud/spanner_dbapi/cursor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google/cloud/spanner_dbapi/cursor.py b/google/cloud/spanner_dbapi/cursor.py index 047fb50874..036158522f 100644 --- a/google/cloud/spanner_dbapi/cursor.py +++ b/google/cloud/spanner_dbapi/cursor.py @@ -181,9 +181,9 @@ def execute(self, sql, args=None): statement = Statement( sql, args, - {} + get_param_types(args) if classification != parse_utils.STMT_INSERT - else get_param_types(args), + else {}, ResultsChecksum(), classification == parse_utils.STMT_INSERT, ) From e40ccc60c0ddd969cbf63ad138c499de3b77499b Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Mon, 21 Dec 2020 14:06:38 +0300 Subject: [PATCH 19/23] fix error in transactions.tests.NonAutocommitTests.test_orm_query_without_autocommit --- google/cloud/spanner_dbapi/connection.py | 2 +- tests/unit/spanner_dbapi/test_connection.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/google/cloud/spanner_dbapi/connection.py b/google/cloud/spanner_dbapi/connection.py index a210d77579..2540ec0fcb 100644 --- a/google/cloud/spanner_dbapi/connection.py +++ b/google/cloud/spanner_dbapi/connection.py @@ -244,7 +244,7 @@ def commit(self): """ if self._autocommit: warnings.warn(AUTOCOMMIT_MODE_WARNING, UserWarning, stacklevel=2) - elif self._transaction: + elif self._transaction and not self._transaction.rolled_back: try: self._transaction.commit() self._release_session() diff --git a/tests/unit/spanner_dbapi/test_connection.py b/tests/unit/spanner_dbapi/test_connection.py index 2e2fccce5c..f80469d03d 100644 --- a/tests/unit/spanner_dbapi/test_connection.py +++ b/tests/unit/spanner_dbapi/test_connection.py @@ -197,7 +197,7 @@ def test_commit(self, mock_warn): connection.commit() mock_release.assert_not_called() - connection._transaction = mock_transaction = mock.MagicMock() + connection._transaction = mock_transaction = mock.MagicMock(rolled_back=False) mock_transaction.commit = mock_commit = mock.MagicMock() with mock.patch( @@ -383,7 +383,7 @@ def test_clear_statements_on_commit(self): cleared, when the transaction is commited. """ connection = self._make_connection() - connection._transaction = mock.Mock() + connection._transaction = mock.Mock(rolled_back=False) connection._statements = [{}, {}] self.assertEqual(len(connection._statements), 2) @@ -486,7 +486,7 @@ def test_commit_retry_aborted_statements(self): statement = Statement("SELECT 1", [], {}, cursor._checksum, False) connection._statements.append(statement) - connection._transaction = mock.Mock() + connection._transaction = mock.Mock(rolled_back=False) with mock.patch.object( connection._transaction, "commit", side_effect=(Aborted("Aborted"), None), From 2124092523325d39fd3552e9b1da7069d50ca0a2 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Tue, 22 Dec 2020 11:04:45 +0300 Subject: [PATCH 20/23] fix "already committed" error --- google/cloud/spanner_dbapi/connection.py | 35 ++++++++++----------- tests/unit/spanner_dbapi/test_connection.py | 8 +++-- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/google/cloud/spanner_dbapi/connection.py b/google/cloud/spanner_dbapi/connection.py index 2540ec0fcb..6438605d3b 100644 --- a/google/cloud/spanner_dbapi/connection.py +++ b/google/cloud/spanner_dbapi/connection.py @@ -85,13 +85,7 @@ def autocommit(self, value): :type value: bool :param value: New autocommit mode state. """ - if ( - value - and not self._autocommit - and self._transaction - and not self._transaction.committed - and not self._transaction.rolled_back - ): + if value and not self._autocommit and self.inside_transaction: self.commit() self._autocommit = value @@ -105,6 +99,19 @@ def database(self): """ return self._database + @property + def inside_transaction(self): + """Flag: transaction is started. + + Returns: + bool: True if transaction begun, False otherwise. + """ + return ( + self._transaction + and not self._transaction.committed + and not self._transaction.rolled_back + ) + @property def instance(self): """Instance to which this connection relates. @@ -200,11 +207,7 @@ def transaction_checkout(self): :returns: A Cloud Spanner transaction object, ready to use. """ if not self.autocommit: - if ( - not self._transaction - or self._transaction.committed - or self._transaction.rolled_back - ): + if not self.inside_transaction: self._transaction = self._session_checkout().transaction() self._transaction.begin() @@ -225,11 +228,7 @@ def close(self): The connection will be unusable from this point forward. If the connection has an active transaction, it will be rolled back. """ - if ( - self._transaction - and not self._transaction.committed - and not self._transaction.rolled_back - ): + if self.inside_transaction: self._transaction.rollback() if self._own_pool: @@ -244,7 +243,7 @@ def commit(self): """ if self._autocommit: warnings.warn(AUTOCOMMIT_MODE_WARNING, UserWarning, stacklevel=2) - elif self._transaction and not self._transaction.rolled_back: + elif self.inside_transaction: try: self._transaction.commit() self._release_session() diff --git a/tests/unit/spanner_dbapi/test_connection.py b/tests/unit/spanner_dbapi/test_connection.py index f80469d03d..a338055a2c 100644 --- a/tests/unit/spanner_dbapi/test_connection.py +++ b/tests/unit/spanner_dbapi/test_connection.py @@ -197,7 +197,9 @@ def test_commit(self, mock_warn): connection.commit() mock_release.assert_not_called() - connection._transaction = mock_transaction = mock.MagicMock(rolled_back=False) + connection._transaction = mock_transaction = mock.MagicMock( + rolled_back=False, committed=False + ) mock_transaction.commit = mock_commit = mock.MagicMock() with mock.patch( @@ -383,7 +385,7 @@ def test_clear_statements_on_commit(self): cleared, when the transaction is commited. """ connection = self._make_connection() - connection._transaction = mock.Mock(rolled_back=False) + connection._transaction = mock.Mock(rolled_back=False, committed=False) connection._statements = [{}, {}] self.assertEqual(len(connection._statements), 2) @@ -486,7 +488,7 @@ def test_commit_retry_aborted_statements(self): statement = Statement("SELECT 1", [], {}, cursor._checksum, False) connection._statements.append(statement) - connection._transaction = mock.Mock(rolled_back=False) + connection._transaction = mock.Mock(rolled_back=False, committed=False) with mock.patch.object( connection._transaction, "commit", side_effect=(Aborted("Aborted"), None), From 6a4e248f6d0277d63b019246a10c0522faa1ef69 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Wed, 23 Dec 2020 11:15:05 +0300 Subject: [PATCH 21/23] fix for AttributeError: 'tuple' object has no attribute 'items' --- google/cloud/spanner_dbapi/cursor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/cloud/spanner_dbapi/cursor.py b/google/cloud/spanner_dbapi/cursor.py index 036158522f..64f0197233 100644 --- a/google/cloud/spanner_dbapi/cursor.py +++ b/google/cloud/spanner_dbapi/cursor.py @@ -176,7 +176,7 @@ def execute(self, sql, args=None): sql = parse_utils.ensure_where_clause(sql) if classification != parse_utils.STMT_INSERT: - sql, args = sql_pyformat_args_to_spanner(sql, args) + sql, args = sql_pyformat_args_to_spanner(sql, args or None) statement = Statement( sql, From dbd75ced1d4a97b1db0f9c246334fb7db34e3c0b Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Wed, 23 Dec 2020 16:25:07 +0300 Subject: [PATCH 22/23] fix --- google/cloud/spanner_dbapi/cursor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/cloud/spanner_dbapi/cursor.py b/google/cloud/spanner_dbapi/cursor.py index 64f0197233..48037113ab 100644 --- a/google/cloud/spanner_dbapi/cursor.py +++ b/google/cloud/spanner_dbapi/cursor.py @@ -181,7 +181,7 @@ def execute(self, sql, args=None): statement = Statement( sql, args, - get_param_types(args) + get_param_types(args or None) if classification != parse_utils.STMT_INSERT else {}, ResultsChecksum(), From f9306b251b26f60e326bd8763859afedcbd5ee79 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Thu, 24 Dec 2020 12:35:14 +0300 Subject: [PATCH 23/23] fix KeyError: 'type' --- google/cloud/spanner_dbapi/cursor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google/cloud/spanner_dbapi/cursor.py b/google/cloud/spanner_dbapi/cursor.py index 48037113ab..254eb5734a 100644 --- a/google/cloud/spanner_dbapi/cursor.py +++ b/google/cloud/spanner_dbapi/cursor.py @@ -95,9 +95,9 @@ def description(self): for field in row_type.fields: column_info = ColumnInfo( name=field.name, - type_code=field.type.code, + type_code=field.type_.code, # Size of the SQL type of the column. - display_size=code_to_display_size.get(field.type.code), + display_size=code_to_display_size.get(field.type_.code), # Client perceived size of the column. internal_size=field.ByteSize(), )