Opened 14 years ago

Closed 9 years ago

#12268 closed Bug (duplicate)

[PATCH] can't do %s on extra select

Reported by: manuel@… Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: select extra escape
Cc: anssi.kaariainen@… Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

There are times when you want to send to the database manager %s, an example of this usage would be converting from time string to epoch inside the database it self, for doing this you can use a line like: qs=qs.extra(select={'timestamp': "strftime(\"%%s\", time, 'utc')"}) (sqlite3).

Problem with this is that django detects %s in the select argument of extra and then waits for a select_params, unfortunately there's no escape been taken into account. The patch attached fixed this by detecting %%s as a valid escape sequence.

Attachments (2)

django_fixpercent_s_on_extra.patch (860 bytes ) - added by manuel@… 14 years ago.
this patch fixes the bug described.
12268_regex.diff (3.4 KB ) - added by Anssi Kääriäinen 12 years ago.

Download all attachments as: .zip

Change History (10)

by manuel@…, 14 years ago

this patch fixes the bug described.

comment:1 by master, 14 years ago

Exactly the same problem, I have to feed a sqlite DB with a "strftime('%s',date)".

When I saw the '... avoid using the substring "%%s"...' in the documentation, I first thought 'well, so there must be another way to give this pattern, let's have a look at the source' ... but no.

Manuel, your patch works great! Thanks

(I know, my comment may be only a me-too, but it's a support to Manuel and a vote to have this ticket processed)

comment:2 by Russell Keith-Magee, 14 years ago

Needs documentation: set
Needs tests: set
Triage Stage: UnreviewedAccepted

comment:3 by master, 14 years ago

As from version 1.2, this patch is still necessary, but not sufficient:

See also the patch in #13648

comment:4 by Matt McClanahan, 13 years ago

Severity: Normal
Type: Bug

comment:5 by Julien Phalip, 13 years ago

See some other issues relating to the use of the % character and escaping: #3485, #6343, #11391, #13648. Perhaps some of these could be tackled at the same time.

comment:6 by elachuni, 12 years ago

Easy pickings: unset
UI/UX: unset

fwiw, this is still broken in trunk.

by Anssi Kääriäinen, 12 years ago

Attachment: 12268_regex.diff added

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

Cc: anssi.kaariainen@… added
Needs tests: unset
Version: 1.1SVN

Attached patch should fix this in a way that %s needs a param, %%s does not, %%%s does need and so on. The patch is based on regular expressions. According to my performance tests it is slightly faster than the original patch, but slightly slower than current implementation.

Tests attached. Does this really need documentation? Isn't it expected behavior that %%s is escaped?

Last edited 12 years ago by Anssi Kääriäinen (previous) (diff)

comment:8 by Tim Graham, 9 years ago

Resolution: duplicate
Status: newclosed

Duplicate of #23460

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