[Django] #25343: pk attribute not set when having own primary key
Django
----------------------------------------------+--------------------
Reporter: IlianIliev | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: 1.8
Severity: Release blocker | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
Example:
{{{
# models.py
class Language(models.Model):
language = models.IntegerField(primary_key=True)
code = models.CharField(verbose_name='Language code (2 chars)',
blank=False, max_length=2)
# views.py
l = Language.objects.create(code='x')
print l.pk # return None
}}}
The above mention problem happens only if we have "primary_key=True"
--
Ticket URL: <https://code.djangoproject.com/ticket/25343>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: IlianIliev | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by IlianIliev):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Old description:
> Example:
>
> {{{
> # models.py
> class Language(models.Model):
> language = models.IntegerField(primary_key=True)
> code = models.CharField(verbose_name='Language code (2 chars)',
> blank=False, max_length=2)
>
> # views.py
> l = Language.objects.create(code='x')
> print l.pk # return None
> }}}
>
> The above mention problem happens only if we have "primary_key=True"
New description:
Example:
{{{
# models.py
class Language(models.Model):
language = models.IntegerField(primary_key=True)
code = models.CharField(verbose_name='Language code (2 chars)',
blank=False, max_length=2)
# views.py
l = Language.objects.create(code='x')
print l.pk # return None
}}}
The above mention problem happens only if we have "primary_key=True"
otherwise it works as expected.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/25343#comment:1>
Django
Reporter: IlianIliev | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* severity: Release blocker => Normal
Comment:
What is the expected behavior? I wouldn't expect an `IntegerField` to
automatically generate a primary key value.
--
Ticket URL: <https://code.djangoproject.com/ticket/25343#comment:2>
Django
Reporter: IlianIliev | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by IlianIliev):
The expected behaviour is that the `pk` property to be set to the value of
the primary key.
Because if you do like this
{{{
lang = Language.objects.create(code='x')
lang.pk is None # returns True
lang = Language.objects.get(code='x')
lang.pk is not None # for example 1
}}}
Then you have quite inconsistent behaviour
--
Ticket URL: <https://code.djangoproject.com/ticket/25343#comment:3>
Django
Reporter: IlianIliev | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
Is this really your exact code? Unless you have `null=True` on the
`language` field, that code should raise a database error like "null value
not allowed in column 'language'".
But it looks like your database is automatically setting a value for that
field instead of raising an error. So maybe your database schema doesn't
match your model definition? What database are you using, and what is the
actual schema for that table? Did Django create it, or did you create it
manually?
If you set a value for the `language` field, that will be the value of the
`pk` attribute.
If you don't want to have to explicitly set a value for the primary key,
you should be using `AutoField`, not `IntegerField` (or just let Django
automatically add an `AutoField` named `id`, as it does if you don't set
`primary_key=True` on any of your fields).
--
Ticket URL: <https://code.djangoproject.com/ticket/25343#comment:4>
Django
Reporter: IlianIliev | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by IlianIliev):
Replying to [comment:4 carljm]:
> Is this really your exact code? Unless you have `null=True` on the
`language` field, that code should raise a database error like "null value
not allowed in column 'language'".
>
> But it looks like your database is automatically setting a value for
that field instead of raising an error. So maybe your database schema
doesn't match your model definition? What database are you using, and what
is the actual schema for that table? Did Django create it, or did you
create it manually?
>
> If you set a value for the `language` field, that will be the value of
the `pk` attribute.
>
> If you don't want to have to explicitly set a value for the primary key,
you should be using `AutoField`, not `IntegerField` (or just let Django
automatically add an `AutoField` named `id`, as it does if you don't set
`primary_key=True` on any of your fields).
No actually the exact code is:
{{{
class Language(models.Model):
language = models.IntegerField(verbose_name='Language',
primary_key=True) #, choices=LANGUAGES, max_length=2)
code = models.CharField(verbose_name='Language code (2 chars)',
blank=False, max_length=2)
priority = models.PositiveIntegerField(verbose_name="Priority",
default=0, null=False, blank=True)
}}}
it is a legacy code, and I am not sure how the DB has been created but I
can reproduce it if I recreate the table with syncdb in a new DB (sqlite).
The value is automatically autoincremented.
I also found the the `language` property is also not set in the object
after create. However after getting the object from the DB everything is
fine.
--
Ticket URL: <https://code.djangoproject.com/ticket/25343#comment:5>
Django
Reporter: IlianIliev | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
It looks like `AutoField()` and `IntegerField(primary_key=True)` create
the same "integer PRIMARY KEY" type on SQLite, but Django doesn't know to
treat the latter definition as an `AutoField` (and I don't think it should
try to). Other databases (tested with PostgreSQL) with throw the expected
`IntegrityError` for a null value in the `language` column.
We could [https://docs.djangoproject.com/en/dev/ref/databases/ document
the issue] or add a database-specific check when SQLite is in use. Is it
worthwhile?
--
Ticket URL: <https://code.djangoproject.com/ticket/25343#comment:6>
Django
Reporter: IlianIliev | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by IlianIliev):
Replying to [comment:6 timgraham]:
> It looks like `AutoField()` and `IntegerField(primary_key=True)` create
the same "integer PRIMARY KEY" type on SQLite, but Django doesn't know to
treat the latter definition as an `AutoField` (and I don't think it should
try to). Other databases (tested with PostgreSQL) with throw the expected
`IntegrityError` for a null value in the `language` column.
>
> We could [https://docs.djangoproject.com/en/dev/ref/databases/ document
the issue] or add a database-specific check when SQLite is in use. Is it
worthwhile?
That makes sense. Regarding whether it should be documented or not - it is
a strange corner case caused by ugly legacy code that I stumbled upon so
probably this is quite a rare case and there is no need to add it to the
docs. It will stay documented here if anyone hits the same issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/25343#comment:7>
Django
doesn't treat it as such
-------------------------------------+-------------------------------------
Reporter: IlianIliev | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: 1.8
(models, ORM) |
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: new => closed
* resolution: => wontfix
Comment:
Okay, thanks for the feedback
--
Ticket URL: <https://code.djangoproject.com/ticket/25343#comment:8>
Django
doesn't treat it as such
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution: wontfix
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
For future reference, in case that decision ever changes (and frankly, I
wouldn't turn down a solid patch for this if someone supplied one), the
way to fix this would be with a db backend feature flag, I think;
something like `integer_pk_always_autoincrements`.
--
Ticket URL: <https://code.djangoproject.com/ticket/25343#comment:9>
Django
doesn't treat it as such
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution: wontfix
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
For what it's worth, I don't know if 'autoincrements' in the feature name
might be slightly misleading as the default behavior for SQLite is merely
[https://www.sqlite.org/autoinc.html to select as unused id]. Django does
add `AUTOINCREMENT` for `AutoField`, so what it generates for the two
definitions is actually not strictly equivalent in that respect.
--
Ticket URL: <https://code.djangoproject.com/ticket/25343#comment:10>
Django
doesn't treat it as such
-------------------------------------+-------------------------------------
Type: Bug | Status: closed
Component: Database layer | Version: 1.8
(models, ORM) |
Severity: Normal | Resolution: wontfix
| Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
Good point. Something like `integer_pk_always_autofills` might be better.
--
Ticket URL: <https://code.djangoproject.com/ticket/25343#comment:11>