Opened 4 days ago

Last modified 5 hours ago

#36064 assigned Bug

save() behavior for new model instance with default primary key does not apply to CompositePrimaryKey

Reported by: Jacob Walls Owned by: Csirmaz Bendegúz
Component: Database layer (models, ORM) Version: dev
Severity: Release blocker Keywords:
Cc: Csirmaz Bendegúz Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Based on the description of how save() is different than update_or_create() if the model's primary key has a default, i.e. that only an INSERT is attempted if the pk is not set by the user, I would expect models with a CompositePrimaryKey to behave as if they "have a default" if all of their composed fields have a default, or else for this to be called out in the doc.

This test shows this is not the case. The first test showing 2 queries instead of 1 is not such a serious problem, but the second test not raising IntegrityError seems like it could bite a user who was expecting to catch IntegrityError and do a custom update (based on how other models behave since Django 3.0 and #29260).

  • tests/composite_pk/models/tenant.py

    diff --git a/tests/composite_pk/models/tenant.py b/tests/composite_pk/models/tenant.py
    index ac0b3d9715..2273908031 100644
    a b  
     1import uuid
    12from django.db import models
    23
    34
    class Comment(models.Model):  
    4647
    4748class Post(models.Model):
    4849    pk = models.CompositePrimaryKey("tenant_id", "id")
    49     tenant = models.ForeignKey(Tenant, on_delete=models.CASCADE)
    50     id = models.UUIDField()
     50    tenant = models.ForeignKey(Tenant, on_delete=models.CASCADE, default=1)
     51    id = models.UUIDField(default=uuid.uuid4)
  • tests/composite_pk/test_create.py

    diff --git a/tests/composite_pk/test_create.py b/tests/composite_pk/test_create.py
    index 7c9925b946..af7ceb7df2 100644
    a b  
     1from django.db import IntegrityError
    12from django.test import TestCase
    23
    3 from .models import Tenant, User
     4from .models import Post, Tenant, User
    45
    56
    67class CompositePKCreateTests(TestCase):
    class CompositePKCreateTests(TestCase):  
    1415            id=1,
    1516            email="user0001@example.com",
    1617        )
     18        cls.post = Post.objects.create()
     19
     20    def test_save_default_pk_not_set(self):
     21        with self.assertNumQueries(1):
     22            Post().save()
     23
     24    def test_save_default_pk_set(self):
     25        with self.assertRaises(IntegrityError):
     26            Post(tenant_id=1, id=self.post.id).save()
     27
    1728
    1829    def test_create_user(self):
    1930        test_cases = (

Change History (8)

comment:1 by Sarah Boyce, 2 days ago

Cc: Csirmaz Bendegúz added
Triage Stage: UnreviewedAccepted

Thank you Jacob! Agree this needs either a fix or some docs

comment:2 by mao xin chen, 2 days ago

This is my first time trying to submit code. Please see if it can meet the requirements. If so, I will learn to submit PR. Thanks

Version 0, edited 2 days ago by mao xin chen (next)

in reply to:  2 comment:3 by Csirmaz Bendegúz, 37 hours ago

Replying to mao xin chen:

This is my first time trying to submit code. Please see if it can meet the requirements. If so, I will learn to submit PR. Thanks

Thanks for the patch! Please check the docs on how to contribute. If you open a PR on GitHub we can review your code.

comment:4 by Csirmaz Bendegúz, 36 hours ago

Thanks Jacob!

n default obj.save() conflict
1 yes insert IntegrityError
2 no update & insert update
  1. If pk has a default or db_default, saving a new object with a conflicting pk attempts an insert and raises an IntegrityError
  2. If pk doesn't have a default or db_default, saving a new object with a conflicting pk attempts an update (if no rows affected attempts an insert)

Is that correct?

# 1. Model.id has a default
foo = Model(id=1)
foo.save() # INSERT
bar = Model(id=1)
bar.save() # INSERT & IntegrityError

# 2. Model.id doesn't have a default
foo = Model(id=1)
foo.save() # UPDATE & INSERT
bar = Model(id=1)
bar.save() # UPDATE
Last edited 36 hours ago by Csirmaz Bendegúz (previous) (diff)

comment:5 by Csirmaz Bendegúz, 35 hours ago

Has patch: set

comment:6 by Sarah Boyce, 34 hours ago

Owner: set to Csirmaz Bendegúz
Status: newassigned

comment:7 by Sarah Boyce, 5 hours ago

Triage Stage: AcceptedReady for checkin

comment:8 by Sarah Boyce, 5 hours ago

Triage Stage: Ready for checkinAccepted
Note: See TracTickets for help on using tickets.
Back to Top