Cast regression in PostgresSQL using built-in lookups

[Peter J. Farrell]

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:

​https://github.com/django/django/commit/658f1e81a7ea29acf30bbf8f61ffec2beff24639#diff-8e3817dff3483fe3eaecb84d56803e32

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

[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?

[Peter J. Farrell]

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.

[Simon Charette]

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(key)) on small datasets or relying on a ​GIN indexed functional lookup instead on large ones.

Anyway, here's the ​PR.

[Peter J. Farrell]

Replying to Simon Charette:

Anyway, here's the ​PR.

Thanks for the fix.

[Simon Charette <charette.s@…>]

In a354c690:

Fixed #28038 -- Restored casting to text of builtin lookups on PostgreSQL.

Reverted 658f1e8 which broke code using icontains's implicit cast to ::text
on ArrayField.

Thanks Peter J. Farrell for the report.

[Simon Charette <charette.s@…>]

In 5d35e8eb:

[1.11.x] Fixed #28038 -- Restored casting to text of builtin lookups on PostgreSQL.

Reverted 658f1e8 which broke code using icontains's implicit cast to ::text
on ArrayField.

Thanks Peter J. Farrell for the report.

Backport of a354c69055fd818e612ce22eaa2da0576a4b89ee from master