Re: [Django] #35234: ExclusionConstraint.expressions should be checked for foreign relationship references

[Django]

#35234: ExclusionConstraint.expressions should be checked for foreign 
relationship
references
-+-
 Reporter:  Simon Charette   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  closed
Component:  contrib.postgres |  Version:  5.0
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Comment (by GitHub ):

 In [changeset:"425b26092f038accd2a5c5fc5a9bd3f82d4dd847" 425b260]:
 {{{#!CommitTicketReference repository=""
 revision="425b26092f038accd2a5c5fc5a9bd3f82d4dd847"
 Refs #35234 -- Skipped CheckConstraint system checks if not supported.

 Thanks Tim Graham for the report.

 Regression in 0fb104dda287431f5ab74532e45e8471e22b58c8.
 }}}
-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018e8f7fedf7-5b56c752-6b42-4671-b492-5b813b094095-00%40eu-central-1.amazonses.com.

Re: [Django] #35234: ExclusionConstraint.expressions should be checked for foreign relationship references

[Django]

#35234: ExclusionConstraint.expressions should be checked for foreign 
relationship
references
-+-
 Reporter:  Simon Charette   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  closed
Component:  contrib.postgres |  Version:  5.0
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Comment (by Mariusz Felisiak ):

 In [changeset:"daf7d482dbaaa2604241a994c49f442fa15142c1" daf7d48]:
 {{{#!CommitTicketReference repository=""
 revision="daf7d482dbaaa2604241a994c49f442fa15142c1"
 Refs #35234 -- Deprecated CheckConstraint.check in favor of .condition.

 Once the deprecation period ends CheckConstraint.check() can become the
 documented method that performs system checks for BaseConstraint
 subclasses.
 }}}
-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018df8d389ef-923aed00-faff-4bbb-93d6-5533771168b1-00%40eu-central-1.amazonses.com.

Re: [Django] #35234: ExclusionConstraint.expressions should be checked for foreign relationship references

[Django]

#35234: ExclusionConstraint.expressions should be checked for foreign 
relationship
references
-+-
 Reporter:  Simon Charette   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  closed
Component:  contrib.postgres |  Version:  5.0
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Mariusz Felisiak ):

 * resolution:   => fixed
 * status:  assigned => closed

Comment:

 In [changeset:"f82c67aa217c8dd4320ef697b04a6da1681aa799" f82c67aa]:
 {{{#!CommitTicketReference repository=""
 revision="f82c67aa217c8dd4320ef697b04a6da1681aa799"
 Fixed #35234 -- Added system checks for invalid model field names in
 ExclusionConstraint.expressions.
 }}}
-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018df6018987-49ded6e5-b219-4106-ae5e-36c5d79b5eb8-00%40eu-central-1.amazonses.com.

Re: [Django] #35234: ExclusionConstraint.expressions should be checked for foreign relationship references

[Django]

#35234: ExclusionConstraint.expressions should be checked for foreign 
relationship
references
-+-
 Reporter:  Simon Charette   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  assigned
Component:  contrib.postgres |  Version:  5.0
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Comment (by Mariusz Felisiak ):

 In [changeset:"0fb104dda287431f5ab74532e45e8471e22b58c8" 0fb104dd]:
 {{{#!CommitTicketReference repository=""
 revision="0fb104dda287431f5ab74532e45e8471e22b58c8"
 Refs #35234 -- Moved constraint system checks to Check/UniqueConstraint
 methods.
 }}}
-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018df4643faf-c5e06bdf-894a-475c-a85d-01165e5845fe-00%40eu-central-1.amazonses.com.

Re: [Django] #35234: ExclusionConstraint.expressions should be checked for foreign relationship references

[Django]

#35234: ExclusionConstraint.expressions should be checked for foreign 
relationship
references
-+-
 Reporter:  Simon Charette   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  assigned
Component:  contrib.postgres |  Version:  5.0
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Comment (by Simon Charette):

 Adjusted the MR to this effect by adding a commit that deprecates
 `CheckConstraint.check` in favour of `.condition`.

 The only part that might be another set of eyes is the `stacklevel` which
 I haven't confirmed is exactly what it should be for the warning to point
 at `CheckConstraint(...)` and `.check` interactions.
-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018de4033a14-f3717ac6-478f-4486-97ba-8ac1d3cc86b0-00%40eu-central-1.amazonses.com.

Re: [Django] #35234: ExclusionConstraint.expressions should be checked for foreign relationship references

[Django]

#35234: ExclusionConstraint.expressions should be checked for foreign 
relationship
references
-+-
 Reporter:  Simon Charette   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  assigned
Component:  contrib.postgres |  Version:  5.0
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Comment (by Mariusz Felisiak):

 Replying to [comment:8 Simon Charette]:
 > I have a slight inclination for the second option as it seems less
 disruptive? It involves a single method and `CheckConstraint.check` has
 been added quite more recently that all the other abstractions. So what
 about the following approach
 >
 > 1. Store the attribute in `CheckExpression.condition` and allow either
 `condition` or `check` to be passed to `__init__`
 > 2. Raise a deprecation warning when `check` is provided.
 > 3. Add a `@property` shim for `CheckExpression.check` that returns
 `self.condition` and raise a deprecation warning
 > 4. Merge this patch that has `Model._check_constraints` call
 `BaseConstraint._check` for the time being to perform system checks
 > 5. When the deprecation period ends remove the shim and have
 `_check_constraints` call into the newly instated `BaseConstraint.check`
 method and take the opportunity to document it.

 Sounds like a plan 
-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018dd78209da-ac6a279a-0edd-41f0-b594-1c9420ac9a76-00%40eu-central-1.amazonses.com.

Re: [Django] #35234: ExclusionConstraint.expressions should be checked for foreign relationship references

[Django]

#35234: ExclusionConstraint.expressions should be checked for foreign 
relationship
references
-+-
 Reporter:  Simon Charette   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  assigned
Component:  contrib.postgres |  Version:  5.0
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Comment (by Simon Charette):

 I have a slight inclination for the second option as it seems less
 disruptive? It involves a single method and `CheckConstraint.check` has
 been added quite more recently that all the other abstractions. So what
 about the following approach

 1. Store the attribute in `CheckExpression.condition` and allow either
 `condition` or `check` to be passed to `__init__`
 2. Raise a deprecation warning when `check` is provided.
 3. Add a `@property` deprecation shim for `CheckExpression.check` that
 returns `self.condition`
 4. Merge this patch that has `Model._check_constraints` call
 `BaseConstraint._check` for the time being to perform system checks
 5. When the deprecation period ends remove the shim and have the
 `_check_constraints` call into the newly instated `BaseConstraint.check`
 method and take the opportunity to document it.
-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018dd698d86c-4465a49e-3a48-4320-b9d4-f59a845837ef-00%40eu-central-1.amazonses.com.

Re: [Django] #35234: ExclusionConstraint.expressions should be checked for foreign relationship references

[Django]

#35234: ExclusionConstraint.expressions should be checked for foreign 
relationship
references
-+-
 Reporter:  Simon Charette   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  assigned
Component:  contrib.postgres |  Version:  5.0
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Comment (by Mariusz Felisiak):

 Replying to [comment:6 Simon Charette]:
 > > But then we have the dilemma of diverging from the kwarg check passed
 to CheckConstraint.
 >
 > yeah that's the crux of the issue, what Mariusz was referring to is that
 the attribute name is documented as the patch didn't suggest changing the
 kwarg name.
 >
 > If we are going on the more explicit route I guess that `def
 system_checks` would be the most unambiguous choice? I pushed a version of
 the patch that opts to use `_check` for system checks as that maintains
 the public contract on `ChekConstriant.check` but it doesn't lift the
 ambiguity on the notion of ''check'' which might be warranted here to
 avoid any confusion.

 Yes, that's unfortunate. We will not be able to make it consistent without
 a proper deprecation of some kind. We can
 - deprecate `check()` for
 [https://docs.djangoproject.com/en/5.0/topics/checks/#field-model-manager-
 and-database-checks fields, models, model managers, and database
 backends], and rename it to `system_check()`,
 - deprecate `check` argument for `CheckConstraint` and rename it to the
 `check_expression` or `condition`.
-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018dcfe643bc-17ff4523-f05b-453d-bbc9-c5e0d99a918e-00%40eu-central-1.amazonses.com.

Re: [Django] #35234: ExclusionConstraint.expressions should be checked for foreign relationship references

[Django]

#35234: ExclusionConstraint.expressions should be checked for foreign 
relationship
references
-+-
 Reporter:  Simon Charette   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  assigned
Component:  contrib.postgres |  Version:  5.0
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Comment (by Simon Charette):

 > But then we have the dilemma of diverging from the kwarg check passed to
 CheckConstraint.

 yeah that's the crux of the issu, what Mariusz was referring to is that
 the attribute name is documented as the patch didn't suggest changing the
 kwarg name.

 If we are going on the more explicit route I guess that `def
 system_checks` would be the most unambiguous choice? I pushed a version of
 the patch that opts to use `_check` for system checks as that maintains
 the public contract on `ChekConstriant.check` but it doesn't lift the
 ambiguity on the notion of ''check'' which might be warranted here to
 avoid any confusion.
-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018dcf6cf079-d2f540c8-7b96-41a1-b771-1d74695abcea-00%40eu-central-1.amazonses.com.

Re: [Django] #35234: ExclusionConstraint.expressions should be checked for foreign relationship references

[Django]

#35234: ExclusionConstraint.expressions should be checked for foreign 
relationship
references
-+-
 Reporter:  Simon Charette   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  assigned
Component:  contrib.postgres |  Version:  5.0
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by David Sanders):

 * cc: David Sanders (added)

Comment:

 Just on the `._check` naming, imho this could confuse people by leading
 them to believe there's a relationship between `.check()` and `._check`?
 路‍♂️

 As for suggestions, the only thing I could think of was to be more
 specific, perhaps `._check_expression` ?  Would be inline with
 `._db_default_expression`. But then we have the dilemma of diverging from
 the kwarg `check` passed to CheckConstraint.
-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018dcf58a214-c97fa6bf-8815-4944-9cb0-a63280381cd5-00%40eu-central-1.amazonses.com.

Re: [Django] #35234: ExclusionConstraint.expressions should be checked for foreign relationship references

[Django]

#35234: ExclusionConstraint.expressions should be checked for foreign 
relationship
references
-+-
 Reporter:  Simon Charette   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  assigned
Component:  contrib.postgres |  Version:  5.0
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Simon Charette):

 * needs_better_patch:  1 => 0

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018dcf4c7d1c-e84e1108-2c64-42da-b40c-dc648458320a-00%40eu-central-1.amazonses.com.

Re: [Django] #35234: ExclusionConstraint.expressions should be checked for foreign relationship references

[Django]

#35234: ExclusionConstraint.expressions should be checked for foreign 
relationship
references
-+-
 Reporter:  Simon Charette   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  assigned
Component:  contrib.postgres |  Version:  5.0
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Mariusz Felisiak):

 * needs_better_patch:  0 => 1

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018dcae9892e-7d9cc093-9591-4de8-943e-33b9e3a91c1a-00%40eu-central-1.amazonses.com.

Re: [Django] #35234: ExclusionConstraint.expressions should be checked for foreign relationship references

[Django]

#35234: ExclusionConstraint.expressions should be checked for foreign 
relationship
references
-+-
 Reporter:  Simon Charette   |Owner:  Simon
 Type:   |  Charette
  Cleanup/optimization   |   Status:  assigned
Component:  contrib.postgres |  Version:  5.0
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Mariusz Felisiak):

 * owner:  (none) => Simon Charette

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018dc05139bb-c1aad979-3ad9-4212-b700-db759fc3b6c3-00%40eu-central-1.amazonses.com.

Re: [Django] #35234: ExclusionConstraint.expressions should be checked for foreign relationship references

[Django]

#35234: ExclusionConstraint.expressions should be checked for foreign 
relationship
references
--+
 Reporter:  Simon Charette|Owner:  (none)
 Type:  Cleanup/optimization  |   Status:  assigned
Component:  contrib.postgres  |  Version:  5.0
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  1 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+
Changes (by Mariusz Felisiak):

 * has_patch:  0 => 1
 * stage:  Unreviewed => Accepted

Comment:

 [https://github.com/django/django/pull/17876 PR]
-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018dc036a815-9c66a6f3-8dcc-441f-a703-e9354db1917d-00%40eu-central-1.amazonses.com.