Description

When using get_or_create with id=0, django does create an object, but doesn't write the id back in the returned object. The object is therefore not usable. We should get the new object's id, or at least get an error message to prevent using 0 as an id.

The problem appears with MySQL. sqlite3 works properly. I've not checked other backends.

I think that the problem is in django.db.models.fields.AutoField which is integer (see ​https://docs.djangoproject.com/en/1.3/ref/models/fields/#autofield) especially it can be zero. It should works like PositiveIntegerField or something like that. Supporting AutoField validation can be a solution, but it's backward incompatible. What's more, the validation is not necessary for backends other than MySQL because zero is valid value for id in most databases. We can check if id is not zero if the backend is MySQL otherwise don't validate id, but it makes the issue more comlex and it's still backward incompatible.

You could do a check in model.base_save(): connection.validate_pk(val), which should just raise an exception if the backend doesn't work with the given value. So, if you try to save a model with id=0, you would get an exception on MySQL on 0-valued PK value, and otherwise things would work as expected. This change should be simple to implement and safe to use. I don't think backwards compatibility for MySQL 0-valued PKs is worth worrying about: the current behavior can be considered a bug.

Another option is to just skip this issue by saying this is a bug in MySQL, not in Django. But as in this case it is pretty easy to prevent the confusing behavior, then why not?

FYI django's db backend features already keeps track of this misbehaviour for ​mysql using the allows_primary_key_0 feature. I haven't look where it's used but that's the reason why I did comment:1. It might be worth investigating where this flag is used.

Or the if connection.features.allows_primary_key_0 could be changed to if connection.ops.validate_autopk_value(0).

Now that I look at it, it seems the name for the validate_autopk_value is wrong, maybe check_autopk_value()?

I quickly tested primary key value of 0 using MySQL, and it seems to work. It is the automatic PK which doesn't work for zero value. So, it really isn't about allowing 0 as primary key value, it is about autopk values. I am no MySQL expert, so I might be wrong here.

In general I am opposed to these boolean allows_primary_key_0 type features. An external backend might disallow all non-positive values and they have no way to express that with the boolean. Granted, this one isn't particularly bad. And this really isn't this ticket's problem.

I attached a patch similar to akaariai's patch. It validates value for primary key in new method BaseDatabaseOperations.validate_autopk_value in module db.backends. The method is overriden in DatabaseOperations in module db.backends.mysql, so it does not use allows_primary_key_0 flag. The method is called by AutoField.get_db_prep_value from module db.models.fields. The patch includes simple test.

I'm not sure if get_db_prep_value is right place for validation. The doc of the method says:

Returns field's value prepared for interacting with the database
backend.
Used by the default implementations of ``get_db_prep_save``and
`get_db_prep_lookup```

so maybe we should add note that get_db_prep_value do validation and may raise error?

I am now getting the error "The database backend does not accept 0 as a value for AutoField." using the latest SVN version. What am I doing wrong? I'm not even using any AutoField (besides the django automatic id on all models)

Alright, I've managed to track what's my problem. I have a ForeignKey to the user model. If this value is 0 it will trigger this exception. I believe the patch isn't checking anywhere if it is in fact an autofield. Or this the desired behavior?

It seems you are trying to save 0 to a ForeignKey which points to an AutoField. But, this is illegal, as the AutoField will not accept that value and so the ForeignKey can not hold that value either.

I am closing this still as fixed - you can again open this, but please provide more details if you do. The above explanation leaves us guessing if there is some bug or not in the newly introduced behavior. A testcase/testproject would be really valuable.

EDIT: Didn't spot the error message earlier. That might be a little confusing... It says AutoField while the field infact is a ForeignKey. Is this the problem?

Sorry that I opened it twice. Yes, I guess I found it a bit confusing that it said AutoField. My first reaction was to check the id of the object, which was not 0 (in fact I was editing something).

But why is it illegal? I mean, it had worked before with no problem. I know it's not a good practice to have a foreign key as 0, but I don't understand how it can be illegal. Pointing a Foreign Key to 0 for me would be just like pointing to an id that doesn't exist, I don't see the difference. I did understand the bug you were trying to fix, but should this also apply to ForeignKeys?

So, you are creating ForeignKeys with value 0 knowing they point nowhere? You should set the ForeignKey as null=True and use None instead. You could get incorrect results from some queries if you use 0 instead of null=True.

I don't think ForeignKeys should be fixed to allow 0 as a value. Django's foreign keys are really meant to point at things if they have a value set.

Here is an example of my situation. I am collecting school test score data with a marker for demographics (e.g., Race/Ethnicity). Each school should only be allowed to have a single test score entry for each race. There should also be an allowance for a single entry for overall (no race). Logic suggests using a unique constraint on race and school and allowing race to be nullable. However, the SQL rules specify that null values within unique indexes are not counted as duplicates. (That was an unexpected discovery, BTW). So, I either have to create my own unique index field or take off the null constraint on the race field (shown below).

A reasonable value for this special fake-null field would be 0. If you create SQL directly without going through the django cursor, you can create a new field and then change the auto-generated field back to 0.

So, I guess there are two issues here.
1) It is possible to insert a new row with ID 0, but it requires a two-step process of insert followed by update.
2) Once you have this auto value of 0 (whether it was generated through Django or otherwise), you can't actually point at it!

This would actually fix the error, i.e. make everything work, rather than exporting the problem as a validation error. So I will try reopening the bug. If you disagree, close it again and I will go away.

Removing ready for checkin as there isn't anything to commit in this ticket.

Maybe we could allow users to set SQL_MODE to NO_AUTO_VALUE_ON_ZERO and then allow usage of zero as autopk value, or maybe we could just allow usage of zero in .update(). However the .update() option seems a bit hard to implement, as we would need information that we are updating to get_db_prep_value(), but there doesn't seem to be any way to do that.

To me it seems a database settings option ALLOW_AUTO_VALUE_ZERO for MySQL to switch the SQL mode, and to also switch off the autopk checking is the best approach.