#36118 closed Bug (fixed)
Some usage of bulk_batch_size might not account for composite primary keys on SQLite and Oracle
| Reported by: | Simon Charette | Owned by: | Sarah Boyce |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | 5.2 |
| Severity: | Release blocker | Keywords: | |
| Cc: | Jacob Walls | Triage Stage: | Ready for checkin |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
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 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.
Change History (8)
comment:1 by , 5 weeks ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:2 by , 4 weeks ago
comment:3 by , 4 weeks ago
| Has patch: | set |
|---|---|
| Owner: | set to |
| Patch needs improvement: | set |
| Status: | new → assigned |
comment:4 by , 4 weeks ago
| Patch needs improvement: | unset |
|---|
comment:5 by , 3 weeks ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
After the discussions in the PR which investigated bulk_batch_size, we found:
- The
max_query_params valueused 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.
comment:6 by , 3 weeks ago
| Summary: | Some usage of bulk_batch_size might not account for composite primary keys on SQLite → Some usage of bulk_batch_size might not account for composite primary keys on SQLite and Oracle |
|---|
I have been struggling to prove that the "pk" additions in
bulk_updateare required to prevent anOperationalError. See draft PRThis is also the only case I can find of
bulk_batch_sizewith specific logic for handling primary keysIf 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.