[Django] #36075: Field.primary_key documentation should details its interaction with CompositePrimaryKey

26 views
Skip to first unread message

Django

unread,
Jan 9, 2025, 12:51:46 AM1/9/25
to django-...@googlegroups.com
#36075: Field.primary_key documentation should details its interaction with
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Type:
| Cleanup/optimization
Status: new | Component:
| Documentation
Version: dev | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
A particular focus should be put on the fact that the flag will and should
only be set to `True` if the field is the single member of the primary
key. When a composite primary is defined on the model this flag will be
`False` for all the fields defined on the model and `_meta.pk_fields`
(which should be documented) should be used instead to build composite
primary key ready third-party application.

In other words, it should be made clear that code that relies on
`Field.primary_key` is not composite field ready and that
`_meta.pk_fields` should be used instead.
--
Ticket URL: <https://code.djangoproject.com/ticket/36075>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jan 9, 2025, 12:52:31 AM1/9/25
to django-...@googlegroups.com
#36075: Field.primary_key documentation should details its interaction with
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: Documentation | Version: dev
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 Simon Charette:

Old description:

> A particular focus should be put on the fact that the flag will and
> should only be set to `True` if the field is the single member of the
> primary key. When a composite primary is defined on the model this flag
> will be `False` for all the fields defined on the model and
> `_meta.pk_fields` (which should be documented) should be used instead to
> build composite primary key ready third-party application.
>
> In other words, it should be made clear that code that relies on
> `Field.primary_key` is not composite field ready and that
> `_meta.pk_fields` should be used instead.

New description:

A particular focus should be put on the fact that the flag will and should
only be set to `True` if the field is the single member of the primary
key. When a composite primary is defined on the model this flag will be
`False` for all the fields defined on the model and `_meta.pk_fields`
(which should be documented) should be used instead to build composite
primary key ready third-party application.

In other words, it should be made clear that code that relies solely on
`Field.primary_key` is not composite primary key ready and that
`_meta.pk_fields` should be used instead.

--
--
Ticket URL: <https://code.djangoproject.com/ticket/36075#comment:1>

Django

unread,
Jan 9, 2025, 12:53:14 AM1/9/25
to django-...@googlegroups.com
#36075: Field.primary_key documentation should details its interaction with
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: Documentation | Version: dev
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 Simon Charette:

Old description:

> A particular focus should be put on the fact that the flag will and
> should only be set to `True` if the field is the single member of the
> primary key. When a composite primary is defined on the model this flag
> will be `False` for all the fields defined on the model and
> `_meta.pk_fields` (which should be documented) should be used instead to
> build composite primary key ready third-party application.
>
> In other words, it should be made clear that code that relies solely on
> `Field.primary_key` is not composite primary key ready and that
> `_meta.pk_fields` should be used instead.

New description:

A particular focus should be put on the fact that the flag will and should
only be set to `True` if the field is the single member of the primary
key. When a composite primary is defined on the model this flag will be
`False` for all the fields defined on the model and `_meta.pk_fields`
(which should be documented) should be used instead to build composite
primary key ready third-party application.

In other words, it should be made clear that code that relies solely on
`Field.primary_key` is not composite primary key ready and that
`_meta.pk_fields` should be used instead. I feel like this is something
that should be clearly pointed out in the release notes as well.

--
--
Ticket URL: <https://code.djangoproject.com/ticket/36075#comment:2>

Django

unread,
Jan 9, 2025, 12:55:06 AM1/9/25
to django-...@googlegroups.com
#36075: Field.primary_key documentation should details its interaction with
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: Documentation | Version: dev
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 Simon Charette:

Old description:

> A particular focus should be put on the fact that the flag will and
> should only be set to `True` if the field is the single member of the
> primary key. When a composite primary is defined on the model this flag
> will be `False` for all the fields defined on the model and
> `_meta.pk_fields` (which should be documented) should be used instead to
> build composite primary key ready third-party application.
>
> In other words, it should be made clear that code that relies solely on
> `Field.primary_key` is not composite primary key ready and that
> `_meta.pk_fields` should be used instead. I feel like this is something
> that should be clearly pointed out in the release notes as well.

New description:

A particular focus should be put on the fact that the flag will and should
only be set to `True` if the field is the single member of the primary
key. When a composite primary is defined on the model this flag will be
`False` for all the fields defined on the model and `_meta.pk_fields`
(which should be documented) should be used instead to build composite
primary key ready third-party application.

In other words, it should be made clear that code that relies solely on
`Field.primary_key` is not composite primary key ready and that
`_meta.pk_fields` should be used instead. I feel like this is something
that should be clearly pointed out in the composite primary key
documentation on how to make reusable app code composite primary key
ready.

--
--
Ticket URL: <https://code.djangoproject.com/ticket/36075#comment:3>

Django

unread,
Jan 9, 2025, 2:26:13 AM1/9/25
to django-...@googlegroups.com
#36075: Field.primary_key documentation should details its interaction with
CompositePrimaryKey
--------------------------------------+------------------------------------
Reporter: Simon Charette | Owner: (none)
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: dev
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Sarah Boyce):

* severity: Normal => Release blocker
* stage: Unreviewed => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/36075#comment:4>

Django

unread,
Jan 9, 2025, 2:37:09 AM1/9/25
to django-...@googlegroups.com
#36075: Field.primary_key documentation should details its interaction with
CompositePrimaryKey
--------------------------------------+------------------------------------
Reporter: Simon Charette | Owner: (none)
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: dev
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Sarah Boyce):

* cc: Csirmaz Bendegúz (added)

Comment:

From #36074
> If you grep for .primary_key you'll notice that most of the checks
against this field need to be adjusted to use field in opts.pk_fields
instead which makes me wonder if we took the right decision by not setting
this flag to True when included as a member of CompositePrimaryKey.
--
Ticket URL: <https://code.djangoproject.com/ticket/36075#comment:5>

Django

unread,
Jan 9, 2025, 11:26:50 AM1/9/25
to django-...@googlegroups.com
#36075: Field.primary_key documentation should details its interaction with
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Simon
Type: | Charette
Cleanup/optimization | Status: assigned
Component: Documentation | Version: dev
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):

* owner: (none) => Simon Charette
* status: new => assigned

Comment:

Sleeping on it I think it's fine that we keep things as they are code wise
on the assignment of `primary_key = False` when used with
`CompositePrimaryKey` as long as we clearly document that
`_meta.pk_fields` should be used. The reason is that many of the
`field.primary_key` usages immediately stop when they first their first
match so they are likely broken anyway.

Forcing third-party applications that wish to support composite primary
keys to migrate to `_meta.pk_fields` and abandon `.primary_key` seem like
a better option that having code implicitly break because they only expect
one field to have `primary_key = True` which is decade old assumption.
--
Ticket URL: <https://code.djangoproject.com/ticket/36075#comment:6>

Django

unread,
Jan 12, 2025, 11:18:02 PM1/12/25
to django-...@googlegroups.com
#36075: Field.primary_key documentation should details its interaction with
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Simon
Type: | Charette
Cleanup/optimization | Status: assigned
Component: Documentation | Version: dev
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Simon Charette):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36075#comment:7>

Django

unread,
Jan 13, 2025, 6:05:10 AM1/13/25
to django-...@googlegroups.com
#36075: Field.primary_key documentation should details its interaction with
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Simon
Type: | Charette
Cleanup/optimization | Status: assigned
Component: Documentation | Version: dev
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"bf7b17d16d3978b2e1cee4a0f7ce8840bd1a8dc4" bf7b17d1]:
{{{#!CommitTicketReference repository=""
revision="bf7b17d16d3978b2e1cee4a0f7ce8840bd1a8dc4"
Refs #36075 -- Used field in pk_fields over field.primary_key.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36075#comment:8>

Django

unread,
Jan 14, 2025, 6:16:00 AM1/14/25
to django-...@googlegroups.com
#36075: Field.primary_key documentation should details its interaction with
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Simon
Type: | Charette
Cleanup/optimization | Status: assigned
Component: Documentation | Version: dev
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/36075#comment:9>

Django

unread,
Jan 14, 2025, 10:20:29 AM1/14/25
to django-...@googlegroups.com
#36075: Field.primary_key documentation should details its interaction with
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Simon
Type: | Charette
Cleanup/optimization | Status: closed
Component: Documentation | Version: dev
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce <42296566+sarahboyce@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"e580926d74f4d5788feac05ac1d50626936631d7" e580926d]:
{{{#!CommitTicketReference repository=""
revision="e580926d74f4d5788feac05ac1d50626936631d7"
Fixed #36075 -- Documented how to introspect composite primary keys.

Document _meta.pk_fields and interactions between Field.primary_key and
CompositePrimaryKey.

Thanks Mariusz for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36075#comment:10>

Django

unread,
Jan 14, 2025, 10:21:39 AM1/14/25
to django-...@googlegroups.com
#36075: Field.primary_key documentation should details its interaction with
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Simon
Type: | Charette
Cleanup/optimization | Status: closed
Component: Documentation | Version: dev
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"f07360e8087d3b403d1d12ff696da3138116055a" f07360e8]:
{{{#!CommitTicketReference repository=""
revision="f07360e8087d3b403d1d12ff696da3138116055a"
Refs #36075 -- Adjusted MTI handling of _non_pk_concrete_field_names.

Regression in bf7b17d16d3978b2e1cee4a0f7ce8840bd1a8dc4.

Thanks Sage Abdullah for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36075#comment:13>

Django

unread,
Jan 14, 2025, 10:21:39 AM1/14/25
to django-...@googlegroups.com
#36075: Field.primary_key documentation should details its interaction with
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Simon
Type: | Charette
Cleanup/optimization | Status: closed
Component: Documentation | Version: dev
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"161e79d277ffe8b79b15ad51cb0d23de54270202" 161e79d2]:
{{{#!CommitTicketReference repository=""
revision="161e79d277ffe8b79b15ad51cb0d23de54270202"
Refs #36075 -- Adjusted pk_fields usage in bulk_update eligibility checks.

Regression in bf7b17d16d3978b2e1cee4a0f7ce8840bd1a8dc4.

Thanks Sage Abdullah for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36075#comment:12>

Django

unread,
Jan 14, 2025, 10:21:39 AM1/14/25
to django-...@googlegroups.com
#36075: Field.primary_key documentation should details its interaction with
CompositePrimaryKey
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Simon
Type: | Charette
Cleanup/optimization | Status: closed
Component: Documentation | Version: dev
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"4bfec242b488b174f7d386ab5bbd3363751cdb93" 4bfec242]:
{{{#!CommitTicketReference repository=""
revision="4bfec242b488b174f7d386ab5bbd3363751cdb93"
Fixed #36093 -- Adjusted unique checks to account for inherited primary
keys.

Regression in bf7b17d16d3978b2e1cee4a0f7ce8840bd1a8dc4 refs #36075.

Thanks Sage Abdullah for the report and tests.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36075#comment:11>
Reply all
Reply to author
Forward
0 new messages