Crash in sqlite3 last_executed_query when using printf() formatting

[Matthias Kestenholz]

Here's a minimal testcase which shows the problem:

diff --git a/tests/backends/sqlite/tests.py b/tests/backends/sqlite/tests.py                                                                                                                                                           
index c82ed1667d..297b658810 100644                                                                                                                                                                                                    
--- a/tests/backends/sqlite/tests.py                                                                                                                                                                                                   
+++ b/tests/backends/sqlite/tests.py                                                                                                                                                                                                   
@@ -128,6 +128,12 @@ class LastExecutedQueryTest(TestCase):
             # This should not raise an exception.
             cursor.db.ops.last_executed_query(cursor.cursor, sql, params)
 
+    def test_printf(self):
+        with connection.cursor() as cursor:
+            sql = "SELECT printf(\"%x\", 10), %s"
+            params = ["hello"]
+            cursor.db.ops.last_executed_query(cursor.cursor, sql, params)
+
 
 @unittest.skipUnless(connection.vendor == 'sqlite', 'SQLite tests')
 class EscapingChecks(TestCase):

Here's the exception I got:

ERROR: test_printf (backends.sqlite.tests.LastExecutedQueryTest)                                                                                                                                                                       
----------------------------------------------------------------------                                                                                                                                                                 
Traceback (most recent call last):                                                                                                                                                                                                     
  File "/home/matthias/Projects/django/tests/backends/sqlite/tests.py", line 135, in test_printf                                                                                                                                       
    cursor.db.ops.last_executed_query(cursor.cursor, sql, params)                                                                                                                                                                      
  File "/home/matthias/Projects/django/django/db/backends/sqlite3/operations.py", line 147, in last_executed_query                                                                                                                     
    return sql % params                                                                                                                                                                                                                
TypeError: %x format: an integer is required, not str    

A real world example of using printf() can be found here: ​https://github.com/matthiask/django-tree-queries/blob/7eb6996203ad128dcef8fc88b29afa87dc2a4164/tree_queries/compiler.py#L69

[Ramiro Morales]

If you want the raw SQL "%x" formatting specifier to reach the sqlite printf function you'll need to double the % so it gets passed "un-interpreted" by the query building stage.

We have documentation about this requirement in other areas all of them related to raw SQL handling (circumventing the ORM):

• ​https://docs.djangoproject.com/en/2.0/topics/db/sql/#executing-custom-sql-directly
• ​https://docs.djangoproject.com/en/2.0/releases/1.6/#percent-literals-in-cursor-execute-queries
• ​https://docs.djangoproject.com/en/2.0/releases/1.7.1/#bugfixes (search for 'percent')
• ​https://docs.djangoproject.com/en/2.0/ref/migration-operations/#runsql

It could argued a similar note should be added to the connection.cursor.db.ops.last_executed_query() docs but currently it isn't documented, which IMHO isn't strange as it isn't a public API at all.

Do we want it to be a public API? The answer to this question would decide if this ticket should be converted to a documentation issue or closed as wontfix.

Or we could accept a patch for the last_executed_query() docstring.

[Matthias Kestenholz]

Oh wow, thanks! I'll retire this bug report. Someone else may reopen if they think that something should be done about this.

I definitely don't think making last_executed_query public API by documenting it is the right way to go. I'm using str.format in this project and it simply didn't occur to me that the printf argument should be escaped too (since everything worked fine as is)