Opened 21 months ago

Closed 20 months ago

Last modified 20 months ago

#30827 closed Bug (fixed)

bulk_create batch_size param overrides the compatible batch size calculation

Reported by: Ahmet Kucuk Owned by: Ahmet Kucuk
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Georgi Yanchev 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 (last modified by Ahmet Kucuk)

At this line: https://github.com/django/django/blob/stable/2.2.x/django/db/models/query.py#L1197

batch_size param overrides compatible batch size calculation. This looks like a bug as bulk_update properly picks the minimum of two:
https://github.com/django/django/blob/stable/2.2.x/django/db/models/query.py#L504

I suggest using similar

 batch_size = min(batch_size, max_batch_size) if batch_size else max_batch_size

logic in bulk_create as well. I am happy to open a PR for it.

Attachments (1)

ticket30827RegressionTests.diff (2.1 KB) - added by Chetan Khanna 20 months ago.
Adding regression tests

Download all attachments as: .zip

Change History (10)

comment:1 Changed 21 months ago by Ahmet Kucuk

Description: modified (diff)

comment:2 Changed 21 months ago by Georgi Yanchev

Cc: Georgi Yanchev added

comment:3 Changed 21 months ago by Claude Paroz

Has patch: set
Needs tests: set
Triage Stage: UnreviewedAccepted

I guess testing it will be harder than the fix!

comment:4 Changed 21 months ago by Claude Paroz

Component: UncategorizedDatabase layer (models, ORM)

Changed 20 months ago by Chetan Khanna

Adding regression tests

comment:5 Changed 20 months ago by Chetan Khanna

Needs tests: unset

Hi!
I added a couple of tests to the patch. Since this is the first time I'm contributing to django, I'd love to hear any feedback or advice.
Thanks.

comment:6 Changed 20 months ago by Mariusz Felisiak

Chetan Khanna, thanks, I added your test to the patch.

comment:7 Changed 20 months ago by Mariusz Felisiak

Owner: changed from nobody to Ahmet Kucuk
Status: newassigned
Triage Stage: AcceptedReady for checkin
Version: 2.2master
Last edited 20 months ago by Mariusz Felisiak (previous) (diff)

comment:8 Changed 20 months ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: assignedclosed

In 09578f6:

Fixed #30827 -- Made batch_size arg of QuerySet.bulk_create() respect DatabaseOperations.bulk_batch_size().

Thanks Chetan Khanna for tests.

comment:9 Changed 20 months ago by Chetan Khanna

Glad :)

Note: See TracTickets for help on using tickets.
Back to Top