Output field for combined PositiveIntegerField is not properly resolved.

[Toan Vuong]

Tested on Django 4.2.4.

My model is defined like this:

class Choice(models.Model):
    choice_text = models.CharField(max_length=200)
    votes = models.IntegerField(default=0)
    arbitrary_num = models.PositiveIntegerField(default=1)
    arbitrary_num2 = models.PositiveIntegerField(default=2)

And this query is failing with Oracle (Tested on 19.3), but not Postgres:

s = Coalesce(F('arbitrary_num') + F('arbitrary_num2'),
                 Value(0, PositiveIntegerField()))
case = Case(
        When(arbitrary_num=1, then=Value(1, PositiveIntegerField())),
        default=s,
        output_field=PositiveIntegerField()
)

qs = Choice.objects.annotate(ss=case)
list(qs)

#qs = Choice.objects.annotate(s=s)
#list(qs)

The error (It's talking about the Coalesce function not having an output_field):

django.core.exceptions.FieldError: Expression contains mixed types: IntegerField, PositiveIntegerField. You must set output_field.

I believe the error is correct, because I think adding two PositiveIntegerField() seem to result in an IntegerField, so we *need* to specify an output_field on Coalesce to tell Django the proper output type to use. My question is that this seem to only throw an error in Oracle, and *not* Postgres. So it seems like the behavior on Postgres is unexpected? Somehow, the Case statement swallows the exception in Postgres and generates correct result. This can be demonstrated by the queryset where the Coalesce function is annotated directly without a case statement -- in this case, both Postgres and Oracle fails with the above error.

Note that this used to work prior to 4.2 (we upgraded from 3.x, where this used to work), although again I believe it was incorrect behavior and an output_field *should* be specified.

On the other hand, it's a little strange that adding two PositiveIntegerField() results in an output type of IntegerField(), so perhaps that's a bug as well...

[Mariusz Felisiak]

Thanks for the ticket. This is not strictly related with Oracle, the main issue is that output_field is not properly resolved for a combination of PositiveIntegerField. Coalesce() on Oracle checks the output_field of arguments and crashes. We should be able to fix this but prioritize PositiveIntegerField, e.g.

django/db/models/expressions.py

@@ -512 +512 @@
 
 _connector_combinations = [
     # Numeric operations - operands of same type.
+    {
+        # PositiveIntegerField should take precedence over IntegerField.
+        connector: [
+            (
+                fields.PositiveIntegerField,
+                fields.PositiveIntegerField,
+                fields.PositiveIntegerField,
+            ),
+        ]
+        for connector in (
+            Combinable.ADD,
+            Combinable.MUL,
+            Combinable.DIV,
+            Combinable.MOD,
+            Combinable.POW,
+        )
+    },
     {
         connector: [
             (fields.IntegerField, fields.IntegerField, fields.IntegerField),

Would you like to prepare a patch (via GitHub PR)? a regression test is required.

Regression in 40b8a6174f001a310aa33f7880db0efeeb04d4c4 (#33397).

[Toan Vuong]

Thanks for the reply! I can work on a PR for the PositiveIntegerField change following the patch you wrote.

However, regarding your statement:

Coalesce() on Oracle checks the output_field of arguments and crashes

This seems incorrect, because using Coalesce alone would crash on both Postgres and Oracle. The weird thing is if I have a Case on top of the "bad" Coalesce, then only Oracle crashes. The Postgres queryset executes successfully. So it seems like Case behaves differently between the two, and probably incorrectly swallows the error thrown by Coalesce in the Postgres scenario which also seems like a bug?

[Toan Vuong]

Opened a PR:
Edit: ​https://github.com/django/django/pull/17299 (Merging to main)

Still wondering about whether we need to do something about Case between Postgres vs. Oracle, because the behavior still seems inconsistent there.

Edit: Also not sure what the process is to open PRs for other branches(?). Seems like I should also merge this to main and stable/5.0.x?
Edit 2: Per the ​first PR's comment, I recreated a PR for main only.

[Mariusz Felisiak]

Replying to Toan Vuong:

Still wondering about whether we need to do something about Case between Postgres vs. Oracle, because the behavior still seems inconsistent there.

Coalesce() on Oracle ​explicitly checks the output_field when it's compiled (for Cast.default), on other database the output_field is not used when Coalesce() is compiled so it doesn't crash. The crash on Oracle is a side-effect of the current implementation, IMO, there is no need to change it.

Edit: Also not sure what the process is to open PRs for other branches(?). Seems like I should also merge this to main and stable/5.0.x?

Mergers will backport if needed.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 4de31ec:

Fixed #34858 -- Corrected resolving output_field for PositiveIntegerField.

Regression in 40b8a6174f001a310aa33f7880db0efeeb04d4c4.

[Mariusz Felisiak <felisiak.mariusz@…>]

In dcd3a03:

[5.0.x] Fixed #34858 -- Corrected resolving output_field for PositiveIntegerField.

Regression in 40b8a6174f001a310aa33f7880db0efeeb04d4c4.

Backport of 4de31ec680df062e5964b630f1b881ead5004e15 from main

[Toan Vuong]

Coalesce() on Oracle ​explicitly checks the output_field when it's compiled (for Cast.default), on other database the output_field is not used when Coalesce() is compiled so it doesn't crash. The crash on Oracle is a side-effect of the current implementation, IMO, there is no need to change it.

Thanks, this is what I was missing. In our local copy of Django I essentially added a try/catch on that as_oracle function which worked, but I didn't realize that was only called in some cases but not others.