Opened 9 years ago
Last modified 9 years ago
#25579 closed Bug
Lack of type adaptation in ArrayField querying/lookups — at Version 8
Reported by: | Matt C | Owned by: | Matt C |
---|---|---|---|
Component: | contrib.postgres | Version: | 1.8 |
Severity: | Normal | Keywords: | ArrayField query lookup |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
This commit: https://github.com/django/django/commit/39d95fb6ada99c59d47fa0eae6d3128abafe2d58 replaces the get_prep_value
method with get_db_prep_value
and I believe this causes a regression with regards to querying on an ArrayField.
get_db_prep_value
is never called during querying/lookup, hence the array items/values are passed directly to the low-level database Python wrappers without first being transformed (from Python objects to SQL-friendly parameters).
See this question: http://stackoverflow.com/questions/33250371/arrayfield-class-missing-query-lookup-methods
Could both the get_prep_value
and get_db_prep_value
methods be overridden with duplicate/shared logic to overcome this issue?
Patch pull request: https://github.com/django/django/pull/6278
Change History (8)
comment:1 by , 9 years ago
comment:2 by , 9 years ago
#25143 is to do with converting DB query result values to Python values, whilst this is to do with converting Python values to DB values, for the purpose of lookups/querying.
I'll change the title to remove the word regression.
comment:3 by , 9 years ago
Summary: | ArrayField query/lookup regression → Lack of type adaptation in ArrayField querying/lookups |
---|
comment:4 by , 9 years ago
Triage Stage: | Unreviewed → Accepted |
---|
I can reproduce "can't adapt type 'Tag'" by adding list(OtherTypesArrayModel.objects.filter(tags=Tag(1)))
to the PR from #25143.
comment:5 by , 9 years ago
I've created a pull request for this, but I'm not sure if I need to change any of the configuration (e.g. "Has patch" and whether I should take ownership) for this ticket, or if that's done by the core devs.
comment:6 by , 9 years ago
Please assign this ticket to yourself to prevent parallel work, check Has Patch and preferably link to your PR.
The only thing you can't do is mark your own patch as Ready for checkin.
comment:7 by , 9 years ago
Has patch: | set |
---|---|
Owner: | set to |
Status: | new → assigned |
Thanks @charettes.
I wasn't sure if linking to the pull request was necessary, because it seemed to happen automatically in the details box at the top, but here is the link anyway:
comment:8 by , 9 years ago
Description: | modified (diff) |
---|
Looks similar to #25143 but I think it's a separate issue. I'm not sure about the characterization of the issue as a regression since
ArrayField
was never present in a stable release without the commit you pointed out, but it might be okay to backport anyway under the "bug in a new feature" rationale.