[Django] #30621: CheckConstraint: outer range contains inner range casts rhs field to lhs.inner_type
Django
lhs.inner_type
-------------------------------------+-------------------------------------
Reporter: Tilman | Owner: nobody
Koschnick |
Type: Bug | Status: new
Component: Database | Version: 2.2
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 1
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
I have a CheckConstraint:
{{{
CheckConstraint(check=m.Q(outer__contains=m.F('inner')),
name='outer_contains_inner')
}}}
where both outer and inner are DateTimeRangeFields. During migration, the
inner range gets cast to its underlying type:
{{{
ALTER TABLE "test" ADD CONSTRAINT "outer_contains_inner" CHECK ("outer" @>
("inner")::timestamp with time zone)
}}}
The attached patch avoids the cast if both ranges are of compatible type
(have not checked what happens if one is DateRange and the other is
DateTimeRange, though).
Another workaround would be to switch the relation around:
{{{
CheckConstraint(check=m.Q(inner__contained_by=m.F('outer')),
name='inner_contained_by_outer')
}}}
but I feel since both give equal results, they should both be possible.
--
Ticket URL: <https://code.djangoproject.com/ticket/30621>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
lhs.inner_type
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* Attachment "ranges.diff" added.
Django
-------------------------------------+-------------------------------------
Reporter: Tilman Koschnick | Owner: nobody
Component: Database layer | Version: 2.2
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Ian Foote (added)
* has_patch: 1 => 0
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
Thanks for the report. Would you like to submit a patch (with release
notes and tests) via GitHub?
Reproduced at 34a88b21dae71a26a9b136b599e5cbcf817eae16.
--
Ticket URL: <https://code.djangoproject.com/ticket/30621#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Tilman Koschnick | Owner: nobody
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tilman Koschnick):
To be honest, I am not quite sure why and when you would cast the
expression in the first place. Shouldn't the programmer make sure to pass
the right kind of value?
--
Ticket URL: <https://code.djangoproject.com/ticket/30621#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Tilman Koschnick | Owner: nobody
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by felixxm):
For me it is a bug and you're patch looks reasonable at first glance.
--
Ticket URL: <https://code.djangoproject.com/ticket/30621#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Tilman Koschnick | Owner: nobody
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tilman Koschnick):
Replying to [comment:3 felixxm]:
> For me it is a bug and you're patch looks reasonable at first glance.
It's a bug for sure - but I believe a better fix would be to drop the
casting bit entirely, but I don't know about the reasoning for putting it
in in the first place, so would be reluctant to remove it.
--
Ticket URL: <https://code.djangoproject.com/ticket/30621#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Tilman Koschnick | Owner: nobody
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by felixxm):
You can check #26903 and 6b048b364ca1e0e56a0d3815bf2be33ac9998355 for the
reasoning.
--
Ticket URL: <https://code.djangoproject.com/ticket/30621#comment:5>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => assigned
* owner: nobody => felixxm
--
Ticket URL: <https://code.djangoproject.com/ticket/30621#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Tilman Koschnick | Owner: felixxm
Type: Bug | Status: assigned
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/11553 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/30621#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Tilman Koschnick | Owner: felixxm
Type: Bug | Status: assigned
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Release blocker | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/30621#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Tilman Koschnick | Owner: felixxm
Component: Database layer | Version: 2.2
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"7991111af12056ec9a856f35935d273526338c1f" 7991111]:
{{{
#!CommitTicketReference repository=""
revision="7991111af12056ec9a856f35935d273526338c1f"
Fixed #30621 -- Fixed crash of __contains lookup for
Date/DateTimeRangeField when the right hand side is the same type.
Thanks Tilman Koschnick for the report and initial patch.
Thanks Carlton Gibson the review.
Regression in 6b048b364ca1e0e56a0d3815bf2be33ac9998355.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30621#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Tilman Koschnick | Owner: felixxm
Type: Bug | Status: closed
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"1088a9777da86dbf398106761c776edab29b163b" 1088a977]:
{{{
#!CommitTicketReference repository=""
revision="1088a9777da86dbf398106761c776edab29b163b"
[2.2.x] Fixed #30621 -- Fixed crash of __contains lookup for
Date/DateTimeRangeField when the right hand side is the same type.
Thanks Tilman Koschnick for the report and initial patch.
Thanks Carlton Gibson for the review.
Regression in 6b048b364ca1e0e56a0d3815bf2be33ac9998355.
Backport of 7991111af12056ec9a856f35935d273526338c1f from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30621#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Tilman Koschnick | Owner: felixxm
Type: Bug | Status: closed
Component: Database layer | Version: 2.2
(models, ORM) |
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tilman Koschnick):
Thanks a lot for fixing this!
--
Ticket URL: <https://code.djangoproject.com/ticket/30621#comment:11>