From 1dfc4be58aa636e6760ae085d962ec4e6c006b01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Wed, 11 Nov 2026 18:21:12 +0100 Subject: [PATCH] fix(spanner): eliminate silent SQL truncation in DB-API When SQL queries executed through the DB-API driver contained non-ASCII characters (such as accented characters in table names, string values, or comments), an internal routine attempted to automatically wrap those words in backticks. Due to a bug in that routine, any SQL text following the non-ASCII character was silently discarded. For targeted UPDATE and DELETE statements (such as deleting a single row by ID), stripping the trailing WHERE clause caused the driver to detect an unqualified statement and automatically append "WHERE 1=1", turning a scoped single-row modification into an accidental full-table wipe. Additionally, string literals containing Unicode characters were erroneously converted into column identifiers, and adjacent punctuation was corrupted. This change removes the legacy auto-backticking routine entirely. The database driver now treats incoming SQL statements as transparent text rather than attempting to guess and quote identifiers using regular expressions. Frameworks like SQLAlchemy and Django already quote identifiers safely at the query compiler level. Query preprocessing in the driver is now limited strictly to translating parameter placeholders and unescaping literal percent signs. Comprehensive regression tests have been added to ensure queries containing Unicode table names, string literals, and comments preserve their complete WHERE predicates. --- .../google/cloud/spanner_dbapi/parse_utils.py | 7 +- .../google/cloud/spanner_dbapi/utils.py | 53 --------- .../tests/unit/spanner_dbapi/test_cursor.py | 26 ++++- .../unit/spanner_dbapi/test_parse_utils.py | 103 ++++++++++++++++++ .../tests/unit/spanner_dbapi/test_utils.py | 15 --- 5 files changed, 130 insertions(+), 74 deletions(-) diff --git a/packages/google-cloud-spanner/google/cloud/spanner_dbapi/parse_utils.py b/packages/google-cloud-spanner/google/cloud/spanner_dbapi/parse_utils.py index c00aaf245ee2..bcaddd4a2832 100644 --- a/packages/google-cloud-spanner/google/cloud/spanner_dbapi/parse_utils.py +++ b/packages/google-cloud-spanner/google/cloud/spanner_dbapi/parse_utils.py @@ -28,7 +28,6 @@ from .exceptions import Error from .parsed_statement import ParsedStatement, Statement, StatementType from .types import DateStr, TimestampStr -from .utils import sanitize_literals_for_upload # Note: This mapping deliberately does not contain a value for float. # The reason for that is that it is better to just let Spanner determine @@ -309,14 +308,14 @@ def sql_pyformat_args_to_spanner(sql, params): arguments. """ if not params: - return sanitize_literals_for_upload(sql), None + return sql.replace("%%", "%"), None found_pyformat_placeholders = RE_PYFORMAT.findall(sql) params_is_dict = isinstance(params, dict) if params_is_dict: if not found_pyformat_placeholders: - return sanitize_literals_for_upload(sql), params + return sql.replace("%%", "%"), params else: n_params = len(params) if params else 0 n_matches = len(found_pyformat_placeholders) @@ -343,7 +342,7 @@ def sql_pyformat_args_to_spanner(sql, params): else: named_args[key] = params[i] - return sanitize_literals_for_upload(sql), named_args + return sql.replace("%%", "%"), named_args def get_param_types(params): diff --git a/packages/google-cloud-spanner/google/cloud/spanner_dbapi/utils.py b/packages/google-cloud-spanner/google/cloud/spanner_dbapi/utils.py index f5db4a2c9629..3160f33090ec 100644 --- a/packages/google-cloud-spanner/google/cloud/spanner_dbapi/utils.py +++ b/packages/google-cloud-spanner/google/cloud/spanner_dbapi/utils.py @@ -12,10 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import re - -re_UNICODE_POINTS = re.compile(r"([^\s]*[\u0080-\uFFFF]+[^\s]*)") - class BufferedIterator: """ @@ -126,52 +122,3 @@ def __next__(self): def __iter__(self): return self - - -def backtick_unicode(sql): - """Check the SQL to be valid and split it by segments. - - :type sql: str - :param sql: A SQL request. - - :rtype: str - :returns: A SQL parsed by segments in unicode if initial SQL is valid, - initial string otherwise. - """ - matches = list(re_UNICODE_POINTS.finditer(sql)) - if not matches: - return sql - - segments = [] - - last_end = 0 - for match in matches: - start, end = match.span() - if sql[start] != "`" and sql[end - 1] != "`": - segments.append(sql[last_end:start] + "`" + sql[start:end] + "`") - else: - segments.append(sql[last_end:end]) - - last_end = end - - return "".join(segments) - - -def sanitize_literals_for_upload(s): - """Convert literals in s, to be fit for consumption by Cloud Spanner. - - * Convert %% (escaped percent literals) to %. Percent signs must be escaped - when values like %s are used as SQL parameter placeholders but Spanner's - query language uses placeholders like @a0 and doesn't expect percent - signs to be escaped. - * Quote words containing non-ASCII, with backticks, for example föö to - `föö`. - - :type s: str - :param s: A string with literals to escaped for consumption by Cloud - Spanner. - - :rtype: str - :returns: A sanitized string for uploading. - """ - return backtick_unicode(s.replace("%%", "%")) diff --git a/packages/google-cloud-spanner/tests/unit/spanner_dbapi/test_cursor.py b/packages/google-cloud-spanner/tests/unit/spanner_dbapi/test_cursor.py index 52b4026f5437..8d7b33fe4a98 100644 --- a/packages/google-cloud-spanner/tests/unit/spanner_dbapi/test_cursor.py +++ b/packages/google-cloud-spanner/tests/unit/spanner_dbapi/test_cursor.py @@ -22,6 +22,7 @@ from google.auth.credentials import AnonymousCredentials from google.rpc.code_pb2 import ABORTED +from google.cloud import spanner from google.cloud.spanner_dbapi.connection import connect from google.cloud.spanner_dbapi.parsed_statement import ( ParsedStatement, @@ -1008,9 +1009,30 @@ def test_handle_dql_priority(self, MockedPeekIterator): sql, None, None, request_options=RequestOptions(priority=1) ) - def test_execute_query_with_auto_partition_mode(self): - from google.cloud import spanner + @mock.patch("google.cloud.spanner_dbapi.cursor.PeekIterator") + def test_execute_query_with_unicode_identifier_and_literal( + self, MockedPeekIterator + ): + connection = self._make_connection(self.INSTANCE, mock.MagicMock()) + connection.autocommit = True + mock_snapshot = mock.MagicMock() + connection.database.snapshot.return_value.__enter__.return_value = mock_snapshot + mock_result_set = mock.Mock() + mock_snapshot.execute_sql.return_value = mock_result_set + cursor = self._make_one(connection) + sql = "SELECT `tätä`, name FROM `Tëst` WHERE dept = 'café' AND id = %s" + cursor.execute(sql, (42,)) + + self.assertEqual(cursor._result_set, mock_result_set) + mock_snapshot.execute_sql.assert_called_once_with( + "SELECT `tätä`, name FROM `Tëst` WHERE dept = 'café' AND id = @a0", + {"a0": 42}, + {"a0": spanner.param_types.INT64}, + request_options=None, + ) + + def test_execute_query_with_auto_partition_mode(self): connection = self._make_connection( self.INSTANCE, mock.MagicMock(), diff --git a/packages/google-cloud-spanner/tests/unit/spanner_dbapi/test_parse_utils.py b/packages/google-cloud-spanner/tests/unit/spanner_dbapi/test_parse_utils.py index 71c4bb4d7fda..bec68c727a93 100644 --- a/packages/google-cloud-spanner/tests/unit/spanner_dbapi/test_parse_utils.py +++ b/packages/google-cloud-spanner/tests/unit/spanner_dbapi/test_parse_utils.py @@ -402,6 +402,61 @@ def test_sql_pyformat_args_to_spanner(self): ("SELECT * from t WHERE id=10", {"f1": "app", "f2": "name"}), ("SELECT * from t WHERE id=10", {"f1": "app", "f2": "name"}), ), + ( + # Preserves Unicode in identifiers and trailing WHERE predicates + ( + "SELECT * FROM `Tëst` WHERE tenant_id = %s", + ("tenant_1",), + ), + ( + "SELECT * FROM `Tëst` WHERE tenant_id = @a0", + {"a0": "tenant_1"}, + ), + ), + ( + # Preserves Unicode in string literals without backticking or truncating + ( + "SELECT * FROM users WHERE dept = 'café' AND tenant_id = %s", + ("tenant_1",), + ), + ( + "SELECT * FROM users WHERE dept = 'café' AND tenant_id = @a0", + {"a0": "tenant_1"}, + ), + ), + ( + # Preserves percent escaping (%% -> %) alongside parameters + ( + "SELECT * FROM users WHERE name LIKE 'A%%' AND age > %s", + (21,), + ), + ( + "SELECT * FROM users WHERE name LIKE 'A%' AND age > @a0", + {"a0": 21}, + ), + ), + ( + # Preserves Unicode in unparameterized queries (params=None) + ( + "SELECT * FROM users WHERE city = 'München' AND active = true", + None, + ), + ( + "SELECT * FROM users WHERE city = 'München' AND active = true", + None, + ), + ), + ( + # Preserves Unicode in dict-based named parameters and literals + ( + "SELECT * FROM users WHERE name = %(name)s AND dept = 'café'", + {"name": "Alice"}, + ), + ( + "SELECT * FROM users WHERE name = @a0 AND dept = 'café'", + {"a0": "Alice"}, + ), + ), ] for (sql_in, params), sql_want in cases: with self.subTest(sql=sql_in): @@ -532,3 +587,51 @@ def test_escape_name(self): with self.subTest(name=name): got = escape_name(name) self.assertEqual(got, want) + + def test_classify_statement_preserves_unicode_and_predicates(self): + # Scoped SELECT with Unicode identifier preserves WHERE clause + query = "SELECT * FROM `Tëst` WHERE tenant_id = %s" + parsed = classify_statement(query, ("tenant_1",)) + self.assertEqual( + parsed.statement.sql, "SELECT * FROM `Tëst` WHERE tenant_id = @a0" + ) + self.assertEqual(parsed.statement.params, {"a0": "tenant_1"}) + self.assertEqual(parsed.statement_type, StatementType.QUERY) + + # Scoped DELETE with Unicode identifier does not drop WHERE or append WHERE 1=1 + query = "DELETE FROM `Tëst` WHERE id = %s" + parsed = classify_statement(query, (42,)) + self.assertEqual(parsed.statement.sql, "DELETE FROM `Tëst` WHERE id = @a0") + self.assertEqual(parsed.statement.params, {"a0": 42}) + self.assertEqual(parsed.statement_type, StatementType.UPDATE) + self.assertNotIn("WHERE 1=1", parsed.statement.sql) + + # Scoped UPDATE with Unicode in SET clause does not convert WHERE clause to WHERE 1=1 + query = "UPDATE users SET note = 'Café' WHERE id = %s" + parsed = classify_statement(query, (42,)) + self.assertEqual( + parsed.statement.sql, "UPDATE users SET note = 'Café' WHERE id = @a0" + ) + self.assertEqual(parsed.statement.params, {"a0": 42}) + self.assertEqual(parsed.statement_type, StatementType.UPDATE) + self.assertNotIn("WHERE 1=1", parsed.statement.sql) + + # Scoped UPDATE with Unicode literal preserves literal and WHERE clause + query = "UPDATE users SET status = 'active' WHERE name = 'José' AND id = %s" + parsed = classify_statement(query, (1,)) + self.assertEqual( + parsed.statement.sql, + "UPDATE users SET status = 'active' WHERE name = 'José' AND id = @a0", + ) + self.assertEqual(parsed.statement.params, {"a0": 1}) + self.assertEqual(parsed.statement_type, StatementType.UPDATE) + self.assertNotIn("WHERE 1=1", parsed.statement.sql) + + # Adjacent punctuation with Unicode identifiers does not swallow punctuation + query = "SELECT `tätä`, id FROM users WHERE id = %s" + parsed = classify_statement(query, (1,)) + self.assertEqual( + parsed.statement.sql, + "SELECT `tätä`, id FROM users WHERE id = @a0", + ) + self.assertEqual(parsed.statement.params, {"a0": 1}) diff --git a/packages/google-cloud-spanner/tests/unit/spanner_dbapi/test_utils.py b/packages/google-cloud-spanner/tests/unit/spanner_dbapi/test_utils.py index 9fb3c492c3b9..234aac617e7d 100644 --- a/packages/google-cloud-spanner/tests/unit/spanner_dbapi/test_utils.py +++ b/packages/google-cloud-spanner/tests/unit/spanner_dbapi/test_utils.py @@ -62,21 +62,6 @@ def test_peekIterator_nonlist_rows_unconverted(self): want = ["a", "b", "c", "d", "e"] self.assertEqual(got, want, "Values should be returned unchanged") - def test_backtick_unicode(self): - from google.cloud.spanner_dbapi.utils import backtick_unicode - - cases = [ - ("SELECT (1) as foo WHERE 1=1", "SELECT (1) as foo WHERE 1=1"), - ("SELECT (1) as föö", "SELECT (1) as `föö`"), - ("SELECT (1) as `föö`", "SELECT (1) as `föö`"), - ("SELECT (1) as `föö` `umläut", "SELECT (1) as `föö` `umläut"), - ("SELECT (1) as `föö", "SELECT (1) as `föö"), - ] - for sql, want in cases: - with self.subTest(sql=sql): - got = backtick_unicode(sql) - self.assertEqual(got, want) - def test_StreamedManyResultSets(self): from google.cloud.spanner_dbapi.utils import StreamedManyResultSets