[Django] #28880: The QuerySet.query sql representation of an ORM call on a PostgreSQL ArrayField is missing a key bit of syntax.

8 views
Skip to first unread message

Django

unread,
Dec 3, 2017, 4:39:01 PM12/3/17
to django-...@googlegroups.com
#28880: The QuerySet.query sql representation of an ORM call on a PostgreSQL
ArrayField is missing a key bit of syntax.
-------------------------------------+-------------------------------------
Reporter: Alexander | Owner: (none)
Kavanaugh |
Type: Bug | Status: new
Component: | Version: 1.11
contrib.postgres | Keywords:
Severity: Normal | postgres,arrayfield,syntaxerror,.query,as_sql,ARRAY
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------+-------------------------------------
Representations of Arrays in PostgreSQL operations are preceeded by the
word ARRAY (e.g. `ARRAY['3', '4', '5']`; see also
[https://www.postgresql.org/docs/current/static/functions-array.html array
operation docs]. The output of `QuerySet.query` is missing "ARRAY" -- it
currently just injects a python list representation into the rhs of the
operation sql string.

It is worth noting that actual operations on ArrayFields use the correct
syntax; only the `QuerySet.query` representation is incorrect. The key
ramification of this issue is that copying and pasting the
`QuerySet.query` output into a psql shell and running it results in a
SyntaxError (`ERROR: syntax error at or near "["`). This can be quite
confusing for people unfamiliar with PostgreSQL array syntax (like me,
before I dug into this) attempting to troubleshoot their code.

I'll happily work on a fix and submit a PR if I can get some guidance. I'm
assuming the fix would need to be somewhere along the
[https://github.com/django/django/blob/master/django/contrib/postgres/lookups.py#L7
postgres Lookup as_sql] code path, but I'm not sure what an elegant
solution would be. Is overriding `process_rhs` the right move?


Relevant bit of the Django Model:
{{{#!python
class Message(Model):
network_lookup_ids = ArrayField(base_field=CharField(max_length=160))
}}}

Django ORM Code:
{{{#!python
Message.objects.filter(network_lookup_ids__overlap=["3", "4",
"5"]).values("id", "network_lookup_ids")
}}}

Django QuerySet.query representation:
{{{#!sql
SELECT "production_message"."id",
"production_message"."network_lookup_ids" FROM "production_message" WHERE
"production_message"."network_lookup_ids" && ['3', '4',
'5']::varchar(160)[]
}}}

Actual call made to the database (from the `pg_stat_activity` table):
{{{#!sql
SELECT "production_message"."id",
"production_message"."network_lookup_ids" FROM "production_message" WHERE
"production_message"."network_lookup_ids" && ARRAY['3', '4',
'5']::varchar(160)[] LIMIT 21
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/28880>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Dec 3, 2017, 5:34:14 PM12/3/17
to django-...@googlegroups.com
#28880: The QuerySet.query sql representation of an ORM call on a PostgreSQL
ArrayField is missing a key bit of syntax.
-------------------------------------+-------------------------------------
Reporter: Alexander Kavanaugh | Owner: (none)
Type: Bug | Status: new
Component: contrib.postgres | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage:
postgres,arrayfield,syntaxerror,.query,as_sql,ARRAY| Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Curtis Maloney):

It's important to remember that calling str(queryset.query) has not yet
passed the query to the DB-API driver for proper escaping and type
coercion, so it's not always _exactly_ what's sent to the DBMS.

Although ArrayField is postgres only currently, should some other DBMS
support it, and Django add support for their syntax, forcing the repr to
use Postgres's syntax would only result in someone raising a similar
ticket.

--
Ticket URL: <https://code.djangoproject.com/ticket/28880#comment:1>

Django

unread,
Dec 3, 2017, 9:26:00 PM12/3/17
to django-...@googlegroups.com
#28880: The QuerySet.query sql representation of an ORM call on a PostgreSQL
ArrayField is missing a key bit of syntax.
-------------------------------------+-------------------------------------
Reporter: Alexander Kavanaugh | Owner: (none)
Type: Bug | Status: new

Component: contrib.postgres | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage:
postgres,arrayfield,syntaxerror,.query,as_sql,ARRAY| Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Alexander Kavanaugh):

> It's important to remember that calling str(queryset.query) has not yet
passed the query to the DB-API driver for proper escaping and type
coercion, so it's not always _exactly_ what's sent to the DBMS.

Fair point. See related #17741

--
Ticket URL: <https://code.djangoproject.com/ticket/28880#comment:2>

Django

unread,
Dec 3, 2017, 9:27:07 PM12/3/17
to django-...@googlegroups.com
#28880: The QuerySet.query sql representation of an ORM call on a PostgreSQL
ArrayField is missing a key bit of syntax.
-------------------------------------+-------------------------------------
Reporter: Alexander Kavanaugh | Owner: (none)
Type: Bug | Status: closed
Component: contrib.postgres | Version: 1.11
Severity: Normal | Resolution: invalid

Keywords: | Triage Stage:
postgres,arrayfield,syntaxerror,.query,as_sql,ARRAY| Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Alexander Kavanaugh):

* status: new => closed
* resolution: => invalid


--
Ticket URL: <https://code.djangoproject.com/ticket/28880#comment:3>

Reply all
Reply to author
Forward
0 new messages