Re: [Django] #34080: __exact lookup on nested arrays with None values fails on PostgreSQL.
Django
----------------------------------+------------------------------------
Reporter: Ion Alberdi | Owner: nobody
Type: Bug | Status: new
Component: contrib.postgres | Version: 4.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+------------------------------------
Comment (by Ion Alberdi):
Replying to [comment:1 Mariusz Felisiak]:
> Thanks for the report (see related
84633905273fc916e3d17883810d9969c03f73c2 and #27808). It's an issue in
[https://github.com/django/django/blob/84206607d6bfd61e7f7a88b51163ffd4153e3b5a/django/contrib/postgres/fields/array.py#L238-L252
ArrayRHSMixin].
Actually by running both test cases (bug=None nested array, fix=nested
array with one element that is not null.
{{{
def test_exact_nested_null(self):
instance =
NullableIntegerArrayModel.objects.create(field_nested=[[None, None],
[None, None]])
self.assertSequenceEqual(
NullableIntegerArrayModel.objects.filter(field_nested=[[None,
None], [None, None]]), [instance]
)
def test_exact_nested_null_and_not_null(self):
instance =
NullableIntegerArrayModel.objects.create(field_nested=[[None, 1], [None,
None]])
self.assertSequenceEqual(
NullableIntegerArrayModel.objects.filter(field_nested=[[None,
1], [None, None]]), [instance]
)
}}}
{{{
class ArrayRHSMixin:
def __init__(self, lhs, rhs):
if isinstance(rhs, (tuple, list)):
expressions = []
for value in rhs:
if not hasattr(value, "resolve_expression"):
field = lhs.output_field
value = Value(field.base_field.get_prep_value(value))
expressions.append(value)
rhs = Func(
*expressions,
function="ARRAY",
template="%(function)s[%(expressions)s]",
)
super().__init__(lhs, rhs)
}}}
By looking at the rhs variable in both scenarios:
- test_exact_nested_null(bug)
{{{
Func(Value([None, None]), Value([None, None]), function=ARRAY,
template=%(function)s[%(expressions)s])
}}}
- test_exact_nested_null_and_not_null(correct)
{{{
Func(Value([None, 1]), Value([None, None]), function=ARRAY,
template=%(function)s[%(expressions)s])
}}}
The rhs variables look correct to me. I'll continue digging, so far I tend
to go towards the psycopg2 bug hypothesis. Thanks a lot for these links
Marius,
https://github.com/psycopg/psycopg2/issues/325 seems indeed to be pretty
related to this one.
--
Ticket URL: <https://code.djangoproject.com/ticket/34080#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
----------------------------------+------------------------------------
Reporter: Ion Alberdi | Owner: nobody
Type: Bug | Status: new
Component: contrib.postgres | Version: 4.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+------------------------------------
Comment (by Mariusz Felisiak):
> The rhs variables look correct to me. I'll continue digging, so far I
tend to go towards the psycopg2 bug hypothesis.
As far as I'm aware it's not a `psycopg2` issue but PostgreSQL-behavior.
Regression tests work when we skip wrapping values in `ARRAY()` for
`None`-only arrays, so this should be fixable on Django side.
--
Ticket URL: <https://code.djangoproject.com/ticket/34080#comment:3>
Django
----------------------------------+------------------------------------
Reporter: Ion Alberdi | Owner: nobody
Type: Bug | Status: new
Component: contrib.postgres | Version: 4.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+------------------------------------
Comment (by Ion Alberdi):
I reproduce the bug in psycopg2
Given the values above, django generates queries as
{{{
'(ARRAY[%s, %s])::integer[][]', [[None, 1], [None, None]]'
}}}
with
{{{
def test_psycopg2_all_none(self):
with connection.cursor() as cursor:
cursor.execute("SELECT ARRAY[%s]::integer[][]", ([None,
None],))
}}}
I reproduce the bug in psycopg2 (2.9.3) as it fails with
{{{
invalid input syntax for type integer: "{NULL,NULL}"
LINE 1: SELECT ARRAY['{NULL,NULL}']::integer[][]
}}}
By removing the ArrayRHSMixin, the test added in the PR above passes.
However, by running the test_array suite with a PR that removes the
ArrayRHSMixin I get the following result
{{{
FAILED (failures=6, errors=12, expected failures=3)
}}}
vs
I'll open an issue in psycopg2 to get their point of view on
{{{
def test_psycopg2_all_none(self):
with connection.cursor() as cursor:
cursor.execute("SELECT ARRAY[%s]::integer[][]", ([None,
None],))
}}}
Thanks again Mariusz!
--
Ticket URL: <https://code.djangoproject.com/ticket/34080#comment:4>
Django
----------------------------------+------------------------------------
Reporter: Ion Alberdi | Owner: nobody
Type: Bug | Status: new
Component: contrib.postgres | Version: 4.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+------------------------------------
Comment (by Ion Alberdi):
Putting the link to https://github.com/psycopg/psycopg2/issues/1507 for
the sake of tracking.
--
Ticket URL: <https://code.djangoproject.com/ticket/34080#comment:5>
Django
----------------------------------+------------------------------------
Reporter: Ion Alberdi | Owner: nobody
Type: Bug | Status: new
Component: contrib.postgres | Version: 4.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+------------------------------------
Comment (by Ion Alberdi):
A PR is available at
https://github.com/django/django/pull/16175
--
Ticket URL: <https://code.djangoproject.com/ticket/34080#comment:6>
Django
Reporter: Ion Alberdi | Owner: Ion Alberdi
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 4.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by David Sanders):
* owner: nobody => Ion Alberdi
* status: new => assigned
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/34080#comment:7>
Django
----------------------------------+---------------------------------------
Reporter: Ion Alberdi | Owner: Ion Alberdi
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 4.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+---------------------------------------
Comment (by David Sanders):
Thanks for the patch Ion :)
I have some time today to take a look at this otherwise Mariusz or Carlton
will also take a look.
--
Ticket URL: <https://code.djangoproject.com/ticket/34080#comment:8>
Django
----------------------------------+---------------------------------------
Reporter: Ion Alberdi | Owner: Ion Alberdi
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 4.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+---------------------------------------
Comment (by David Sanders):
ok so just my 2¢ worth in investigating this:
Mix-n-matching `array[]` constructor with array literals `{}` seems to be
the issue which I reckon has to do with postgres not understanding how to
infer the appropriate types when nested and `NULL`s are present. In my
experience postgres generally won't do this for complex types including
json; see my related ticket #33905. The solution often involves just being
more explicit.
I tried manually altering the SQL Django has output for a sample model
with nested ArrayField with the following problematic output:
{{{
# SELECT *
FROM "ticket_34080_foo"
WHERE "ticket_34080_foo"."field_nested" = (ARRAY['{NULL}'])::integer[][]
LIMIT 21;
ERROR: invalid input syntax for type integer: "{NULL}"
LINE 3: WHERE "ticket_34080_foo"."field_nested" = (ARRAY['{NULL}'])...
^
}}}
Using `array[]` at both levels works:
{{{
# SELECT *
FROM "ticket_34080_foo"
WHERE "ticket_34080_foo"."field_nested" =
(ARRAY[ARRAY[NULL]])::integer[][]
LIMIT 21;
id | field | field_nested
----+-------+--------------
(0 rows)
}}}
Using an array literal for both levels works:
{{{
# SELECT *
FROM "ticket_34080_foo"
WHERE "ticket_34080_foo"."field_nested" = ('{{NULL}}')::integer[][]
LIMIT 21;
id | field | field_nested
----+-------+--------------
(0 rows)
}}}
Explicitly casting the literal also work when mixing with a constructor:
{{{
# SELECT *
FROM "ticket_34080_foo"
WHERE "ticket_34080_foo"."field_nested" =
(ARRAY['{NULL}'::integer[]])::integer[][]
LIMIT 21;
id | field | field_nested
----+-------+--------------
(0 rows)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34080#comment:9>
Django
----------------------------------+---------------------------------------
Reporter: Ion Alberdi | Owner: Ion Alberdi
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 4.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+---------------------------------------
Comment (by David Sanders):
ok so further looking into this it appears that the array constructor
doesn't try to infer types in literals at all even if the literal contains
only int; using an array literal within a constructor should always be
cast.
{{{
# SELECT *
FROM "ticket_34080_foo"
WHERE "ticket_34080_foo"."field_nested" = (ARRAY['{1}'])::integer[][]
LIMIT 21;
ERROR: invalid input syntax for type integer: "{1}"
LINE 3: WHERE "ticket_34080_foo"."field_nested" = (ARRAY['{1}'])::i...
^
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34080#comment:10>
Django
----------------------------------+---------------------------------------
Reporter: Ion Alberdi | Owner: Ion Alberdi
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 4.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+---------------------------------------
Comment (by David Sanders):
ok so…
after trailing through multiple threads here, psycopg2 and the ML I see
the issue with ArrayRHSMixin being used in an incompatible way with
psycopg2 :)
the stuff I reported above is interesting but can probably be ignored.
I've gone through the submitted PR and have some comments to make :)
--
Ticket URL: <https://code.djangoproject.com/ticket/34080#comment:11>
Django
----------------------------------+---------------------------------------
Reporter: Ion Alberdi | Owner: Ion Alberdi
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 4.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+---------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/34080#comment:12>
Django
Reporter: Ion Alberdi | Owner: Ion
| Alberdi
Type: Bug | Status: assigned
Component: contrib.postgres | Version: 4.1
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/34080#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: Ion Alberdi | Owner: Ion
| Alberdi
Component: contrib.postgres | Version: 4.1
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"3dc9f3ac6960c83cd32058677eb0ddb5a5e5da43" 3dc9f3ac]:
{{{
#!CommitTicketReference repository=""
revision="3dc9f3ac6960c83cd32058677eb0ddb5a5e5da43"
Fixed #34080 -- Fixed __exact lookup when nested arrays contain only NULL
values.
Thanks jerch and David Sanders for reviews.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34080#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: Ion Alberdi | Owner: Ion
| Alberdi
Component: contrib.postgres | Version: 4.1
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"34d63d5a41188a32b77733e33c1fad64f37c6772" 34d63d5a]:
{{{
#!CommitTicketReference repository=""
revision="34d63d5a41188a32b77733e33c1fad64f37c6772"
Refs #34080 -- Added tests for __exact lookup when non-nested arrays
contain only NULL values.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34080#comment:14>