Opened 14 years ago

Closed 3 weeks ago

#10070 closed Bug (fixed)

Named parameters not working on raw sql queries with sqlite

Reported by: Matias Surdi Owned by: Ryan Cheley
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: shai@…, mike@…, Joseph Gordon 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 (last modified by Ramiro Morales)

The following code shows the problem when using sqlite:

$ python manage.py shell
Python 2.5.2 (r252:60911, Oct  5 2008, 19:29:17) 
[GCC 4.3.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> from django.db import connection
>>> c = connection.cursor()
>>> c.execute("select name from inventory_host where id=%(id)s",{'id':'1'})
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/usr/lib/python2.5/site-packages/django/db/backends/util.py", line 19, in execute
    return self.cursor.execute(sql, params)
  File "/usr/lib/python2.5/site-packages/django/db/backends/sqlite3/base.py", line 167, in execute
    query = self.convert_query(query, len(params))
  File "/usr/lib/python2.5/site-packages/django/db/backends/sqlite3/base.py", line 179, in convert_query
    return query % tuple("?" * num_params)
TypeError: format requires a mapping
>>> 
>>> import django
>>> django.VERSION
(1, 0, 2, 'final', 0)
$ sqlite3 --version
3.5.9

When using Mysql or Postgres, that works (not tested by me, but by others on django-users).

Attachments (2)

svnpatch.diff (1.3 KB) - added by Matias Surdi 14 years ago.
allow dict as parameters
base.diff (1.9 KB) - added by anonymous 13 years ago.
Another approach using try and a pseudo dictionary

Download all attachments as: .zip

Change History (36)

comment:1 Changed 14 years ago by Ramiro Morales

Description: modified (diff)

(description formatting)

comment:2 Changed 14 years ago by Ramiro Morales

Adding info from the django-users thread:

SQLite and Oracle backends don't support this. In fact, Oracle used to have this and it was deleted in [9418] in response to a ticket (#9478) that reported it was faulty with this commit message "Removed support for passing params as dicts in the oracle backend. Wasn't documented, didn't work, isn't necessary".

The relevant docs (http://docs.djangoproject.com/en/dev/topics/db/sql/#topics-db-sql) don't state clearly if using named parameter marker plus a mapping is supported or not, so maybe this is a documentation issue?.

comment:3 Changed 14 years ago by Ian Kelly

The main reason for removing it from Oracle was that it never worked in the first place, nobody seemed to be using it, and it didn't seem worthwhile to maintain at the time. If there is demand to add this as an explicitly documented feature, I'd be happy to put it back in.

comment:4 Changed 14 years ago by Matias Surdi

I'm attaching a patch for SQLite.

With this patch, now the parameters can be passed as a dict, and the query can contain %(parameter)s as with mysql or postgres.

Changed 14 years ago by Matias Surdi

Attachment: svnpatch.diff added

allow dict as parameters

comment:5 Changed 14 years ago by Alex Gaynor

Triage Stage: UnreviewedDesign decision needed

comment:6 Changed 14 years ago by Ivan Giuliani

Component: UncategorizedDatabase layer (models, ORM)
Needs tests: set

Changed 13 years ago by anonymous

Attachment: base.diff added

Another approach using try and a pseudo dictionary

comment:7 Changed 13 years ago by ke1g.nh@…

I've attached a different fix (http://code.djangoproject.com/attachment/ticket/10070/base.diff) for this (though similar in places to msurdi's). Sorry about the annonymous user on the attachement, but I didn't see how to combine it with this.

It should be faster in the apparently more common case of supplying a sequence, since the only extra overhead is the try block setup.

Also, since the text replacement is done by the format operator itself, I'm more confident that the result will be correct, where I have to worry about missing the fact that a percent sign is quoted when using the regular expression replace approach of the original patch.

A hint if anyone doesn't find the colonifier to be obvious: the format operator only cares that its right operand has a getitem method. (Tested with python versions 2.4 and 2.6.)

This now allows my PostgreSQL developed mapping queries to work with sqlite3 (Django-1.0.3, python 2.6).

Sadly, I have no test.py code to offer.

Bill

comment:8 Changed 12 years ago by Chris Beaven

Severity: Normal
Type: Bug

comment:9 Changed 11 years ago by Aymeric Augustin

Easy pickings: unset
Triage Stage: Design decision neededAccepted
UI/UX: unset

Based on Ramiro's and Ian's input in the first comments, there's no fundamental reason not to support named parameters on Oracle and SQL, it's just a matter of writing a solid implementation.

comment:10 in reply to:  2 Changed 10 years ago by Shai Berger

Cc: shai@… added
Has patch: set

Replying to ramiro:

In fact, Oracle used to have this and it was deleted in [9418] in response to a ticket (#9478)

For future reference, the relevant ticket is #9408. The code that was removed indeed did not really support named parameters (except by names "arg0", "arg1" etc.).

Pull Request 411 supports named parameters for real. No tests provided (it still isn't a documented feature as far as I know...). I can do the documentation and add tests if this is deemed required.

comment:11 Changed 10 years ago by anonymous

I think at very least this should be mentioned explicitly in the docs.

comment:12 Changed 10 years ago by Mike Fogel

Cc: mike@… added

comment:13 Changed 10 years ago by rupa108

I started a related discussion here https://groups.google.com/forum/#!topic/django-developers/ts14HX9Yz0I

named parameters do currently work with some db interfaces but the code breaks on the repr() method of a RawQuerySet:

param = dict(lname = 'Doe')
qs = Person.objects.raw('SELECT * FROM myapp_person WHERE last_name = %(lname)s', param)
repr(qs)

/home/user/vpy/dev/lib/python2.7/site-packages/django/db/models/query.pyc
in __repr__(self)
   1530
   1531     def __repr__(self):
-> 1532         return "<RawQuerySet: %r>" % (self.raw_query %
tuple(self.params))
   1533
   1534     def __getitem__(self, k):

TypeError: format requires a mapping
Last edited 7 years ago by Tim Graham (previous) (diff)

comment:14 Changed 9 years ago by Shai Berger <shai@…>

In d097417025e71286ad5bbde6e0a79caacabbbd64:

Support 'pyformat' style parameters in raw queries, Refs #10070

Add support for Oracle, fix an issue with the repr of RawQuerySet,
add tests and documentations. Also added a 'supports_paramstyle_pyformat'
database feature, True by default, False for SQLite.

Thanks Donald Stufft for review of documentation.

comment:15 Changed 7 years ago by Joseph Gordon

Cc: Joseph Gordon added

comment:16 Changed 4 years ago by Tobias Kunze

Version: 1.0master

This still breaks on SQLite (3.27.2), although with a less helpful error message:

Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/home/rixx/.local/share/virtualenvs/demo/lib/python3.7/site-packages/django/db/backends/utils.py", line 67, in execute
    return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
  File "/home/rixx/.local/share/virtualenvs/demo/lib/python3.7/site-packages/django/db/backends/utils.py", line 76, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "/home/rixx/.local/share/virtualenvs/demo/lib/python3.7/site-packages/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
  File "/home/rixx/.local/share/virtualenvs/demo/lib/python3.7/site-packages/django/db/utils.py", line 89, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/home/rixx/.local/share/virtualenvs/demo/lib/python3.7/site-packages/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
  File "/home/rixx/.local/share/virtualenvs/demo/lib/python3.7/site-packages/django/db/backends/sqlite3/base.py", line 383, in execute
    return Database.Cursor.execute(self, query, params)
django.db.utils.OperationalError: near "%": syntax error

This syntax isn't documented (as far as I can see, at least not in topics/db/sql) – do we want to add documentation for this, and exclude SQLite?

comment:17 in reply to:  16 Changed 4 years ago by Shai Berger

Replying to Tobias Kunze:

This syntax isn't documented (as far as I can see, at least not in topics/db/sql) – do we want to add documentation for this, and exclude SQLite?

That much I did almost six years ago, as noted in comment:14 -- and it is still there.

comment:18 Changed 3 years ago by Baptiste Mispelon

I don't know how recent it is, but sqlite does support named parameters, albeit with a different syntax [1].

Both of these work on my machine:

c.execute("select name from inventory_host where id=:id", {'id': '1'})
Host.objects.raw("select * from inventory_host where id=:id", {'id': '1'})

Do we want to try and make the syntax consistent across all backends or would it be enough to document the syntax for each one?

[1] https://docs.python.org/3/library/sqlite3.html#sqlite3.Cursor.execute

comment:19 Changed 3 years ago by Simon Charette

Do we want to try and make the syntax consistent across all backends or would it be enough to document the syntax for each one?

Given the most common use case of raw and execute is to perform non database-agnostic SQL I think documenting the syntax would be fine and easier to maintain. I think we already so format massaging for Oracle though.

comment:20 Changed 2 years ago by Charlie DeTar

I encountered a use case where I wanted to do an upsert query (INSERT ... ON CONFLICT DO UPDATE SET ...). This is raw SQL that is not supported by the ORM, but has consistent syntax between sqlite and postgresql. I ran into this trouble with named parameters. The query syntax itself worked for both backends, but the parameter naming syntax did not. I worked around this by just using positional arguments instead -- but there are valid needs for raw SQL targeting more than one database backend, and consistent named parameter syntax would be useful.

comment:21 Changed 6 weeks ago by Ryan Cheley

Owner: changed from nobody to Ryan Cheley
Status: newassigned

I'm at DjangoCon US and I'm looking into this ticket

comment:22 Changed 6 weeks ago by Ryan Cheley

Resolution: fixed
Status: assignedclosed

In looking at the tests in raw_query\tests I see a few tests that already exist:

These are already available and appear to be testing the very thing that the ticket is asking for.

Charlie DeTar, is there anything that is being missed here?

comment:23 Changed 6 weeks ago by Matias Surdi

Thanks for looking into this. I can get back to work now :)

comment:24 Changed 5 weeks ago by Shai Berger

Resolution: fixed
Status: closednew

Sorry -- this is still broken on Sqlite. The additions and fixes mentioned in comment:22 are mostly for Oracle and other backends. The Sqlite backend, at the time I write this, still has

    supports_paramstyle_pyformat = False

and still borks with a syntax error if you try to replicate the session in the ticket description.

comment:25 Changed 5 weeks ago by Ryan Cheley

Shai Berger can you confirm which version of SQLite you are using?

comment:26 in reply to:  25 Changed 5 weeks ago by Shai Berger

Replying to Ryan Cheley:

Shai Berger can you confirm which version of SQLite you are using?

Python 3.10.7 (main, Oct  1 2022, 04:31:04) [GCC 12.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import  sqlite3
>>> sqlite3.sqlite_version
'3.39.4'

(in the env where I checked)

comment:27 Changed 4 weeks ago by Ryan Cheley

I've done a bit of research and it looks like django.db.backends.sqlite3.base has a class called SQLiteCursorWrapper.

This in turn has a method convert_query which per the doc string of the SQLiteCursorWrapper method does:

Django uses "format" style placeholders, but pysqlite2 uses "qmark" style.
This fixes it -- but note that if you want to use a literal "%s" in a query,
you'll need to use "%%s".

I think the way to fix the issue is to update that method to include the following logic:

query  = re.sub("\)s", "",  re.sub("%\(", ":", query))

this will convert the string passed using the expectations of the ORM API to what SQLite is expecting for parameters, i.e. a passed in query will go from this

'select * from content_injuredlist where id =  %(id)s and app_name = %(app_name)s'

to this

'select * from content_injuredlist where id =  :id and app_name = :app_name'

which will then work with SQLite

Still to do:

  • Update the method
  • Write any tests
  • submit PR

comment:28 in reply to:  27 Changed 4 weeks ago by Shai Berger

Replying to Ryan Cheley:

I think the way to fix the issue is to update that method to include the following logic:

query  = re.sub("\)s", "",  re.sub("%\(", ":", query))

This looks quite fragile (consider, e.g. a query which, for whatever reason, includes )s in a string constant).
A more robust approach is to create a naming-dict from all the param-names, and use that to format the original text:

naming_dict = { param: f":{param}" for param in param_names}
query = query % naming_dict

Where param_names can be collected either by parsing the query, or from the parameters given.

(take a look at the Oracle backend -- I wrote that piece too long ago to remember which it takes, but at the time I knew what I was doing there)

comment:29 Changed 4 weeks ago by Simon Charette

I agree with Shai that we should avoid using regex here.

The most straightforward solution I can think of, I haven't looked at the Oracle implementation in details, would be to simply implement __getitem__

 class SQLiteParams:
    def __getitem__(self, param):
        return f":{param}"

sql = sql % SQLiteParams()

This will ensure the backend reports the low level missing param message in cases a parameter is missing instead of a KeyError during the Django translation phase

comment:30 Changed 4 weeks ago by Ryan Cheley

Thanks for the feed back Shai and Simon. I will take a look and see how I can work that into the solution.

comment:31 Changed 4 weeks ago by Mariusz Felisiak

Patch needs improvement: set
Status: newassigned

comment:32 Changed 4 weeks ago by Ryan Cheley

I've implemented the solution as outlined by Shia.

I'll be making changes to the documentation as suggested by Simon here https://github.com/django/django/pull/16243#pullrequestreview-1161175532 next

comment:33 Changed 3 weeks ago by Mariusz Felisiak

Needs tests: unset
Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:34 Changed 3 weeks ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: assignedclosed

In 8e6ea1d:

Fixed #10070 -- Added support for pyformat style parameters on SQLite.

Co-authored-by: Nick Pope <nick@…>

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