#36133 closed Bug (invalid)
Passing expression with wrongly attributed text output_field to Concat crashes on Postgres due the lack of casting
Reported by: | Siburg | Owned by: | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 5.1 |
Severity: | Normal | Keywords: | Cast, ExpressionWrapper |
Cc: | Siburg | Triage Stage: | Unreviewed |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
It seems that output_field
for an ExpressionWrapper
no longer works in Django 5.1 as in previous versions. Example code and test is below.
class PartnerQuerySet(models.QuerySet): def order_by_nick_name(self): qs = self.annotate( nick_name_for_ordering=Case( When( nick_name='', # The starting 'zzzzzzzz' is intended to ensure they # will be after records with nicknames. As ugly # as it is, this should work on all databases. # Subtracting the pk from a large number provides a # reverse ordering by pk. Let's assume we never # reach 900,000 records. then=Concat( Value('zzzzzzzz'), ExpressionWrapper(999_999 - F('pk'), output_field=models.CharField()), # This is probably a bug in Django 5.1, but it no longer # worked when using `CharField` as output_field for the # ExpressionWrapper. Amazingly, the 2-stage casting # does work. # Cast( # ExpressionWrapper(999_999 - F('pk'), output_field=models.BigAutoField()), # output_field=models.CharField() # ), ), ), default=Lower('nick_name'), ) ) return qs.order_by('nick_name_for_ordering') class Partner(models.Model): name = models.CharField(max_length=128, unique=True) nick_name = models.CharField(max_length=64, default='', blank=True) objects = PartnerQuerySet().as_manager() class PartnerQuerySetTests(TestCase): def test_ordering_by_nick_name(self): # Given aaron = Partner.objects.create(name='aaron', nick_name='Zorro') bert = Partner.objects.create(name='bert') zelda = Partner.objects.create(name='Zelda', nick_name='ace') ernie = Partner.objects.create(name='Ernie') # When qs = Partner.objects.order_by_nick_name() # Then self.assertEqual(list(qs), [zelda, aaron, ernie, bert]) # And self.assertEqual(qs[2].nick_name_for_ordering, 'zzzzzzzz999995')
I'm not especially proud of that code, but it worked in Django 4.2 and 5.0 and passed the test. It fails in 5.1. My workaround for it, wrapping the ExpressionWrapper
itself in a Cast
solves the problem; as in the commented out snippet above. However, I don't see why that should have become necessary.
I don't know what causes this change in behaviour. I think this ticket may be related to https://code.djangoproject.com/ticket/26650
Change History (6)
comment:1 by , 4 weeks ago
comment:2 by , 4 weeks ago
In order to help with triaging could you please provide the exception you are encountering and the database backend you as using.
Thank you for fast response. I'm running it on PostgreSQL. Exception from the test is
psycopg.errors.InvalidTextRepresentation: invalid input syntax for type bigint: "" LINE 1: ... '') || COALESCE((999999 - "calls_partner"."id"), '')) ELSE ...
From various trials and errors that I undertook I learned that the problem is not actually with the COALESCE though. The problem is caused by the ExpressionWrapper
.
comment:3 by , 4 weeks ago
Well ExpressionWrapper
should not change the SQL generated at all so unless you are getting a Django level crash about issues resolving output_field
, which doesn't seem to be case here, there is likely something else at play.
Could you provide the SQL that was previously generated, your Postgres version (if it changed between Django upgrades), and which version of psycopg2
or psycopg
you are using?
comment:4 by , 4 weeks ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
The issue you are running into has more to do with Concat
changes than ExpressionWrapper
The pre-5.1 SQL was along the lines of
CONCAT(('zzzzzzzz')::text, ((999999 - "test_34444_partner"."id"))::text)
and the post 5.1 SQL is
COALESCE('zzzzzzzz', '') || COALESCE((999999 - "test_34444_partner"."id"), '')
due to the change to string concatenation on Postgres to the immutable ||
in #34955.
The reason why your code broke is that you explicitly tell the ORM that ExpressionWrapper(999_999 - F('pk'), output_field=models.CharField())
resolves to CharField
when it actually resolves to BigIntegerField
which prevents the ORM from implicitly applying adequate casting.
Simply removing all usage of ExpressionWrapper
should solve your issue as it's doing more harm than good in its current form.
Concat( Value("zzzzzzzz"), 999_999 - F("pk"), output_field=models.CharField(), )
comment:5 by , 4 weeks ago
Summary: | ExpressionWrapper output_field no longer works → Passing expression with wrongly attributed text output_field to Concat crashes on Postgres due the lack of casting |
---|
comment:6 by , 4 weeks ago
Thank you. I'm using PostgreSQL 14 and psycopg == 3.2.4 . Just tried it out and the test passes with SQLite, so it is indeed a PostgreSQL thing.
I appreciate your simplification of removing the ExpressionWrapper
altogether. Happy to confirm that indeed works, and will adopt that in my code.
But, still surprised by the unexpected breakage in Django 5.1. I naively expected that specifying output_field=models.CharField()
for an ExpressionWrapper
would lead to the intended result. I think that is the gist of https://code.djangoproject.com/ticket/26650 as well. So it's annoying that such a specification is useless, and the ExpressionWrapper
will provide a bigint
anyway. Perhaps it would be good to clarify the documentation in https://docs.djangoproject.com/en/5.1/ref/models/expressions/#output-field.
Thinking a little more about it, I realize that output_field
is a very trick thing and can depend on the database. For example, I am aware from painful experience that SQLite causes problems with DecimalField
. So I don't have any clear solution myself for this breakage in 5.1.
In order to help with triaging could you please provide the exception you are encountering and the database backend you as using.