Opened 3 years ago

Closed 2 years ago

#17158 closed Bug (fixed)

django.db.backends.BaseDatabaseOperations.last_executed_query can raise an exception

Reported by: a.gerchenov@… Owned by: aaugustin
Component: Database layer (models, ORM) Version: 1.2
Severity: Normal Keywords:
Cc: rmanoch@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Method "last_executed_query" should not always return "smart_unicode(sql) % u_params". If params variable is empty it should return just "smart_unicode(sql)". The problem is when we try to execute query with MySQL DATA_FORMAT function it says error.

Example:

from django.db import connection
c = connection.cursor()
c.execute("SELECT * FROM VAR_current GROUP BY DATE_FORMAT(closing_month, '%Y-%m');")
print c.fetchall()

Error:

django/db/backends/__init__.py", line 216, in last_executed_query
    return smart_unicode(sql) % u_params
TypeError: not enough arguments for format string

This issue is closely related to #9055.

Attachments (2)

17158-testcase.patch (944 bytes) - added by aaugustin 3 years ago.
17158.patch (3.1 KB) - added by aaugustin 2 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 3 years ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Summary changed from django.db.backends.BaseDatabaseOperations.last_executed_query issue to django.db.backends.BaseDatabaseOperations.last_executed_query can raise an exception
  • Triage Stage changed from Unreviewed to Accepted

Since r16081, there is a better implementation of last_executed_query for MySQL (and Oracle), so the problem is solved as far as MySQL is concerned.

It still exists for SQLite, and although we came to the conclusion that we couldn't build a reliable last_executed_query for SQLite in #14091, it shouldn't raise an exception.

I see three possible solutions:

  • 1) give up on interpolation entirely, and return a string like "SQL = ..., PARAMS = ...".
  • 2) catch errors in the interpolation and return only smart_unicode(sql)
  • 3) mangle the SQL to replace some % by %% before interpolation.

I'm in favor of 1 because it will avoid returning subtly invalid SQL when the database backend in SQLite. It's backwards incompatible, but last_executed_query never returned something usable under SQLite. Returning obviously incomplete SQL is less likely to confuse newcomers.

I'm against 3 because it's overengineering, and I'm not convinced it's possible to get it right for all cases anyway.

Version 0, edited 3 years ago by aaugustin (next)

Changed 3 years ago by aaugustin

comment:2 Changed 3 years ago by aaugustin

I've just uploaded a test case demonstrating the issue under SQLite. The assertion in this test case may have to be changed depending on the solution that we actually choose.

comment:3 Changed 2 years ago by aaugustin

  • Owner changed from nobody to aaugustin

Changed 2 years ago by aaugustin

comment:4 Changed 2 years ago by aaugustin

  • Has patch set
  • Triage Stage changed from Accepted to Ready for checkin

I'll commit the patch I just attached if no one objects.

comment:5 Changed 2 years ago by akaariai

looks good.

comment:6 Changed 2 years ago by Aymeric Augustin <aymeric.augustin@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 6605ac331a9e03fa41c301d122c5727c0d98b970:

Fixed #17158 -- Used a non-ambiguous representation of SQL queries

when a correct representation cannot be obtained.

Note: See TracTickets for help on using tickets.
Back to Top