From bad60824fa47f1c5e67767f4eef1a0342a0ff089 Mon Sep 17 00:00:00 2001 From: jpburbank Date: Fri, 10 Sep 2021 15:04:52 -0700 Subject: [PATCH 1/9] allow updates within transaction to be dealt as update update statements executed with autocommit False are being handled as insert statements. The results in scenario will not have a row count of rows updated. Interestingly updates that are being executed with autocommit True, go through the proper path and are treated as update statements and come back with valid update row counts. Inline comments added for clarity to changes and adjacent blocks to help understand intent and usage. So the next person can read it quicker than myself. --- google/cloud/spanner_dbapi/cursor.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/google/cloud/spanner_dbapi/cursor.py b/google/cloud/spanner_dbapi/cursor.py index dccbf04dc8..8c2e2c95e0 100644 --- a/google/cloud/spanner_dbapi/cursor.py +++ b/google/cloud/spanner_dbapi/cursor.py @@ -206,13 +206,18 @@ def execute(self, sql, args=None): # self._run_prior_DDL_statements() self.connection.run_prior_DDL_statements() + # Statements run with existing transaction if not self.connection.autocommit: - if classification == parse_utils.STMT_UPDATING: - sql = parse_utils.ensure_where_clause(sql) - if classification != parse_utils.STMT_INSERT: sql, args = sql_pyformat_args_to_spanner(sql, args or None) + # Update statement + if classification == parse_utils.STMT_UPDATING: + sql = parse_utils.ensure_where_clause(sql) + transaction = self.connection.transaction_checkout() + self._do_execute_update(transaction, sql, args) + + # Other statements that either are insert or should be treated as inserts statement = Statement( sql, args, @@ -233,6 +238,7 @@ def execute(self, sql, args=None): self.connection.retry_transaction() return + # Statements run with out existing transactions if classification == parse_utils.STMT_NON_UPDATING: self._handle_DQL(sql, args or None) elif classification == parse_utils.STMT_INSERT: From 848ea545d7020de8e189049d7a64096d55569413 Mon Sep 17 00:00:00 2001 From: jpburbank Date: Fri, 10 Sep 2021 15:05:31 -0700 Subject: [PATCH 2/9] transaction update test test that the execute method will send an autoccommit false update to the proper method. As well return back proper updated row count --- tests/system/test_dbapi.py | 17 +++++++++++++++++ tests/unit/spanner_dbapi/test_cursor.py | 21 +++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/tests/system/test_dbapi.py b/tests/system/test_dbapi.py index 210a4f5e90..73b97e8d80 100644 --- a/tests/system/test_dbapi.py +++ b/tests/system/test_dbapi.py @@ -357,3 +357,20 @@ def test_ping(shared_instance, dbapi_database): conn = Connection(shared_instance, dbapi_database) conn.validate() conn.close() + +def test_update_non_autocommit(shared_instance, dbapi_database): + setup_rows = """ +INSERT INTO contacts (contact_id, first_name, last_name, email) +VALUES +(1, 'first-name', 'last-name', 'get@domen.ru'), +(2, 'first-name', 'last-name', 'get@domen.ru'), +(3, 'first-name', 'last-name', 'ignore@domen.ru') + """ + conn = Connection(shared_instance, dbapi_database) + cursor = conn.cursor() + cursor.execute(setup_rows) + conn.commit() + + cursor.execute("UPDATE contacts SET first_name='changed' WHERE email='get@domen.ru'") + conn.commit() + assert cursor.rowcount == 2 \ No newline at end of file diff --git a/tests/unit/spanner_dbapi/test_cursor.py b/tests/unit/spanner_dbapi/test_cursor.py index 07deffd707..837a5f5631 100644 --- a/tests/unit/spanner_dbapi/test_cursor.py +++ b/tests/unit/spanner_dbapi/test_cursor.py @@ -168,6 +168,27 @@ def test_execute_insert_statement_autocommit_off(self): self.assertIsInstance(cursor._result_set, mock.MagicMock) self.assertIsInstance(cursor._itr, PeekIterator) + def test_execute_update_statement_autocommit_off(self): + from google.cloud.spanner_dbapi import parse_utils + + connection = self._make_connection(self.INSTANCE, mock.MagicMock()) + cursor = self._make_one(connection) + cursor.connection._autocommit = False + cursor.connection.transaction_checkout = mock.MagicMock(autospec=True) + + with mock.patch( + "google.cloud.spanner_dbapi.parse_utils.classify_stmt", + return_value=parse_utils.STMT_UPDATING, + ): + with mock.patch( + "google.cloud.spanner_dbapi.cursor.Cursor._do_execute_update", + return_value=mock.MagicMock(), + ): + cursor.execute( + sql="UPDATE bogus row1=\"a\" WHERE row2=\"b\"" + ) + self.assertIsInstance(cursor._result_set, mock.MagicMock) + def test_execute_statement(self): from google.cloud.spanner_dbapi import parse_utils From 658d666fc775190242535019fedd7d9ffa107c18 Mon Sep 17 00:00:00 2001 From: jpburbank Date: Fri, 10 Sep 2021 15:26:17 -0700 Subject: [PATCH 3/9] formatting --- tests/system/test_dbapi.py | 7 +++++-- tests/unit/spanner_dbapi/test_cursor.py | 4 +--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/system/test_dbapi.py b/tests/system/test_dbapi.py index 73b97e8d80..790b49b567 100644 --- a/tests/system/test_dbapi.py +++ b/tests/system/test_dbapi.py @@ -358,6 +358,7 @@ def test_ping(shared_instance, dbapi_database): conn.validate() conn.close() + def test_update_non_autocommit(shared_instance, dbapi_database): setup_rows = """ INSERT INTO contacts (contact_id, first_name, last_name, email) @@ -371,6 +372,8 @@ def test_update_non_autocommit(shared_instance, dbapi_database): cursor.execute(setup_rows) conn.commit() - cursor.execute("UPDATE contacts SET first_name='changed' WHERE email='get@domen.ru'") + cursor.execute( + "UPDATE contacts SET first_name='changed' WHERE email='get@domen.ru'" + ) conn.commit() - assert cursor.rowcount == 2 \ No newline at end of file + assert cursor.rowcount == 2 diff --git a/tests/unit/spanner_dbapi/test_cursor.py b/tests/unit/spanner_dbapi/test_cursor.py index 837a5f5631..3983ecca5c 100644 --- a/tests/unit/spanner_dbapi/test_cursor.py +++ b/tests/unit/spanner_dbapi/test_cursor.py @@ -184,9 +184,7 @@ def test_execute_update_statement_autocommit_off(self): "google.cloud.spanner_dbapi.cursor.Cursor._do_execute_update", return_value=mock.MagicMock(), ): - cursor.execute( - sql="UPDATE bogus row1=\"a\" WHERE row2=\"b\"" - ) + cursor.execute(sql='UPDATE bogus row1="a" WHERE row2="b"') self.assertIsInstance(cursor._result_set, mock.MagicMock) def test_execute_statement(self): From fcfea8b050af26d824e54601eefe244c4491ac10 Mon Sep 17 00:00:00 2001 From: jpburbank Date: Tue, 28 Sep 2021 11:36:44 -0700 Subject: [PATCH 4/9] Update README.rst --- README.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.rst b/README.rst index 2eb77dff66..de53ddc949 100644 --- a/README.rst +++ b/README.rst @@ -1,3 +1,7 @@ +This is a fork for a bug fix. Waiting for corporate to sign the CLA. Once that is +done, then a PR is made, and then accepted, this fork should be deleted + + Python Client for Cloud Spanner =============================== From 57e654ac0818f9095fc595ed46f76928b973bef6 Mon Sep 17 00:00:00 2001 From: jpburbank Date: Tue, 28 Sep 2021 11:37:26 -0700 Subject: [PATCH 5/9] Update README.rst --- README.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.rst b/README.rst index de53ddc949..b9979b4c57 100644 --- a/README.rst +++ b/README.rst @@ -1,5 +1,5 @@ -This is a fork for a bug fix. Waiting for corporate to sign the CLA. Once that is -done, then a PR is made, and then accepted, this fork should be deleted +This is a fork for a bug fix. Waiting for corporate to sign the CLA. Once that is done, then a PR is made, and then accepted, this fork should be deleted +========================================================================================================================================================= Python Client for Cloud Spanner From 446033fc21662278a286250007ba39b07840625d Mon Sep 17 00:00:00 2001 From: jpburbank Date: Tue, 28 Sep 2021 11:44:25 -0700 Subject: [PATCH 6/9] Update README.rst --- README.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/README.rst b/README.rst index b9979b4c57..ac50d61863 100644 --- a/README.rst +++ b/README.rst @@ -1,5 +1,6 @@ -This is a fork for a bug fix. Waiting for corporate to sign the CLA. Once that is done, then a PR is made, and then accepted, this fork should be deleted -========================================================================================================================================================= +THIS IS A FORK +============== +This exists for a bug fix. Waiting for corporate to sign the CLA. Once that is done, then a PR is made, and then accepted, this fork should be deleted. Python Client for Cloud Spanner From ab42e74cd9b9a77390e999541acedfa73a8ae06d Mon Sep 17 00:00:00 2001 From: jpburbank Date: Tue, 28 Sep 2021 15:56:32 -0700 Subject: [PATCH 7/9] Update setup.py --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index d2f6d92d7e..2947ab49d8 100644 --- a/setup.py +++ b/setup.py @@ -20,7 +20,7 @@ # Package metadata. -name = "google-cloud-spanner" +name = "google-cloud-spanner-tmp" description = "Cloud Spanner API client library" version = "3.9.0" # Should be one of: From 00e7fdef96cd2603107820a18a02cd8131dff11a Mon Sep 17 00:00:00 2001 From: jpburbank Date: Wed, 29 Sep 2021 12:26:05 -0700 Subject: [PATCH 8/9] Update setup.py --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 2947ab49d8..d2f6d92d7e 100644 --- a/setup.py +++ b/setup.py @@ -20,7 +20,7 @@ # Package metadata. -name = "google-cloud-spanner-tmp" +name = "google-cloud-spanner" description = "Cloud Spanner API client library" version = "3.9.0" # Should be one of: From 02114181a2baf4b178d4e67e672b45b1d49e835d Mon Sep 17 00:00:00 2001 From: jpburbank Date: Wed, 29 Sep 2021 12:28:24 -0700 Subject: [PATCH 9/9] Update cursor.py --- 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 8c2e2c95e0..a85e025b9a 100644 --- a/google/cloud/spanner_dbapi/cursor.py +++ b/google/cloud/spanner_dbapi/cursor.py @@ -479,3 +479,5 @@ def get_table_column_schema(self, table_name): null_ok=is_nullable == "YES", spanner_type=spanner_type ) return column_details + +# This is a test