Opened 7 years ago

Closed 22 months 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: master
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@… 7 years ago.
this patch fixes the bug described.
12268_regex.diff (3.4 KB) - added by Anssi Kääriäinen 5 years ago.

Download all attachments as: .zip

Change History (10)

Changed 7 years ago by manuel@…

this patch fixes the bug described.

comment:1 Changed 7 years ago by master

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 Changed 7 years ago by Russell Keith-Magee

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

comment:3 Changed 7 years ago by master

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

See also the patch in #13648

comment:4 Changed 6 years ago by Matt McClanahan

Severity: Normal
Type: Bug

comment:5 Changed 6 years ago by Julien Phalip

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 Changed 5 years ago by elachuni

Easy pickings: unset
UI/UX: unset

fwiw, this is still broken in trunk.

Changed 5 years ago by Anssi Kääriäinen

Attachment: 12268_regex.diff added

comment:7 Changed 5 years ago by Anssi Kääriäinen

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 5 years ago by Anssi Kääriäinen (previous) (diff)

comment:8 Changed 22 months ago by Tim Graham

Resolution: duplicate
Status: newclosed

Duplicate of #23460

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