dbapi: unescape all %% before SQL upload to server

odeke-em · odeke-em · commit 80cb72635c8d · 2020-03-20T17:57:13.000-07:00
Ensures that every site that transforms sql will unescape escaped SQL format literals %% into plain %. Escaped SQL format literals exist because by default dbapi.Cursor.execute escapes %s as %%s, because %s is used as the format specifier. We transform '%s' into '@nAmed' variables so we should finish up the escaping before uploading the SQL to Cloud Spanner's server. Fixes #252 Fixes #347
diff --git a/packages/django-google-spanner/.travis.yml b/packages/django-google-spanner/.travis.yml
@@ -48,7 +48,7 @@ env:
         - DJANGO_TEST_APPS="queries.test_q queries.test_qs_combinators queries.test_query"
         # Commented out because they take longer 2hr and TravisCI unconditionally terminates them.
         # - DJANGO_TEST_APPS="queries.tests"
-        - DJANGO_TEST_APPS="raw_query redirects_tests reserved_names reverse_lookup"
+        - DJANGO_TEST_APPS="queries.tests.Queries5Tests.test_extra_select_literal_percent_s raw_query redirects_tests reserved_names reverse_lookup"
         - DJANGO_TEST_APPS="save_delete_hooks select_related"
         - DJANGO_TEST_APPS="select_related_onetoone signing sitemaps_tests"
         - DJANGO_TEST_APPS="string_lookup signals"
diff --git a/packages/django-google-spanner/django_spanner/features.py b/packages/django-google-spanner/django_spanner/features.py
@@ -270,9 +270,6 @@ class DatabaseFeatures(BaseDatabaseFeatures):
         # Cloud Spanner limit: "Number of functions exceeds the maximum
         # allowed limit of 1000."
         'queries.test_bulk_update.BulkUpdateTests.test_large_batch',
-        # QuerySet.extra() with select literal percent doesn't work:
-        # https://github.com/orijtech/spanner-orm/issues/252
-        'queries.tests.Queries5Tests.test_extra_select_literal_percent_s',
         # Spanner doesn't support random ordering.
         'ordering.tests.OrderingTests.test_random_ordering',
         # No matching signature for function MOD for argument types: FLOAT64,
@@ -307,10 +304,6 @@ class DatabaseFeatures(BaseDatabaseFeatures):
         # DatabaseIntrospection.get_relations() isn't implemented:
         # https://github.com/orijtech/django-spanner/issues/311
         'introspection.tests.IntrospectionTests.test_get_relations',
-        # parameter escaping of % not working correctly:
-        # https://github.com/orijtech/django-spanner/issues/347
-        'backends.tests.EscapingChecks.test_parameter_escaping',
-        'backends.tests.EscapingChecksDebug.test_parameter_escaping',
         # Non-ascii SELECT alias crashes "Syntax error: Illegal input character"
         # https://github.com/orijtech/django-spanner/issues/341
         'backends.tests.LastExecutedQueryTest.test_query_encoding',
diff --git a/packages/django-google-spanner/spanner/dbapi/parse_utils.py b/packages/django-google-spanner/spanner/dbapi/parse_utils.py
@@ -15,6 +15,7 @@
 from .exceptions import Error, ProgrammingError
 from .parser import parse_values
 from .types import DateStr, TimestampStr
+from .utils import unescape_percent_literals
 
 STMT_DDL = 'DDL'
 STMT_NON_UPDATING = 'NON_UPDATING'
@@ -143,6 +144,7 @@ def parse_insert(insert_sql, params):
     after_values_sql = re_VALUES_TILL_END.findall(insert_sql)
     if not after_values_sql:
         # Case b)
+        insert_sql = unescape_percent_literals(insert_sql)
         return {
             'sql_params_list': [(insert_sql, None,)],
         }
@@ -154,6 +156,7 @@ def parse_insert(insert_sql, params):
         if pyformat_str_count > 0:
             raise ProgrammingError('no params yet there are %d "%s" tokens' % pyformat_str_count)
 
+        insert_sql = unescape_percent_literals(insert_sql)
         # Confirmed case of:
         # SQL: INSERT INTO T (a1, a2) VALUES (1, 2)
         # Params: None
@@ -177,6 +180,7 @@ def parse_insert(insert_sql, params):
         )
         values_pyformat = [str(arg) for arg in values.argv]
         rows_list = rows_for_insert_or_update(columns, params, values_pyformat)
+        insert_sql_preamble = unescape_percent_literals(insert_sql_preamble)
         for row in rows_list:
             sql_params_list.append((insert_sql_preamble, row,))
 
@@ -202,6 +206,7 @@ def parse_insert(insert_sql, params):
     sql_param_tuples = []
     for token_arg in values.argv:
         row_sql = before_values_sql + ' VALUES%s' % token_arg
+        row_sql = unescape_percent_literals(row_sql)
         row_params, params = tuple(params[0:len(token_arg)]), params[len(token_arg):]
         sql_param_tuples.append((row_sql, row_params,))
 
@@ -297,6 +302,8 @@ def rows_for_insert_or_update(columns, params, pyformat_args=None):
 def sql_pyformat_args_to_spanner(sql, params):
     """
     Transform pyformat set SQL to named arguments for Cloud Spanner.
+    It will also unescape previously escaped format specifiers
+    like %%s to %s.
     For example:
         SQL:      'SELECT * from t where f1=%s, f2=%s, f3=%s'
         Params:   ('a', 23, '888***')
@@ -312,14 +319,14 @@ def sql_pyformat_args_to_spanner(sql, params):
         Params:   {'a0': 'a', 'a1': 23, 'a2': '888***'}
     """
     if not params:
-        return sql, params
+        return unescape_percent_literals(sql), params
 
     found_pyformat_placeholders = re_PYFORMAT.findall(sql)
     params_is_dict = isinstance(params, dict)
 
     if params_is_dict:
         if not found_pyformat_placeholders:
-            return sql, params
+            return unescape_percent_literals(sql), params
     else:
         n_params = len(params) if params else 0
         n_matches = len(found_pyformat_placeholders)
@@ -329,7 +336,7 @@ def sql_pyformat_args_to_spanner(sql, params):
                 'want %d args in %s' % (n_matches, found_pyformat_placeholders, n_params, params))
 
     if len(params) == 0:
-        return sql, params
+        return unescape_percent_literals(sql), params
 
     named_args = {}
     # We've now got for example:
@@ -347,7 +354,7 @@ def sql_pyformat_args_to_spanner(sql, params):
         else:
             named_args[key] = cast_for_spanner(params[i])
 
-    return sql, named_args
+    return unescape_percent_literals(sql), named_args
 
 
 def cast_for_spanner(param):
diff --git a/packages/django-google-spanner/spanner/dbapi/utils.py b/packages/django-google-spanner/spanner/dbapi/utils.py
@@ -73,3 +73,12 @@ def get_table_column_schema(spanner_db, table_name):
             )
 
         return column_details
+
+
+def unescape_percent_literals(s):
+    """
+    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.
+    """
+    return s.replace('%%', '%')
