Opened 3 months ago
Closed 3 months ago
#36433 closed Bug (fixed)
Model validation of constraints fails if condition's Q object references foreign key by _id
Reported by: | Jacob Walls | Owned by: | colleenDunlap |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Release blocker | Keywords: | |
Cc: | Mariusz Felisiak | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Bisected to e44e8327d3d88d86895735c0e427102063ff5b55 (refs #36222).
With these models:
class Language(models.Model): pass class LanguageValue(models.Model): language = models.ForeignKey( Language, on_delete=models.PROTECT, null=True, blank=True, ) value = models.CharField(max_length=1024, null=False, blank=True) class Meta: constraints = [ models.CheckConstraint( condition=Q(language_id__isnull=False), name="require_language", ), ]
This works on 5.2 and fails on main. Notice wrong "Choices are: _check". (Curiously it works if the Q()
object is adjusted to reference language
rather than language_id
.)
In [1]: lang = Language.objects.create() In [2]: value = LanguageValue.objects.create(language=lang) In [3]: value.full_clean() -------------------------------------------- FieldError Traceback (most recent call last) Cell In[3], line 1 ----> 1 value.full_clean() File ~/django/django/db/models/base.py:1633, in Model.full_clean(self, exclude, validate_unique, validate_constraints) 1631 exclude.add(name) 1632 try: -> 1633 self.validate_constraints(exclude=exclude) 1634 except ValidationError as e: 1635 errors = e.update_error_dict(errors) File ~/django/django/db/models/base.py:1581, in Model.validate_constraints(self, exclude) 1579 for constraint in model_constraints: 1580 try: -> 1581 constraint.validate(model_class, self, exclude=exclude, using=using) 1582 except ValidationError as e: 1583 if ( 1584 getattr(e, "code", None) == "unique" 1585 and len(constraint.fields) == 1 1586 ): File ~/django/django/db/models/constraints.py:212, in CheckConstraint.validate(self, model, instance, exclude, using) 210 if exclude and self._expression_refs_exclude(model, self.condition, exclude): 211 return --> 212 if not Q(self.condition).check(against, using=using): 213 raise ValidationError( 214 self.get_violation_error_message(), code=self.violation_error_code 215 ) File ~/django/django/db/models/query_utils.py:137, in Q.check(self, against, using) 135 # This will raise a FieldError if a field is missing in "against". 136 if connection.features.supports_comparing_boolean_expr: --> 137 query.add_q(Q(Coalesce(self, True, output_field=BooleanField()))) 138 else: 139 query.add_q(self) File ~/django/django/db/models/sql/query.py:1646, in Query.add_q(self, q_object, reuse_all) 1644 else: 1645 can_reuse = self.used_aliases -> 1646 clause, _ = self._add_q(q_object, can_reuse) 1647 if clause: 1648 self.where.add(clause, AND) File ~/django/django/db/models/sql/query.py:1678, in Query._add_q(self, q_object, used_aliases, branch_negated, current_negated, allow_joins, split_subq, check_filterable, summarize, update_join_types) 1674 joinpromoter = JoinPromoter( 1675 q_object.connector, len(q_object.children), current_negated 1676 ) 1677 for child in q_object.children: -> 1678 child_clause, needed_inner = self.build_filter( 1679 child, 1680 can_reuse=used_aliases, 1681 branch_negated=branch_negated, 1682 current_negated=current_negated, 1683 allow_joins=allow_joins, 1684 split_subq=split_subq, 1685 check_filterable=check_filterable, 1686 summarize=summarize, 1687 update_join_types=update_join_types, 1688 ) 1689 joinpromoter.add_votes(needed_inner) 1690 if child_clause: File ~/django/django/db/models/sql/query.py:1517, in Query.build_filter(self, filter_expr, branch_negated, current_negated, can_reuse, allow_joins, split_subq, check_filterable, summarize, update_join_types) 1515 if not getattr(filter_expr, "conditional", False): 1516 raise TypeError("Cannot filter against a non-conditional expression.") -> 1517 condition = filter_expr.resolve_expression( 1518 self, allow_joins=allow_joins, reuse=can_reuse, summarize=summarize 1519 ) 1520 if not isinstance(condition, Lookup): 1521 condition = self.build_lookup(["exact"], condition, True) File ~/django/django/db/models/expressions.py:300, in BaseExpression.resolve_expression(self, query, allow_joins, reuse, summarize, for_save) 296 c = self.copy() 297 c.is_summary = summarize 298 source_expressions = [ 299 ( --> 300 expr.resolve_expression(query, allow_joins, reuse, summarize) 301 if expr is not None 302 else None 303 ) 304 for expr in c.get_source_expressions() 305 ] 306 if not self.allows_composite_expressions and any( 307 isinstance(expr, ColPairs) for expr in source_expressions 308 ): 309 raise ValueError( 310 f"{self.__class__.__name__} expression does not support " 311 "composite primary keys." 312 ) File ~/django/django/db/models/query_utils.py:91, in Q.resolve_expression(self, query, allow_joins, reuse, summarize, for_save) 86 def resolve_expression( 87 self, query=None, allow_joins=True, reuse=None, summarize=False, for_save=False 88 ): 89 # We must promote any new joins to left outer joins so that when Q is 90 # used as an expression, rows aren't filtered due to joins. ---> 91 clause, joins = query._add_q( 92 self, 93 reuse, 94 allow_joins=allow_joins, 95 split_subq=False, 96 check_filterable=False, 97 summarize=summarize, 98 ) 99 query.promote_joins(joins) 100 return clause File ~/django/django/db/models/sql/query.py:1678, in Query._add_q(self, q_object, used_aliases, branch_negated, current_negated, allow_joins, split_subq, check_filterable, summarize, update_join_types) 1674 joinpromoter = JoinPromoter( 1675 q_object.connector, len(q_object.children), current_negated 1676 ) 1677 for child in q_object.children: -> 1678 child_clause, needed_inner = self.build_filter( 1679 child, 1680 can_reuse=used_aliases, 1681 branch_negated=branch_negated, 1682 current_negated=current_negated, 1683 allow_joins=allow_joins, 1684 split_subq=split_subq, 1685 check_filterable=check_filterable, 1686 summarize=summarize, 1687 update_join_types=update_join_types, 1688 ) 1689 joinpromoter.add_votes(needed_inner) 1690 if child_clause: File ~/django/django/db/models/sql/query.py:1503, in Query.build_filter(self, filter_expr, branch_negated, current_negated, can_reuse, allow_joins, split_subq, check_filterable, summarize, update_join_types) 1501 raise FieldError("Cannot parse keyword query as dict") 1502 if isinstance(filter_expr, Q): -> 1503 return self._add_q( 1504 filter_expr, 1505 branch_negated=branch_negated, 1506 current_negated=current_negated, 1507 used_aliases=can_reuse, 1508 allow_joins=allow_joins, 1509 split_subq=split_subq, 1510 check_filterable=check_filterable, 1511 summarize=summarize, 1512 update_join_types=update_join_types, 1513 ) 1514 if hasattr(filter_expr, "resolve_expression"): 1515 if not getattr(filter_expr, "conditional", False): File ~/django/django/db/models/sql/query.py:1678, in Query._add_q(self, q_object, used_aliases, branch_negated, current_negated, allow_joins, split_subq, check_filterable, summarize, update_join_types) 1674 joinpromoter = JoinPromoter( 1675 q_object.connector, len(q_object.children), current_negated 1676 ) 1677 for child in q_object.children: -> 1678 child_clause, needed_inner = self.build_filter( 1679 child, 1680 can_reuse=used_aliases, 1681 branch_negated=branch_negated, 1682 current_negated=current_negated, 1683 allow_joins=allow_joins, 1684 split_subq=split_subq, 1685 check_filterable=check_filterable, 1686 summarize=summarize, 1687 update_join_types=update_join_types, 1688 ) 1689 joinpromoter.add_votes(needed_inner) 1690 if child_clause: File ~/django/django/db/models/sql/query.py:1526, in Query.build_filter(self, filter_expr, branch_negated, current_negated, can_reuse, allow_joins, split_subq, check_filterable, summarize, update_join_types) 1524 if not arg: 1525 raise FieldError("Cannot parse keyword query %r" % arg) -> 1526 lookups, parts, reffed_expression = self.solve_lookup_type(arg, summarize) 1528 if check_filterable: 1529 self.check_filterable(reffed_expression) File ~/django/django/db/models/sql/query.py:1333, in Query.solve_lookup_type(self, lookup, summarize) 1331 expression = Ref(annotation, expression) 1332 return expression_lookups, (), expression -> 1333 _, field, _, lookup_parts = self.names_to_path(lookup_splitted, self.get_meta()) 1334 field_parts = lookup_splitted[0 : len(lookup_splitted) - len(lookup_parts)] 1335 if len(lookup_parts) > 1 and not field_parts: File ~/django/django/db/models/sql/query.py:1805, in Query.names_to_path(self, names, opts, allow_many, fail_on_missing) 1797 if pos == -1 or fail_on_missing: 1798 available = sorted( 1799 [ 1800 *get_field_names_from_opts(opts), (...) 1803 ] 1804 ) -> 1805 raise FieldError( 1806 "Cannot resolve keyword '%s' into field. " 1807 "Choices are: %s" % (name, ", ".join(available)) 1808 ) 1809 break 1810 # Check if we need any joins for concrete inheritance cases (the 1811 # field lives in parent, but we are currently in one of its 1812 # children) FieldError: Cannot resolve keyword 'language_id' into field. Choices are: _check
Change History (8)
comment:1 by , 3 months ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 3 months ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:3 by , 3 months ago
Cc: | added |
---|
comment:4 by , 3 months ago
Has patch: | set |
---|
comment:7 by , 3 months ago
Triage Stage: | Accepted → Ready for checkin |
---|
Note:
See TracTickets
for help on using tickets.
Thanks for the report Jacob!
I think this is a bug in
_get_field_expression_map
, it should augment the map withField.attname
as welldjango/db/models/base.py
tests/constraints/tests.py
As for the unhelpful error message I wonder if we should make
Q.check
catchFieldError
and re-reraise withagainst
keys instead. Maybe not worth it as users should hit this code path normally.