Opened 5 years ago

Closed 5 years ago

Last modified 5 years 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 5 years ago.
Adding regression tests

Download all attachments as: .zip

Change History (10)

comment:1 by Ahmet Kucuk, 5 years ago

Description: modified (diff)

comment:2 by Georgi Yanchev, 5 years ago

Cc: Georgi Yanchev added

comment:3 by Claude Paroz, 5 years ago

Has patch: set
Needs tests: set
Triage Stage: Unreviewed → Accepted

I guess testing it will be harder than the fix!

comment:4 by Claude Paroz, 5 years ago

Component: Uncategorized → Database layer (models, ORM)

by Chetan Khanna, 5 years ago

Attachment: ticket30827RegressionTests.diff​ added

Adding regression tests

comment:5 by Chetan Khanna, 5 years ago

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 by Mariusz Felisiak, 5 years ago

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

comment:7 by Mariusz Felisiak, 5 years ago

Owner: changed from nobody to Ahmet Kucuk
Status: new → assigned
Triage Stage: Accepted → Ready for checkin
Version: 2.2 → master
Version 0, edited 5 years ago by Mariusz Felisiak (next)

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: → fixed
Status: assigned → closed

In 09578f6:

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

Thanks Chetan Khanna for tests.

comment:9 by Chetan Khanna, 5 years ago

Glad :)

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