Opened 6 years ago

Closed 8 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


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@… 6 years ago.
this patch fixes the bug described.
12268_regex.diff (3.4 KB) - added by akaariai 4 years ago.

Download all attachments as: .zip

Change History (10)

Changed 6 years ago by manuel@…

this patch fixes the bug described.

comment:1 Changed 6 years ago by master

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 6 years ago by russellm

  • Needs documentation set
  • Needs tests set
  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 5 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 5 years ago by mattmcc

  • Severity set to Normal
  • Type set to Bug

comment:5 Changed 5 years ago by julien

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

  • Easy pickings unset
  • UI/UX unset

fwiw, this is still broken in trunk.

Changed 4 years ago by akaariai

comment:7 Changed 4 years ago by akaariai

  • Cc anssi.kaariainen@… added
  • Needs tests unset
  • Version changed from 1.1 to SVN

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 4 years ago by akaariai (previous) (diff)

comment:8 Changed 8 months ago by timgraham

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

Duplicate of #23460

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