#28038 closed Bug (fixed)
Cast regression in PostgresSQL using built-in lookups
Reported by: | Peter J. Farrell | Owned by: | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 1.11 |
Severity: | Release blocker | Keywords: | postgres postgresql array_agg |
Cc: | 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
The following query works in Django 1.10 or less but fails on Django 1.11 with the following error message:
function upper(character varying[]) does not exist... No function matches the given name and argument types. You might need to add explicit type casts.
query = self.filter( category=category ).annotate( arm_types=ArrayAgg('assets__arm_type') ).filter( arm_types__icontains=arm_type )
The change is that the filter using the icontains no longer casts to ::text. It appears this is regression that is caused by this commit:
The SQL generated from Django 1.10 looks like this (wheres 1.11 no longer includes the ::text cast):
UPPER(ARRAY_AGG("asset_furnitureasset"."arm_type")::text) LIKE UPPER('%Without Arms%') ORDER BY "asset_furniture"."make_model" ASC
Change History (7)
follow-up: 2 comment:1 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:2 by , 8 years ago
Replying to Simon Charette:
I don't have the time to look at this right but it looks like a bug in your code IMHO. Wouldn't using
StringAgg
be more appropriate here?
It's definitely a regression from the behavior in 1.10 and a blocker to upgrade apps. We would have to find all the usages of ArrayAgg
and convert them. In this case, we haven't found any lookups work in conjunction with ArrayAgg
in Django 1.11 whereas they worked in 1.10. It would have to be documented that which lookups work with ArrayAgg
in Django 1.11.
Plus, it much easier to iterator over the array/list of arm_types than having to convert a delimited string into something that can be iterated over.
follow-up: 5 comment:3 by , 8 years ago
Has patch: | set |
---|---|
Owner: | removed |
Severity: | Normal → Release blocker |
Status: | assigned → new |
Triage Stage: | Unreviewed → Accepted |
It's a shame we expose builtin lookups such as __icontains
on these kind of fields as it produces really inefficient SQL, a real footgun API if you ask me.
I think we'd be doing developers a favor in erroring instead and forcing them to choose an implementation that best fit their need. That is using something along EXISTS(SELECT 1 FROM unnest(array_field) key WHERE UPPER(key) = UPPER('lookup'))
on small datasets or relying on a GIN indexed functional lookup instead on large ones.
Anyway, here's the PR.
comment:4 by , 8 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
I don't have the time to look at this right but it looks like a bug in your code IMHO. Wouldn't using
StringAgg
be more appropriate here?