Opened 51 minutes ago
#37446 new Bug
DatabaseError ignored in Q.check() during validate_constraints()
| Reported by: | Jacob Walls | Owned by: | |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | 5.2 |
| Severity: | Normal | Keywords: | constraint, CheckConstraint |
| Cc: | Simon Charette | Triage Stage: | Unreviewed |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
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
diff --git a/django/db/models/query_utils.py b/django/db/models/query_utils.py index 279e153ac1..931f15a182 100644
a b class Q(tree.Node): (this hunk was shorter than expected) 175 175 """ 176 176 # Avoid circular imports. 177 177 from django.db.models import Value 178 from django.db.models.expressions import RawSQL 178 179 from django.db.models.sql import Query 179 180 from django.db.models.sql.constants import SINGLE 180 181 181 182 query = Query(None) 182 183 for name, value in against.items(): … … class Q(tree.Node): 198 200 return compiler.execute_sql(SINGLE) is not None 199 201 except DatabaseError as e: 200 202 logger.warning("Got a database error calling check() on %r: %s", self, e) 201 return True 203 # Be lenient: if any WHERE clause contained RawSQL, return True. 204 if any( 205 isinstance(ref, RawSQL) 206 for leaf in query.where.leaves() 207 for ref in leaf.flatten() 208 ): 209 return True 210 return False 202 211 203 212 def deconstruct(self): 204 213 path = "%s.%s" % (self.__class__.__module__, self.__class__.__name__)