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 559793d990a8..553aa1e1e052 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 @@ -314,14 +313,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) @@ -348,7 +347,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 2bd2570de92b..c876b0b7644e 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 @@ -406,6 +406,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): @@ -590,3 +645,51 @@ def test_spanner_reserved_keywords_compatibility(self): # for backwards compatibility with downstream consumers (e.g. sqlalchemy-spanner). self.assertIn("GRAPH_TABLE", SPANNER_RESERVED_KEYWORDS) self.assertIn("QUALIFY", SPANNER_RESERVED_KEYWORDS) + + 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