Problem with ``Model.objects.create``
Yo-Yo Ma
If you have a model:
class Foo(Model):
spam= CharField(max_length=30)
xyz= CharField(max_length=30)
def __unicode__(self):
return self.xyz
and you use it in the shell like this:
>>> Foo.objects.create(xyz="Hello")
<Foo: Hello>
>>>
No IntegrityError was raised, even though ``spam`` is a required field.
Russell Keith-Magee
Yes, 'spam' is a required field. And if you investigate a little
closer, you'll see that it has a value -- the empty string.
You'll note that the following also works, and is entirely consistent
with get_or_create():
>>> obj = Foo(xyz='Hello')
>>> obj.save()
For reasons of historical significance, the default value for all
fields (CharField or otherwise) is "", unless:
* a default is provided by the user, or
* You've overridden get_default, or
* You're using Oracle (which has it's own special difficulties with "" vs None)
Yours,
Russ Magee %-)
Yo-Yo Ma
thought that Django raised an IntegrityError in this case, and when it
didn't happen I figured it was a bug.
I will make a recommendation (observation). To provide a blank value a
a model and have it validate with full_clean, you have to specify
blank=True. I would suggest that to maintain integrity between your
applications logic and the database, Django should raise an
IntegrityError, if blank=False.
On Jan 2, 6:53 pm, Russell Keith-Magee <russ...@keith-magee.com>
wrote:
Stephen Burrows
integrity of the database is affected, e.g. a foreign key check fails,
duplicate key, etc." [1] Since the creation of a model instance
doesn't affect the database, it should definitely not raise an
IntegrityError. Additionally, blank=True doesn't have anything to do
with database structure: it's purely for validation django-side. The
database doesn't care whether something is blank or not, so saving an
object (or using Foo.objects.create()) with a blank value in a
required field will also not raise an IntegrityError.
That being said, if a model has a blank value in a required field, a
ValidationError should be raised by the full_clean method, and if
that's not happening, it's a bug. If you're creating a model instance
and want to run validation before you it's saved to the database,
you'll want to do the following:
>>> obj = Foo(xyz='Hello')
(Here a ValidationError will be raised. In actual code you would want
to use try/except.)
>>> obj.spam = "w/e"
>>> obj.full_clean() # since the model instance will now validate, this won't raise an exception.
>>> obj.save()
If the ValidationError is being raised correctly, then this might be a
topic more for django-users than django-developers.
Best,
Stephen
[1] http://code.djangoproject.com/wiki/IntegrityError
Yo-Yo Ma
> doesn't affect the database, it should definitely not raise an
Model creation (ie, the use of ``Model.create()`` does. I'm not
arguing that instantiation should raise an exception. I'm arguing that
saving it to the database should (ie, using ``create()``, or
explicitly ``save()``).
Put simply: In general, you cannot "create" a Foo that isn't exactly a
Foo. If it isn't exactly a Foo, it isn't a Foo at all. This is
undeniable logic, no?
Python Example:
class MyClass(object):
def __init__(self, name):
self.name = name
>>> instance = MyClass()
...
TypeError: __init__() takes exactly 2 arguments (1 given)
I don't have to do this to find out I screwed up:
>>> instance = MyClass()
>>> instance.is_messed_up()
TypeError: __init__() takes exactly 2 arguments (1 given)
The difference in Django is that instantiation doesn't represent
"creation". Saving to the database does, and since the ORM is an
abstraction of the database, you don't care that the DB is ok with an
empty VARCHAR value. You only care about your model's required value.
I'm suggesting this:
class Person(Model):
name = CharField(max_length=50)
>>> Person() # Should work fine (assign attributes later, etc)
>>> Person.objects.create()
... Should raise some kind of exception before allowing your model's
"INTEGRITY" to be violated.
Waylan Limberg
[snip]
> The difference in Django is that instantiation doesn't represent
> "creation". Saving to the database does, and since the ORM is an
> abstraction of the database, you don't care that the DB is ok with an
> empty VARCHAR value. You only care about your model's required value.
> I'm suggesting this:
>
> class Person(Model):
> name = CharField(max_length=50)
>
>
>>>> Person() # Should work fine (assign attributes later, etc)
>>>> Person.objects.create()
> ... Should raise some kind of exception before allowing your model's
> "INTEGRITY" to be violated.
>
No it shouldn't as no data validation has been run. Remember, like
save(), create() does not run validation on the instance before
writing to the db. As Russ pointed out, the actual data written to
the db is valid as far as the db is concerned. Therefore, you get no
errors. If you want to ensure the data is valid (as defined in your
python code - not per the db) then you need to validate it in python
*before* writing to the db. Therefore, don't use
Person.objects.create()!!!!
That being said, it does strike me as being a little odd that there is
no way to validate your data when using create() or get_or_create().
In fact, the docs [1] describe both those methods as shortcuts. But if
you're writing an app that always needs to validate your data, then
you will never use those shortcuts which makes them rather useless. Of
course, they existed long before (the current) model validation
mechanism did, so I wonder if it would be reasonable to propose that
they (at least optionally) be updated to run validation before saving.
Or perhaps a warning in the docs that create() and get_or_create() do
not run validation before writing to the db may be appropriate.
[1]: http://docs.djangoproject.com/en/dev/ref/models/querysets/#create
--
----
\X/ /-\ `/ |_ /-\ |\|
Waylan Limberg
Yo-Yo Ma
>> ...it does strike me as being a little odd that there is no way to validate your data...
Let me get this straight:
I just shouldn't "use Person.objects.create()!!!!", if I ever want to
to take advantage of any protection that the ORM provides since there
is "no way to validate" my data? Then, remove ``create()`` from the
ORM. It's not part of the ORM, if that's the case. It's a unintended
booby trap for developers who to accidentally ruin their data because
of naive data input assumptions or faulty validation logic somewhere
along the way, as I did (fortunately during development). It still
provides SQL injection protection (ie, not just "writing to the db"),
so why not basic model integrity protection?
Simple question (A or B): Would you rather A) have a
SomeDjangoException come up where you don't expect it and a visitor
see a 500 page, or B) have your data ruined by a developer mistake?
That's not a loaded question. That's the exact issue I'm speaking
about. Your data is ruined when your code's logic (the view) can no
longer assume that the data's interface (the ORM model) is still valid.
Marty Alchin
> Simple question (A or B): Would you rather A) have a
> SomeDjangoException come up where you don't expect it and a visitor
> see a 500 page, or B) have your data ruined by a developer mistake?
Developer mistakes can ruin your data. Full stop.
That has nothing to do with Django, its ORM or the create() method
being discussed here. A developer mistake *always* has the potential
to severely cripple a website or much, much worse. Sure, we could
argue all day about whether this one particular method could add some
extra protections, but please don't try to kid yourself (or any of us)
into thinking that "fixing" create() will somehow make websites safe
from mistakes.
The fact of the matter is this: create() has a particular behavior,
and that behavior is documented. Sure, the documentation *could* be
more clear, but if we wanted to call out every potentially disastrous
mistake in bold with flashing lights and sirens, the vast majority of
the documentation would look like that, and nobody would ever read any
of it. Worse yet, it still couldn't possibly cover the full range of
possible mistakes, so anybody who did bother to read it would still be
wide open to a whole range of problems. It's our responsibility as
developers to understand the way our tools work and use them
accordingly. No amount of framework design will ever substitute for a
well-informed developer using it.
> Your data is ruined when your code's logic (the view) can no
> longer assume that the data's interface (the ORM model) is still valid.
That's just it: the data's interface (the ORM model) *is* still valid.
It's as valid as it's documented to be. An ORM is a device that
communicates between a database and an object. As long as there's
enough information for those two sides to talk to each other, a
conversation can take place. Anything that needs to happen beyond that
is, as you mentioned, your code's logic. You can encapsulate some of
that logic in a full_clean() method and call it separately to make
sure your objects are valid according your code's logic, but all the
ORM itself cares about is whether objects are valid according to the
database requirements.
Unfortunately, this is one of the many issues where "X doesn't work
the way I think it should" gets confused with "X doesn't work the way
it should" or even "X doesn't work." If you have a constructive
argument for how you think it should work that can be implemented in a
way that doesn't break compatibility with other people's code, feel
free to propose that and discuss it. But spinning around about how
"wrong" the behavior is currently doesn't help anybody.
-Marty
Yo-Yo Ma
> the way I think it should" gets confused with "X doesn't work the way
> it should" or even "X doesn't work." If you have a constructive
> argument for how you think it should work that can be implemented in a
> way that doesn't break compatibility with other people's code, feel
> free to propose that and discuss it. But spinning around about how
> "wrong" the behavior is currently doesn't help anybody.
> ...constructive argument...
Your thesis about the semantics of suggesting features is more
constructive?
Suggesting an idea here gets you just about the same as joining an
elementary school hide-and-go-seek game and trying to change the rules.
Jacob Kaplan-Moss
> What's with all the hostility here? I've heard about this from others.
Let's just stop this, right now. There's no hostility in Marty's tone
-- if you're reading that, then take the night off and come back to
django-dev tomorrow.
This discussion is starting to go down the path of many other ones
you've been involved in in the past, and you need to stop right now.
I've twice warned you privately, and now I'll warn you publicly: ad
homonym attacks and hostile, personal notes like yours aren't
acceptable here. Stop now.
Jacob
Yo-Yo Ma
Are you saying, that's not hostile? And, which ad hominem attack/s are
you referring to? I'm being serious. If I'm going to the gauntlets,
can you at least tell me how you came to the conclusion that I used ad
hominem?
Valts
> That being said, it does strike me as being a little odd that there is
> no way to validate your data when using create() or get_or_create().
> In fact, the docs [1] describe both those methods as shortcuts. But if
> you're writing an app that always needs to validate your data, then
> you will never use those shortcuts which makes them rather useless. Of
> course, they existed long before (the current) model validation
> mechanism did, so I wonder if it would be reasonable to propose that
> they (at least optionally) be updated to run validation before saving.
get_or_create() when I don't want the object to be saved into the
database if it does not validate. There could be added another special
keyword argument "full_clean", "validate" or similar to
get_or_create(). If the proposed idea gets the green light I am
willing to create the ticket and the necessary patches.
In case of create() IMO there is no need for any changes as create()
is really simple and I'd rather make create_clean() shortcut in my own
project without polluting keyword arguments with special cases.
Best regards,
Valts.
Dan Watson
Just my $0.02: I think that such feature would be useful when using
get_or_create() when I don't want the object to be saved into the
database if it does not validate. There could be added another special
keyword argument "full_clean", "validate" or similar to
get_or_create(). If the proposed idea gets the green light I am
willing to create the ticket and the necessary patches.