Opened 13 years ago

Closed 12 years ago

#17158 closed Bug (fixed)

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

Reported by: a.gerchenov@… Owned by: Aymeric Augustin
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 Aymeric Augustin 13 years ago.
17158.patch (3.1 KB ) - added by Aymeric Augustin 12 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 by Aymeric Augustin, 13 years ago

Summary: django.db.backends.BaseDatabaseOperations.last_executed_query issuedjango.db.backends.BaseDatabaseOperations.last_executed_query can raise an exception
Triage Stage: UnreviewedAccepted

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 13 years ago by Aymeric Augustin (next)

by Aymeric Augustin, 13 years ago

Attachment: 17158-testcase.patch added

comment:2 by Aymeric Augustin, 13 years ago

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 by Aymeric Augustin, 12 years ago

Owner: changed from nobody to Aymeric Augustin

by Aymeric Augustin, 12 years ago

Attachment: 17158.patch added

comment:4 by Aymeric Augustin, 12 years ago

Has patch: set
Triage Stage: AcceptedReady for checkin

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

comment:5 by Anssi Kääriäinen, 12 years ago

looks good.

comment:6 by Aymeric Augustin <aymeric.augustin@…>, 12 years ago

Resolution: fixed
Status: newclosed

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