#34231 closed Bug (invalid)
Invalid RawSQL expression on Model.validate_constraints
Reported by: | Manuel Raynaud | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 4.1 |
Severity: | Normal | Keywords: | check constraint rawsql |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Since django 4.1, contrainst check are made during model validation.
If we are using a CheckConstraint and the check is using a RawSQL() expression, the generated SQL is invalid. The FROM clause is missing in the generated SQL leading to non existing columns errors.
In commit 667105877e6723c6985399803a364848891513cc a check is also added (models.W045) explaining that RawSQL() expression are not validated during full_clean. Reading this I understand that constraints using RawSQL() expression should be ignored and it is not the case.
Here an example to reproduce:
class Person(models.Model): first_name = models.CharField( max_length=30, blank=True, null=True, ) last_name = models.CharField( max_length=30, blank=True, null=True, ) class Meta: db_table = "person" constraints = [ models.CheckConstraint( name="check", check=models.expressions.RawSQL( "(first_name IS NULL) = (last_name IS NULL)", {}, models.fields.BooleanField(), ), ), ] person = Person() person.full_clean()
Here the full_clean will generate this error:
Got a database error calling check() on <Q: (AND: RawSQL((first_name IS NULL) = (last_name IS NULL), {}))>: column "first_name" does not exist LINE 1: SELECT 1 AS "_check" WHERE COALESCE((((first_name IS NULL) =...
And the generated SQL query is:
SELECT 1 AS "_check" WHERE COALESCE((((first_name IS NULL) = (last_name IS NULL))), true); args=(1, True); alias=default
Change History (3)
comment:1 by , 23 months ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
comment:2 by , 23 months ago
ok so "it's not a bug, it's a feature"...
This new feature introduced recently is breaking code. So it's a regression. Worst it's a Breaking Change introduced in a minor version.
And moreover, this feature is adding new "hidden" and not expected sql queries. We have to take care of the N+1 query issue when we are using an ORM and here we clearly have more queries executed.
comment:3 by , 23 months ago
This particular expression can be expressed as
from django.db.models.lookups import Exact, IsNull Exact(IsNull(F("first_name")), IsNull(F("last_name")))
Or using Q
via
Q(first_name__isnull=True, last_name__isnull=True) | Q(first_name__isnull=False, last_name__isnull=False)
And possibly
~(Q(first_name=None) ^ Q(last_name=None))
If you're not interested in this validation taking place, you'd prefer save attempts to crash instead with an IntegrityError
, simply pass validate_constraints=False
to full_clean
just like you would have to disable the queries performed to validate unique constraints by passing validate_unique=False
.
We are aware of that and we decided to leave warnings in both places, i.e. in system checks (
models.W045
) and a loggerdjango.db.models
. Most ofRawSQL()
expression "can be expressed using a proper expression that doesn't bake column references but take them asF
that resolves toCol
so definitely something users can act on" and fix.