DatabaseError ignored in Q.check() during validate_constraints()

[Jacob Walls]

A pragmatic compromise was done in 5d91dc8ee3ff7e2230cfacf41005f61b8efbf057 (#35081) to log and swallow a DatabaseError that arises during constraint validation, to support the case where a RawSQL expression in a constraint cannot be translated to SQL.

Still, there are cases like this reproducer without RawSQL (​dry ORM fiddle) where I would expect an error:

from django.db import models
from django.db.models.functions import Cast
from django.db.models import CheckConstraint
from django.db.models.lookups import GreaterThan
from django.db.models.fields.json import KT


class Order(models.Model):
    data = models.JSONField()

    class Meta:
        constraints = [
            models.CheckConstraint(
                condition=GreaterThan(
                    Cast(KT("data__quantity"), models.IntegerField()),
                    0
                ),
                name="positive_quantity",
            )
        ]


def run():
    instance = Order(data={"quantity": "not an int"})
    instance.validate_constraints()
    assert False, "see logged content above"

The above snippet should have failed validation, but it succeeds, and then fails on assert False where you can see this logged output in the fiddle:

Got a database error calling check() on <Q: (AND: Coalesce(GreaterThan(Cast(KeyTextTransform(F(data))), 0), Value(True)))>: invalid input syntax for type integer: "not an int"

---

Swallowing the error was questioned later during triage of #34840, and in ticket:34840#comment:19, Simon asked that we "not forget" about coming back to this.

I think we should avoid swallowing the error, while still being lenient for RawSQL. Maybe something like this, although I'm sure there are interesting nested cases to check:

django/db/models/query_utils.py

@@ -175 +175 @@
         """
         # Avoid circular imports.
         from django.db.models import Value
+        from django.db.models.expressions import RawSQL
         from django.db.models.sql import Query
         from django.db.models.sql.constants import SINGLE
 
         query = Query(None)
         for name, value in against.items():
@@ -198 +200 @@
                 return compiler.execute_sql(SINGLE) is not None
         except DatabaseError as e:
             logger.warning("Got a database error calling check() on %r: %s", self, e)
-            return True
+            # Be lenient: if any WHERE clause contained RawSQL, return True.
+            if any(
+                isinstance(ref, RawSQL)
+                for leaf in query.where.leaves()
+                for ref in leaf.flatten()
+            ):
+                return True
+            return False
 
     def deconstruct(self):
         path = "%s.%s" % (self.__class__.__module__, self.__class__.__name__)

[Jacob Walls]

Copying a portion of Simon's ticket:34840#comment:19:

Maybe a possible alternative solution that doesn't warrant expression introspection (e.g. special casing RawSQL) could be a Constraint kwarg that allows disabling model level validation entirely? That could be a way to silence the warning during the deprecation period.

I like this -- no special case, and no need to visit every nested expression.

[SnippyCodes]

I'd like to work on this! Before opening a PR, could you confirm whether we should proceed with the RawSQL leaf inspection in Q.check() as shown in the description diff, or if we prefer pursuing Simon's suggestion in comment:2 (adding a kwarg on Constraint to disable model-level validation / deprecating swallowing DatabaseError for RawSQL)?