[Django] #30827: bulk_create batch_size param overrides the compatible batch size calculation
Django
calculation
-----------------------------------------+------------------------
Reporter: Ahmet Kucuk | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 2.2
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 |
-----------------------------------------+------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/30827>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
calculation
Reporter: Ahmet Kucuk | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 2.2
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 Ahmet Kucuk:
Old description:
> 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.
New description:
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.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/30827#comment:1>
Django
calculation
Reporter: Ahmet Kucuk | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 2.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Georgi Yanchev):
* cc: Georgi Yanchev (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/30827#comment:2>
Django
calculation
Reporter: Ahmet Kucuk | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 2.2
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz):
* has_patch: 0 => 1
* needs_tests: 0 => 1
* stage: Unreviewed => Accepted
Comment:
I guess testing it will be harder than the fix!
--
Ticket URL: <https://code.djangoproject.com/ticket/30827#comment:3>
Django
calculation
Reporter: Ahmet Kucuk | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz):
* component: Uncategorized => Database layer (models, ORM)
--
Ticket URL: <https://code.djangoproject.com/ticket/30827#comment:4>
Django
calculation
Reporter: Ahmet Kucuk | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Chetan Khanna):
* Attachment "ticket30827RegressionTests.diff" added.
Adding regression tests
Django
calculation
Reporter: Ahmet Kucuk | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Chetan Khanna):
* needs_tests: 1 => 0
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/30827#comment:5>
Django
calculation
Reporter: Ahmet Kucuk | Owner: nobody
Type: Bug | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by felixxm):
Chetan Khanna, thanks, I added your test to the patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/30827#comment:6>
Django
calculation
Reporter: Ahmet Kucuk | Owner: Ahmet
| Kucuk
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* owner: nobody => Ahmet Kucuk
* status: new => assigned
* version: 2.2 => master
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/30827#comment:7>
Django
calculation
Reporter: Ahmet Kucuk | Owner: Ahmet
| Kucuk
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"09578f6dfb757c55f107e30a85434cedeb47465a" 09578f6]:
{{{
#!CommitTicketReference repository=""
revision="09578f6dfb757c55f107e30a85434cedeb47465a"
Fixed #30827 -- Made batch_size arg of QuerySet.bulk_create() respect
DatabaseOperations.bulk_batch_size().
Thanks Chetan Khanna for tests.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30827#comment:8>
Django
calculation
Reporter: Ahmet Kucuk | Owner: Ahmet
| Kucuk
Type: Bug | Status: closed
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Chetan Khanna):
Glad :)
--
Ticket URL: <https://code.djangoproject.com/ticket/30827#comment:9>