[Django] #24698: Relation clear fails silently when using OneToOneField+UUIDField
Django
-------------------------------------+-------------------------------------
Reporter: webjunkie | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.8
(models, ORM) | Keywords: uuidfield onetoonefield
Severity: Normal | manytomanyfield
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
I have the following simplified model:
{{{
class Lead(models.Model):
id = models.UUIDField(primary_key=True, default=uuid.uuid4,
editable=False)
name = models.CharField(max_length=255)
class LeadInfo(models.Model):
lead = models.OneToOneField(Lead, primary_key=True)
url = models.URLField("Blog URL", blank=True)
topics = models.ManyToManyField("Topic", blank=True)
class Topic(models.Model):
id = models.UUIDField(primary_key=True, default=uuid.uuid4,
editable=False)
name = models.CharField(max_length=255)
}}}
There seems to be a problem with the clear() on the ManyToManyField.
Django tries to delete entries, but uses the uuid with hyphens, although
it previously saved the uuid without. The clear does nothing, and the next
add will then fail. (This becomes a problem in ModelForms, as they clear
relations before updating). This is on MySQL.
{{{
>>> l = Lead.objects.create(name="Lead 1")
>>> topic = Topic.objects.create(name="Topic 1")
>>> info = LeadInfo.objects.create(lead=l, url="bla")
>>> info.topics.add(topic)
>>> info.topics.clear()
>>> info.topics.add(topic)
IntegrityError: (1062, "Duplicate entry
'5357d002c2d74e1899b845a596e10cd8-15572c1b20234a6caa917cb200a2436' for key
'leadinfo_id'")
}}}
I've traced it down to Django making a wrong query for the delete, but I
cannot tell why the hyphens end up in there, whereas in the INSERT it does
them right:
{{{
DELETE FROM `uuidtest_leadinfo_topics` WHERE
`uuidtest_leadinfo_topics`.`leadinfo_id` =
'5357d002-c2d7-4e18-99b8-45a596e10cd8'";
INSERT INTO `uuidtest_leadinfo_topics` (`leadinfo_id`, `topic_id`) VALUES
('5357d002c2d74e1899b845a596e10cd8', '15572c1b20234a6caa917cb200a2436b');
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24698>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: webjunkie | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.8
(models, ORM) |
Keywords: uuidfield | Triage Stage: Accepted
onetoonefield manytomanyfield |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_docs: => 0
* needs_better_patch: => 0
* severity: Normal => Release blocker
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
Fixed on master by b68212f539f206679580afbfd008e7d329c9cd31, but
backporting that to 1.8 probably isn't an option.
--
Ticket URL: <https://code.djangoproject.com/ticket/24698#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: webjunkie | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.8
(models, ORM) |
Keywords: uuidfield | Triage Stage: Accepted
onetoonefield manytomanyfield |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by timgraham):
The root cause might be a duplicate of #24712 which was also fixed by the
bisected commit.
--
Ticket URL: <https://code.djangoproject.com/ticket/24698#comment:2>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: Database layer | Version: 1.8
(models, ORM) |
Keywords: uuidfield | Triage Stage: Accepted
onetoonefield manytomanyfield |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => abhaga
* status: new => assigned
Comment:
The root cause for both this and #24712 is indeed same: missing
`get_db_prep_value` on `ForeignKey` which was added in the bisected
commit. Working on a patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/24698#comment:3>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
(models, ORM) |
Keywords: uuidfield | Triage Stage: Accepted
onetoonefield manytomanyfield |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by abhaga):
Thinking about it, since a `ForeignKey` can refer to any unique field on
the related model, this change is likely to be backward incompatible. The
behavior will change in case of all the fields which implement a
`get_db_prep_value` (ex: DateTimeField). Although it doesn't seem a common
use case to have foreign keys pointing to fields other than `IntegerField`
and `UUIDField`.
The patch is simple but can it be applied in 1.8.x branch?
--
Ticket URL: <https://code.djangoproject.com/ticket/24698#comment:4>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
(models, ORM) |
Keywords: uuidfield | Triage Stage: Accepted
onetoonefield manytomanyfield |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by abhaga):
Other option is to patch the `get_db_prep_lookup` on the `Lookup` class
and do a special case fix for related fields pointing to `UUIDField`. That
will not change the behavior for other fields. In any case, the proper fix
is already in place in master for future releases.
--
Ticket URL: <https://code.djangoproject.com/ticket/24698#comment:5>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
(models, ORM) |
Keywords: uuidfield | Triage Stage: Accepted
onetoonefield manytomanyfield |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by akaariai):
It seems clear that not calling get_db_prep_value is a bug. While it might
be backwards incompatible, we don't guarantee backwards incompatibility
for bugs. So, unless it is clear that this is likely to break existing
installations running 1.8, then I think we can go ahead and fix this for
all field types.
It would be interesting to find cases which this change breaks. Can you
show something involving a date field which worked before, but doesn't
work after your patch?
Also, if you have a patch to solve this, please do make a pull request. If
you don't think your PR is actually ready to be merged, you can add [WIP]
to the title of the pull request so that reviewers know this isn't ready
for commit yet.
--
Ticket URL: <https://code.djangoproject.com/ticket/24698#comment:6>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
(models, ORM) |
Keywords: uuidfield | Triage Stage: Accepted
onetoonefield manytomanyfield |
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/4644 PR]. I left some suggestions
for improving the tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/24698#comment:7>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: 1.8
(models, ORM) |
Keywords: uuidfield | Triage Stage: Accepted
onetoonefield manytomanyfield |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"290c9d665490d80b0a1b648fb022190d7dc375fc" 290c9d6]:
{{{
#!CommitTicketReference repository=""
revision="290c9d665490d80b0a1b648fb022190d7dc375fc"
[1.8.x] Fixed #24698, #24712 -- Added ForeignKey.get_db_prep_value()
Fixed crashes with ForeignKey to UUIDField and inheritance with UUIDField
primary keys.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24698#comment:8>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
(models, ORM) |
Keywords: uuidfield | Triage Stage: Accepted
onetoonefield manytomanyfield |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"7c7b85510656cc814814b85bc52201bbd91821a9" 7c7b855]:
{{{
#!CommitTicketReference repository=""
revision="7c7b85510656cc814814b85bc52201bbd91821a9"
[1.8.x] Refs #24698, #24712 -- Forwardported
ForeignKey.get_db_prep_value() test and release notes.
Fixed in master by b68212f539f206679580afbfd008e7d329c9cd31.
Forwardport of 290c9d665490d80b0a1b648fb022190d7dc375fc from stable/1.8.x
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24698#comment:9>