#29358: loading a model with more than one primary_key field should fail
-------------------------------------+-------------------------------------
Reporter: zt_initech | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 2.0
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Description changed by zt_initech:
Old description:
> If a Django Model has two Fields with `primary_key=True`, Django will
> carry on without complaining at all, but silently corrupt data because
> the WHERE clause of UPDATE statement generated for a single model
> instance's `save()` will only use the first field that has
> `primary_key=True`. Same for `refresh_from_db()` and other things.
>
> I know that the documentation at
> https://docs.djangoproject.com/en/dev/ref/models/fields/#primary-key says
> "Only one primary key is allowed on an object.".
>
> I know about https://code.djangoproject.com/ticket/373 and this is not a
> duplicate of it.
>
> I know that migrate will fail with `django.db.utils.OperationalError:
> table "%s" has more than one primary key`, but people use Django with
> pre-existing database tables.
>
> This ticket is only about making Django fail when trying to load a Model
> with more than one primary key field, so that people won't try to use
> such a database table with Django.
>
> If there is some need to let Django access such tables, at least add a
> warning.
>
> Steps to reproduce:
>
> {{{#!bash
> mkdir compositepk
>
> rm -rf venv compositepk
> virtualenv -p `which python3.6` venv
> source venv/bin/activate
> pip install Django
> django-admin startproject compositepk
> cd compositepk/
> ./manage.py startapp app
> echo "INSTALLED_APPS = INSTALLED_APPS + ['app']" >>
> compositepk/settings.py
> VAR1=$(cat <<EOF
> from django.db import models
>
> class M(models.Model):
> f1 = models.IntegerField(primary_key=True)
> f2 = models.IntegerField(primary_key=True)
> f3 = models.IntegerField()
> EOF
> )
> echo "${VAR1}" > app/models.py
> unset VAR1
> ./manage.py migrate contenttypes
> ./manage.py migrate auth
> ./manage.py migrate admin
> ./manage.py migrate sessions
> echo "create table app_m (f1 integer not null, f2 integer not null, f3
> integer, primary key (f1, f2));" | sqlite3 db.sqlite3
> echo "insert into django_migrations (app, name, applied) values ('app',
> '0001_initial', datetime());" | sqlite3 db.sqlite3
> ./manage.py makemigrations
> ./manage.py migrate
> VAR1=$(cat <<EOF
> from app.models import M
> m = M.objects.create(f1=1, f2=1)
> m.refresh_from_db()
> m.f3 = 42
> m.save()
> from django.db import connection
> print(connection.queries[-3]['sql'])
> print(connection.queries[-1]['sql'])
> EOF
> )
> echo "${VAR1}" | ./manage.py shell
> unset VAR1
> deactivate
> cd ..
> }}}
>
> You see it prints:
>
> {{{
> SELECT "app_m"."f1", "app_m"."f2", "app_m"."f3" FROM "app_m" WHERE
> "app_m"."f1" = 1
> UPDATE "app_m" SET "f3" = 42 WHERE "app_m"."f1" = 1
> }}}
New description:
If a Django Model has two Fields with `primary_key=True`, Django will
carry on without complaining at all, but silently corrupt data because the
WHERE clause of UPDATE statement generated for a single model instance's
`save()` will only use the first field that has `primary_key=True`. Same
for `refresh_from_db()` and other things.
I know that the documentation at
https://docs.djangoproject.com/en/dev/ref/models/fields/#primary-key says
"Only one primary key is allowed on an object.".
I know about https://code.djangoproject.com/ticket/373 and this is not a
duplicate of it.
I know that migrate will fail with `django.db.utils.OperationalError:
table "%s" has more than one primary key`, but people use Django with pre-
existing database tables.
This ticket is only about making Django fail when trying to load a Model
with more than one primary key field, so that people won't try to use such
a database table with Django.
If there is some need to let Django access such tables, at least add a
warning.
Steps to reproduce:
{{{#!bash
mkdir compositepk
cd compositepk
rm -rf venv compositepk
virtualenv -p `which python3.6` venv
source venv/bin/activate
pip install Django
django-admin startproject compositepk
cd compositepk/
./manage.py startapp app
echo "INSTALLED_APPS = INSTALLED_APPS + ['app']" >>
compositepk/settings.py
VAR1=$(cat <<EOF
from django.db import models
class M(models.Model):
f1 = models.IntegerField(primary_key=True)
f2 = models.IntegerField(primary_key=True)
f3 = models.IntegerField()
EOF
)
echo "${VAR1}" > app/models.py
unset VAR1
./manage.py migrate contenttypes
./manage.py migrate auth
./manage.py migrate admin
./manage.py migrate sessions
echo "create table app_m (f1 integer not null, f2 integer not null, f3
integer, primary key (f1, f2));" | sqlite3 db.sqlite3
echo "insert into django_migrations (app, name, applied) values ('app',
'0001_initial', datetime());" | sqlite3 db.sqlite3
./manage.py makemigrations
./manage.py migrate
VAR1=$(cat <<EOF
from app.models import M
m = M.objects.create(f1=1, f2=1)
m.refresh_from_db()
m.f3 = 42
m.save()
from django.db import connection
print(connection.queries[-3]['sql'])
print(connection.queries[-1]['sql'])
EOF
)
echo "${VAR1}" | ./manage.py shell
unset VAR1
deactivate
cd ..
}}}
You see it prints:
{{{
SELECT "app_m"."f1", "app_m"."f2", "app_m"."f3" FROM "app_m" WHERE
"app_m"."f1" = 1
UPDATE "app_m" SET "f3" = 42 WHERE "app_m"."f1" = 1
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/29358#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/068.bf0aea00d52039e3debfc80a8f8e66dd%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
