[Django] #36118: Some usage of bulk_batch_size might not account for composite primary keys on SQLite

[Django]

#36118: Some usage of bulk_batch_size might not account for composite primary keys
on SQLite
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Type: Bug
Status: new | Component: Database
| layer (models, ORM)
Version: 5.2 | Severity: Release
| blocker
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Brought up by Jacob Tyler Walls when reviewing proposed changes for
#36116.

-----

While the initially surfaced instance related to a distinct issue (see
ticket:27833#comment:13) there are definitive problems at least with
`bulk_update` which
[https://github.com/django/django/blob/22fc151bb86a553d84c62d7effd289356e9b6c6c/django/db/models/query.py#L895
doesn't expand] `pk` into multiple fields before calling `bulk_batch_size`
and could result in `OperationalError` due to too many parameters on
SQLite.

The `bulk_update` case should be solved and other usage of
`bulk_batch_size` audited.
--
Ticket URL: <https://code.djangoproject.com/ticket/36118>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36118: Some usage of bulk_batch_size might not account for composite primary keys
on SQLite
-------------------------------------+-------------------------------------

Reporter: Simon Charette | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 5.2
(models, ORM) |
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 Jacob Walls):

* stage: Unreviewed => Accepted

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

[Django]

#36118: Some usage of bulk_batch_size might not account for composite primary keys
on SQLite
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: (none)
Type: Bug | Status: new
Component: Database layer | Version: 5.2
(models, ORM) |
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
-------------------------------------+-------------------------------------

Comment (by Sarah Boyce):

I have been struggling to prove that the "pk" additions in `bulk_update`
are required to prevent an `OperationalError`. See
[https://github.com/django/django/pull/19088 draft PR]
This is also the only case I can find of `bulk_batch_size` with specific
logic for handling primary keys

If we agree these pk additions are not needed, then I think this is no
longer a release blocking bug but rather a cleanup/optimization ticket.
--
Ticket URL: <https://code.djangoproject.com/ticket/36118#comment:2>

[Django]

#36118: Some usage of bulk_batch_size might not account for composite primary keys
on SQLite
-------------------------------------+-------------------------------------

Reporter: Simon Charette | Owner: Sarah
| Boyce
Type: Bug | Status: assigned

Component: Database layer | Version: 5.2
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* has_patch: 0 => 1
* needs_better_patch: 0 => 1
* owner: (none) => Sarah Boyce
* status: new => assigned

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

[Django]

#36118: Some usage of bulk_batch_size might not account for composite primary keys
on SQLite
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Sarah
| Boyce
Type: Bug | Status: assigned
Component: Database layer | Version: 5.2
(models, ORM) |
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 Sarah Boyce):

* needs_better_patch: 1 => 0

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

[Django]

#36118: Some usage of bulk_batch_size might not account for composite primary keys
on SQLite
-------------------------------------+-------------------------------------
Reporter: Simon Charette | Owner: Sarah
| Boyce
Type: Bug | Status: assigned
Component: Database layer | Version: 5.2
(models, ORM) |
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

Comment:

After the discussions in the PR which investigated `bulk_batch_size`, we
found:

* The `max_query_params value` used for SQLite is overly protective in
most cases (see ticket #36143)
* There are other factors at play (possibly different settings) than just
the maximum numbers of query parameters that must be considered to have a
complete bullet proof solution (see ticket #36144)

This ticket aims to make sure the current situation (which isn't
perfect/bulletproof) is not worsened by the usage of composite primary
keys.
The solution here has been to have a linear increase (constant time the
number of additional fields in the primary key) of query parameters on
SQLite and Oracle. Further work is captured by the tickets referenced
above.
--
Ticket URL: <https://code.djangoproject.com/ticket/36118#comment:5>