Opened 4 years ago

Closed 4 years ago

Last modified 4 years 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, 4 years ago

Resolution: → invalid
Status: new → closed

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, 4 years 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, 4 years 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))
Version 0, edited 4 years ago by Simon Charette (next)
Note: See TracTickets for help on using tickets.
Back to Top