Skip to content

Commit

Permalink
Fixed #35275 -- Fixed Meta.constraints validation crash on UniqueCons…
Browse files Browse the repository at this point in the history
…traint with OpClass().

This also introduces Expression.constraint_validation_compatible that
allows specifying that expression should be ignored during a constraint
validation.
  • Loading branch information
felixxm authored and sarahboyce committed May 14, 2024
1 parent ceaf1e2 commit f030236
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 11 deletions.
11 changes: 4 additions & 7 deletions django/contrib/postgres/constraints.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from types import NoneType

from django.contrib.postgres.indexes import OpClass
from django.core.exceptions import ValidationError
from django.db import DEFAULT_DB_ALIAS, NotSupportedError
from django.db.backends.ddl_references import Expressions, Statement, Table
Expand Down Expand Up @@ -208,12 +207,10 @@ def validate(self, model, instance, exclude=None, using=DEFAULT_DB_ALIAS):
if isinstance(expr, F) and expr.name in exclude:
return
rhs_expression = expression.replace_expressions(replacements)
# Remove OpClass because it only has sense during the constraint
# creation.
if isinstance(expression, OpClass):
expression = expression.get_source_expressions()[0]
if isinstance(rhs_expression, OpClass):
rhs_expression = rhs_expression.get_source_expressions()[0]
if hasattr(expression, "get_expression_for_validation"):
expression = expression.get_expression_for_validation()
if hasattr(rhs_expression, "get_expression_for_validation"):
rhs_expression = rhs_expression.get_expression_for_validation()
lookup = PostgresOperatorLookup(lhs=expression, rhs=rhs_expression)
lookup.postgres_operator = operator
lookups.append(lookup)
Expand Down
1 change: 1 addition & 0 deletions django/contrib/postgres/indexes.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ def check_supported(self, schema_editor):

class OpClass(Func):
template = "%(expressions)s %(name)s"
constraint_validation_compatible = False

def __init__(self, expression, name):
super().__init__(expression, name=name)
7 changes: 3 additions & 4 deletions django/db/models/constraints.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from django.core.exceptions import FieldDoesNotExist, FieldError, ValidationError
from django.db import connections
from django.db.models.constants import LOOKUP_SEP
from django.db.models.expressions import Exists, ExpressionList, F, OrderBy, RawSQL
from django.db.models.expressions import Exists, ExpressionList, F, RawSQL
from django.db.models.indexes import IndexExpression
from django.db.models.lookups import Exact
from django.db.models.query_utils import Q
Expand Down Expand Up @@ -644,9 +644,8 @@ def validate(self, model, instance, exclude=None, using=DEFAULT_DB_ALIAS):
}
expressions = []
for expr in self.expressions:
# Ignore ordering.
if isinstance(expr, OrderBy):
expr = expr.expression
if hasattr(expr, "get_expression_for_validation"):
expr = expr.get_expression_for_validation()
expressions.append(Exact(expr, expr.replace_expressions(replacements)))
queryset = queryset.filter(*expressions)
model_class_pk = instance._get_pk_val(model._meta)
Expand Down
17 changes: 17 additions & 0 deletions django/db/models/expressions.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,8 @@ class BaseExpression:
window_compatible = False
# Can the expression be used as a database default value?
allowed_default = False
# Can the expression be used during a constraint validation?
constraint_validation_compatible = True

def __init__(self, output_field=None):
if output_field is not None:
Expand Down Expand Up @@ -484,6 +486,20 @@ def select_format(self, compiler, sql, params):
return self.output_field.select_format(compiler, sql, params)
return sql, params

def get_expression_for_validation(self):
# Ignore expressions that cannot be used during a constraint validation.
if not getattr(self, "constraint_validation_compatible", True):
try:
(expression,) = self.get_source_expressions()
except ValueError as e:
raise ValueError(
"Expressions with constraint_validation_compatible set to False "
"must have only one source expression."
) from e
else:
return expression
return self


@deconstructible
class Expression(BaseExpression, Combinable):
Expand Down Expand Up @@ -1716,6 +1732,7 @@ def as_sql(self, compiler, *args, **kwargs):
class OrderBy(Expression):
template = "%(expression)s %(ordering)s"
conditional = False
constraint_validation_compatible = False

def __init__(self, expression, descending=False, nulls_first=None, nulls_last=None):
if nulls_first and nulls_last:
Expand Down
9 changes: 9 additions & 0 deletions docs/ref/models/expressions.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1058,6 +1058,15 @@ calling the appropriate methods on the wrapped expression.
Tells Django that this expression can be used in
:attr:`Field.db_default`. Defaults to ``False``.

.. attribute:: constraint_validation_compatible

.. versionadded:: 5.1

Tells Django that this expression can be used during a constraint
validation. Expressions with ``constraint_validation_compatible`` set
to ``False`` must have only one source expression. Defaults to
``True``.

.. attribute:: contains_aggregate

Tells Django that this expression contains an aggregate and that a
Expand Down
4 changes: 4 additions & 0 deletions docs/releases/5.1.txt
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,10 @@ Models
reload a model's value. This can be used to lock the row before reloading or
to select related objects.

* The new :attr:`.Expression.constraint_validation_compatible` attribute allows
specifying that the expression should be ignored during a constraint
validation.

Requests and Responses
~~~~~~~~~~~~~~~~~~~~~~

Expand Down
10 changes: 10 additions & 0 deletions tests/expressions/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1425,6 +1425,16 @@ class TestModel(Model):
hash(Expression(TestModel._meta.get_field("other_field"))),
)

def test_get_expression_for_validation_only_one_source_expression(self):
expression = Expression()
expression.constraint_validation_compatible = False
msg = (
"Expressions with constraint_validation_compatible set to False must have "
"only one source expression."
)
with self.assertRaisesMessage(ValueError, msg):
expression.get_expression_for_validation()


class ExpressionsNumericTests(TestCase):
@classmethod
Expand Down
13 changes: 13 additions & 0 deletions tests/postgres_tests/test_constraints.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,19 @@ def test_opclass_func(self):
self.assertNotIn(constraint.name, self.get_constraints(Scene._meta.db_table))
Scene.objects.create(scene="ScEnE 10", setting="Sir Bedemir's Castle")

def test_opclass_func_validate_constraints(self):
constraint_name = "test_opclass_func_validate_constraints"
constraint = UniqueConstraint(
OpClass(Lower("scene"), name="text_pattern_ops"),
name="test_opclass_func_validate_constraints",
)
Scene.objects.create(scene="First scene")
# Non-unique scene.
msg = f"Constraint “{constraint_name}” is violated."
with self.assertRaisesMessage(ValidationError, msg):
constraint.validate(Scene, Scene(scene="first Scene"))
constraint.validate(Scene, Scene(scene="second Scene"))


class ExclusionConstraintTests(PostgreSQLTestCase):
def get_constraints(self, table):
Expand Down

0 comments on commit f030236

Please sign in to comment.