Case() and ExpressionWrapper() doesn't work with DecimalField on SQLite.

[Matthijs Kooijman]

I noticed that, on sqlite, some comparisons against DecimalField annotations behave unexpectedly, in particular when wrapping a DecimalField value in a Case/When or ExpressionWrapper. I suspect that there might be some inconsistencies in the type conversions here somehow.

I've created a few testcase to illustrate the problem on current git main: ​https://github.com/matthijskooijman/django/commit/3470b98b42c39fd9a9a4e1443341f16780da7a98 and see below.

    @override_settings(DEBUG=True)                                                                                      
    def test_00compare_field(self):                                                                                     
        """Comparing a Case annotation wrapping a field to a literal works."""                                          
        Foo.objects.create(a='', d=1)                                                                                   
        try:                                                                                                            
            Foo.objects.filter(d__gt=0).get()                                                                           
        finally:                                                                                                        
            from django.db import connection                                                                            
            print(connection.queries[-1]['sql'])                                                                        
                                                                                                                        
    @override_settings(DEBUG=True)                                                                                      
    def test_01compare_annotation_value_literal(self):                                                                  
        """Comparing a literal annotation using Value to a literal works."""                                            
        # Fields are not actually used here                                                                             
        Foo.objects.create(a='', d=0)                                                                                   
        try:                                                                                                            
            Foo.objects.annotate(                                                                                       
                x=models.Value(1, output_field=models.fields.DecimalField(max_digits=1, decimal_places=0)),             
            ).filter(x__gt=0).get()                                                                                     
        finally:                                                                                                        
            from django.db import connection                                                                            
            print(connection.queries[-1]['sql'])                                                                        
                                                                                                                        
    @override_settings(DEBUG=True)                                                                                      
    def test_02compare_annotation_expressionwrapper_literal(self):                                                      
        """Comparing a literal annotation using ExpressionWraper and Value to a literal works."""                       
        # Fields are not actually used here                                                                             
        Foo.objects.create(a='', d=0)                                                                                   
        try:                                                                                                            
            Foo.objects.annotate(                                                                                       
                x=models.ExpressionWrapper(                                                                             
                    models.Value(1),                                                                                    
                    output_field=models.fields.DecimalField(max_digits=1, decimal_places=0),                            
                ),                                                                                                      
            ).filter(x__gt=0).get()                                                                                     
        finally:                                                                                                        
            from django.db import connection                                                                            
            print(connection.queries[-1]['sql'])                                                                        
                                                                                                                        
    @override_settings(DEBUG=True)                                                                                      
    def test_03compare_case_annotation(self):                                                                           
        """Comparing a Case annotation wrapping a field to a literal works."""                                          
        Foo.objects.create(a='', d=1)                                                                                   
        try:                                                                                                            
            Foo.objects.annotate(                                                                                       
                x=models.Case(models.When(a='', then=models.F('d'))),                                                   
            ).filter(x__gt=0).get()                                                                                     
        finally:                                                                                                        
            from django.db import connection                                                                            
            print(connection.queries[-1]['sql'])      

• test_00compare_field compares a field directly with a literal, which works.
• test_01compare_annotation_value_literal adds a literal annotation using just Value and then compares it, which also works.
• test_02compare_annotation_expressionwrapper_literal adds a literal annotation using Value wrapped in ExpressionWrapper, which does not work becomes a literal int, rather than a string like the compared value.
• test_03compare_case_annotation wraps the field in a case/when and then compares it, which also does not work (maybe the CASE changes the type?)

Running these testcases against sqlite gives:

SELECT "model_fields_foo"."id", "model_fields_foo"."a", "model_fields_foo"."d" FROM "model_fields_foo" WHERE "model_fields_foo"."d" > '0' LIMIT 21
.SELECT "model_fields_foo"."id", "model_fields_foo"."a", "model_fields_foo"."d", CAST('1' AS NUMERIC) AS "x" FROM "model_fields_foo" WHERE CAST('1' AS NUMERIC) > '0' LIMIT 21
.SELECT "model_fields_foo"."id", "model_fields_foo"."a", "model_fields_foo"."d", 1 AS "x" FROM "model_fields_foo" WHERE 1 > '0' LIMIT 21
ESELECT "model_fields_foo"."id", "model_fields_foo"."a", "model_fields_foo"."d", CASE WHEN "model_fields_foo"."a" = '' THEN "model_fields_foo"."d" ELSE NULL END AS "x" FROM "model_fields_foo" WHERE CASE WHEN ("model_fields_foo"."a" = '') THEN "model_fields_foo"."d" ELSE NULL END > '0' LIMIT 21
E.s...........
======================================================================
ERROR: test_02compare_annotation_expressionwrapper_literal (model_fields.test_decimalfield.DecimalFieldTests)
Comparing a literal annotation using ExpressionWraper and Value to a literal works.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/matthijs/docs/src/upstream/django/django/test/utils.py", line 437, in inner
    return func(*args, **kwargs)
  File "/home/matthijs/docs/src/upstream/django/tests/model_fields/test_decimalfield.py", line 142, in test_02compare_annotation_expressionwrapper_literal
    Foo.objects.annotate(
  File "/home/matthijs/docs/src/upstream/django/django/db/models/query.py", line 441, in get
    raise self.model.DoesNotExist(
model_fields.models.Foo.DoesNotExist: Foo matching query does not exist.

======================================================================
ERROR: test_03compare_case_annotation (model_fields.test_decimalfield.DecimalFieldTests)
Comparing a Case annotation wrapping a field to a literal works.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/matthijs/docs/src/upstream/django/django/test/utils.py", line 437, in inner
    return func(*args, **kwargs)
  File "/home/matthijs/docs/src/upstream/django/tests/model_fields/test_decimalfield.py", line 154, in test_03compare_case_annotation
    Foo.objects.annotate(
  File "/home/matthijs/docs/src/upstream/django/django/db/models/query.py", line 441, in get
    raise self.model.DoesNotExist(
model_fields.models.Foo.DoesNotExist: Foo matching query does not exist.

----------------------------------------------------------------------

Note in the printed queries that the 0 value that is compared with is a string in the query ('0'), while the ExpressionWrappered value is just a plain number (1). The Value without ExpressionWrapper has a cast to NUMERIC that I suspect makes that case work?

Also note that the DEBUG stuff and try-catch is only added to capture the query that is being generated, it is not needed to trigger the problem. I tried printing the QuerySet.query attribute first, but that seems to hide the quotes around the literal 0 in the query, so took me a while to realize what was happening. The numbers in the testcase names are to ensure the SQL queries are printed in a recognizable order.

All four testcases work on MySQL, there the 0 value is written in the query without the quotes, and the cast to NUMERIC is also missing.

I suspect this issue is highly similar to #18247, and can a fix can probably build on the fix for that issue.

[Matthijs Kooijman]

It seems that adding SQLiteNumericMixin to Case and ExpressionWrapper actually fixes this, with the below patch all my testcases pass:

{{{diff
--- a/django/db/models/expressions.py
+++ b/django/db/models/expressions.py
@@ -933,7 +933,7 @@ def as_sqlite(self, compiler, connection, extra_context):

return self.as_sql(compiler, connection, extra_context)

-class ExpressionWrapper(Expression):
+class ExpressionWrapper(SQLiteNumericMixin, Expression):

"""
An expression that can wrap another expression so that it can provide
extra context to the inner expression, such as the output_field.

@@ -1032,7 +1032,7 @@ def get_group_by_cols(self, alias=None):

return cols

-class Case(Expression):
+class Case(SQLiteNumericMixin, Expression):

"""
An SQL searched CASE expression:

}}}

I wonder if this mixing should be added to all expressions, so maybe to Expression or BaseExpression? I quickly tried, but got django.db.utils.OperationalError: near "WHEN": syntax error though.

[Mariusz Felisiak]

Thanks for the detailed report!

It seems that adding SQLiteNumericMixin to Case and ExpressionWrapper actually fixes this, with the below patch all my testcases pass:

Yes, that should fix the issue (see related #31723 and #32585).

I wonder if this mixing should be added to all expressions, so maybe to Expression or BaseExpression?

This can be more complicated because "partial" expressions as When() (WHEN ... THEN ...) cannot be wrapped.

[Matthijs Kooijman]

Thanks for confirming that SQLiteNumericMixin is the right approach here. I'll try to find some time to prepare a PR with that.

Any suggestions on where the testcases for this should live? I put them at model_fields/decimal_field now, but they also relate to SQLite, query expressions, and maybe other SQLite numeric fields, so maybe there is a better place for them?

I also had a look through expressions.py, and it seems that most expressions already have SQLiteNumericMixin, except:

• Case & ExpressionWrapper: These break without, so should get it.
• When: This is a partial expression, so probably does not need it.
• Subquery: I suspect this also needs it, but I'll try to make a testcase that breaks it first.
• OrderBy: I'm not sure about this one, I couldn't find any docs on how this is used exactly..
• WindowFrame: This looks like a partial expression to me that doesn't need it?

[Mariusz Felisiak]

Any suggestions on where the testcases for this should live?

• Case -> tests/expressions_case,

• ExpressionWrapper -> tests/expressions

I also had a look through expressions.py, and it seems that most expressions already have SQLiteNumericMixin, except:

I agree with you analysis: When, WindowFrame, and OrderBy don't need it. I'm not sure about Subquery, you can try to break it, but as far as I'm aware it returns a single column/expression so casting to a numeric value should be already handled there.

[Matthijs Kooijman]

I agree with you analysis: When, WindowFrame, and OrderBy don't need it.

Ok, I left those out.

I'm not sure about Subquery, you can try to break it, but as far as I'm aware it returns a single column/expression so casting to a numeric value should be already handled there.

I tried, and could indeed not break Subquery.

Pullrequest is here: ​https://github.com/django/django/pull/15062

Regarding Case and ExpressionWrapper, I noticed a small difference between them: ExpressionWrapper with output_field=DecimalField fails to convert a non-decimal value to decimal (but works if the value is already a Decimal literal or DecimalField), while Case actually breaks when applied to existing decimal values (I suspect that sqlite does some type conversion when executing the query). I now put both fixes in the same commit, if you prefer separate commits, let me know.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 1a502388:

Fixed #33257 -- Fixed Case() and ExpressionWrapper() with decimal values on SQLite.