Opened 21 months ago

Closed 21 months ago

Last modified 21 months ago

#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 Mariusz Felisiak, 21 months ago

Resolution: invalid
Status: newclosed

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.

We are aware of that and we decided to leave warnings in both places, i.e. in system checks (models.W045) and a logger django.db.models. Most of RawSQL() expression "can be expressed using a proper expression that doesn't bake column references but take them as F that resolves to Col so definitely something users can act on" and fix.

comment:2 by Manuel Raynaud, 21 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 Simon Charette, 21 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.

Last edited 21 months ago by Simon Charette (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top