[Django] #17713: allows_primary_key_0 is misnamed
Django
----------------------------------------------+--------------------
Reporter: claudep | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Database layer (models, ORM) | Version: SVN
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
In {{{django/db/backends/__init__.py}}}:
{{{
# Can an object have a primary key of 0? MySQL says No.
allows_primary_key_0 = True
}}}
This is not totally right. MySQL refuses 0 only if the primary key is an
autoincrement key. See also comments in #17653. At the very least, the
comment should be updated. And ideally, allows_primary_key_0 should be
renamed as (for example) allows_auto_pk_0.
--
Ticket URL: <https://code.djangoproject.com/ticket/17713>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: claudep | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: SVN
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by akaariai):
* cc: anssi.kaariainen@… (added)
* needs_docs: => 0
* needs_better_patch: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
This is a really minor issue, but also an easy one to fix.
If somebody with older MySQL versions could test this it would be valuable
information. I have tested this on 5.1 where id INTEGER PRIMARY KEY
accepts zero as a value. Based on that marking as accepted.
I think changing the name is the correct thing to do.
--
Ticket URL: <https://code.djangoproject.com/ticket/17713#comment:1>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by vajrasky):
PR: https://github.com/django/django/pull/1984
Later, in a separate ticket I will create a unit test for exercising the
MySQL capability of using non-autoincrement primary key with value 0.
--
Ticket URL: <https://code.djangoproject.com/ticket/17713#comment:2>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
I guess the only concern here is whether or not setting this attribute on
your own database backend is considered private API subject to change?
--
Ticket URL: <https://code.djangoproject.com/ticket/17713#comment:3>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by vajrasky):
This is the separate ticket for adding unit test exercising the capability
of having zero non-autoincrement primary key, #21517.
--
Ticket URL: <https://code.djangoproject.com/ticket/17713#comment:4>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by vajrasky):
@timo: or we can add another attribute (allows_auto_pk_0) and keep this
one (allows_primary_key_0).
--
Ticket URL: <https://code.djangoproject.com/ticket/17713#comment:5>
Django
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization | Version: master
Component: Database layer | Resolution: fixed
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"d3cf6cfacfb828faad4f4f97c904e259304649b3"]:
{{{
#!CommitTicketReference repository=""
revision="d3cf6cfacfb828faad4f4f97c904e259304649b3"
Fixed #17713 -- Renamed BaseDatabaseFeatures.allows_primary_key_0 to
allows_auto_pk_0.
MySQL does allow primary key with value 0. It only forbids autoincrement
primary key with value 0.
Thanks Claude Paroz for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/17713#comment:6>