[Django] #22728: get_or_create with field lookups cause empty values
Django
----------------------------------------------+---------------------------
Reporter: Xudonax | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: 1.7-beta-2
Severity: Normal | Keywords: get_or_create
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+---------------------------
First of, I'm actually using 1.7b4, but there seems to be no version for
that (yet)...
When trying to be smart and using a case insensitive filter for
get_or_create, it doesn't actually fill in the value, because you passed
it in with a field lookup instead of just the field name. An example
explains this better:
{{{
from django.db import models
class Tag(models.Model):
name = models.CharField(unique=True)
slug = models.SlugField(unique=True)
tags = ['django', 'python', 'web', 'Django', 'html5', 'Python'] # Just a
list of tags to make this work
for tag in tags:
obj, created = Tag.objects.get_or_create(name__iexact=tag,
defaults={'slug': tag)
print("Tag {0} created? {1}".format(obj.name, created)
}}}
This will give a IntegrityError while going through the list, complaining
that there already is a tag with an empty value for name.
{{{
IntegrityError: duplicate key value violates unique constraint
"blog_tag_name_key"
DETAIL: Key (name)=() already exists.
}}}
A work-around is to also specify the name field in the defaults, but I was
actually expecting the ORM to be "smart" enough to not require this. There
is no [https://docs.djangoproject.com/en/1.7/ref/models/querysets/#get-or-
create documentation] describing it should happen either way. Is this a
documentation issue or a code issue?
--
Ticket URL: <https://code.djangoproject.com/ticket/22728>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Xudonax | Owner: nobody
Type: Bug | Status: new
(models, ORM) | 1.7-beta-2
Severity: Normal | Resolution:
Keywords: get_or_create | Triage Stage:
Has patch: 0 | Unreviewed
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by valberg):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
You have an error in your code. You are mixing the `name` and `slug`
fields, and it is actually complaining that there already exists a row in
the Tag table with the empty string as a name.
Here is something that works:
{{{
tags = ['django', 'python', 'web', 'Django', 'html5', 'Python']
for tag in tags:
obj, created = Tag.objects.get_or_create(name__iexact=tag,
defaults={'slug': tag, 'name': tag})
print("Tag {0} created? {1}".format(obj.name, created)
}}}
You have to fill in both `name` and `slug` when you've decided to make
both unique.
--
Ticket URL: <https://code.djangoproject.com/ticket/22728#comment:1>
Django
Reporter: Xudonax | Owner: nobody
Type: Bug | Status: new
(models, ORM) | 1.7-beta-2
Severity: Normal | Resolution:
Keywords: get_or_create | Triage Stage:
Has patch: 0 | Unreviewed
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by aaugustin):
Your expectations are a bit higher than what the ORM is actually able to
do ;-)
This is really a feature request. Can you submit it to the django-
developers mailing-list?
--
Ticket URL: <https://code.djangoproject.com/ticket/22728#comment:2>
Django
Reporter: Xudonax | Owner: nobody
Type: Bug | Status: new
(models, ORM) | 1.7-beta-2
Severity: Normal | Resolution:
Keywords: get_or_create | Triage Stage:
Has patch: 0 | Unreviewed
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Xudonax):
I'd swear I used something like this in a previous version... But I guess
you know Django better than I do :)
I'll post it to the django-developers mailing-list, thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/22728#comment:3>
Django
Reporter: Xudonax | Owner: nobody
Type: Bug | Status: new
(models, ORM) | 1.7-beta-2
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by bmispelon):
* stage: Unreviewed => Accepted
Comment:
For reference, the mailing list discussion is here:
https://groups.google.com/forum/#!topic/django-developers/EbLRpYB_1WI
As someone else mentionned on the mailing list, I'm not sure either if we
can do something that works in the general case.
The fact that this "works" currently and that it silently drops the field
that has a lookup seems quite dangerous (and not intentional).
For this reason, I'd be in favor of throwing an error if a user attempts
to use lookups in a `get_or_create` call (similar to what happens if you
try and use one in `create()`).
I'll mark this as `accepted` on this basis.
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/22728#comment:4>
Django
Reporter: Xudonax | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: get_or_create | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timgraham):
* version: 1.7-beta-2 => master
--
Ticket URL: <https://code.djangoproject.com/ticket/22728#comment:5>
Django
Reporter: Xudonax | Owner: coldmind
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Keywords: get_or_create | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by coldmind):
* owner: nobody => coldmind
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/22728#comment:6>
Django
Reporter: Xudonax | Owner: coldmind
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: get_or_create | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by coldmind):
* has_patch: 0 => 1
Comment:
https://github.com/django/django/pull/3793
--
Ticket URL: <https://code.djangoproject.com/ticket/22728#comment:7>
Django
Reporter: Xudonax | Owner: coldmind
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: get_or_create | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by berkerpeksag):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/22728#comment:8>
Django
Reporter: Xudonax | Owner: coldmind
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: get_or_create | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by coldmind):
* needs_better_patch: 1 => 0
Comment:
https://github.com/django/django/pull/4779
--
Ticket URL: <https://code.djangoproject.com/ticket/22728#comment:9>
Django
Reporter: Xudonax | Owner: coldmind
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: get_or_create | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
FWIW, I have code in a current project, written only a few months ago,
that uses join lookups with `get_or_create`, and takes advantage of the
fact that they are used in the lookup but not the create. I don't think
it's an exotic use case, it's simply "find me a user that matches these
criteria, where the criteria are actually based on related objects, and if
no such user exists, create a user with the given defaults." That's
exactly the use case that `get_or_create` is intended for, with the
addition of "the criteria are based on related objects."
Even requiring any field mentioned in a lookup to be present in defaults
is potentially a backwards-incompatible breakage to such use cases. I
don't know how that change would even handle my case, as the relation I'm
querying on is not even a field present on User, it's a reverse FK from
another model, so there's no sensible corresponding value that could be
included in defaults.
I see the possible confusion here, but I don't see any way to address it
without reducing the utility of `get_or_create` in a backwards-
incompatible way. So I think we should make sure the documentation of
`get_or_create` is crystal clear on this point, and otherwise leave the
behavior alone.
--
Ticket URL: <https://code.djangoproject.com/ticket/22728#comment:10>
Django
Reporter: Xudonax | Owner: coldmind
Component: Database layer | Version: master
(models, ORM) |
Keywords: get_or_create | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by mjtamlyn):
* status: assigned => closed
* resolution: => invalid
Comment:
The documentation of `get_or_create` does devote a full paragraph and
sample code to this, so I don't think there's much to improve.
There is a case for an extension of the functionality which allows type-
idempotent transforms such as unaccent or iexact, but I feel this will be
super hard and prone to bugs. It's a possibility, but not worth keeping
this lying around for.
For longer discussion of why this ticket is being closed, see
https://github.com/django/django/pull/4779
--
Ticket URL: <https://code.djangoproject.com/ticket/22728#comment:11>
Django
Reporter: Xudonax | Owner: coldmind
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: get_or_create | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Andriy Sokolovskiy <me@…>):
In [changeset:"fc19f9315be312b5e5f00e0004b8696e36f852e7" fc19f93]:
{{{
#!CommitTicketReference repository=""
revision="fc19f9315be312b5e5f00e0004b8696e36f852e7"
Refs #22728 - Added missing tests for defaults__exact case
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22728#comment:12>
Django
Reporter: Xudonax | Owner: coldmind
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: invalid
Keywords: get_or_create | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Marc Tamlyn <marc.tamlyn@…>):
In [changeset:"1f28521e0ac81dcc660a5b9891f45a20306e093a" 1f28521]:
{{{
#!CommitTicketReference repository=""
revision="1f28521e0ac81dcc660a5b9891f45a20306e093a"
Merge pull request #4786 from coldmind/refs_22728
Refs #22728 - Added missing tests for defaults__exact case
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22728#comment:13>