Skip to content

Conversation

charettes
Copy link
Member

@charettes charettes commented Jul 2, 2025

Trac ticket number

ticket-36490

Branch description

When bulk_create deals with an heterogeneous set of object with regards to primary key assignment that fits in a single batch there's no need to wrap the resulting single INSERT statement in a transaction.

Checklist

  • This PR targets the main branch.
  • The commit message is written in past tense, mentions the ticket number, and ends with a period.
  • I have checked the "Has patch" ticket flag in the Trac system.
  • I have added or updated relevant tests.
  • I have added or updated relevant docs, including release notes if applicable.
  • I have attached screenshots in both light and dark modes for any UI changes.

Copy link
Contributor

@shangxiao shangxiao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I glossed over finer details but generally seems good 🏆

@charettes charettes marked this pull request as ready for review July 2, 2025 06:26
@hermann-wenninger

This comment was marked as spam.

charettes added 3 commits July 2, 2025 13:38
When dealing with an heterogeneous set of object with regards to primary key
assignment that fits in a single batch there's no need to wrap the single
INSERT statement in a transaction.
…andling.

Whether or not returning_fields should be specified to _insert is not a
function of each batches so the conditional can be moved outside of the loop.
Asserting an upper bound for the number of executed queries can be achieved by
using CaptureQueriesContext instead of enabling the whole DEBUG machinery.
@charettes charettes changed the title Fixed #36490 -- Avoided unnecessary transaction creation in bulk_create. Fixed #36490 -- Avoided unnecessary transaction in bulk_create. Jul 2, 2025
@charettes
Copy link
Member Author

FWIW recent improvements in SQLCompiler for default eliding that landed in 4608d34 allow for heterogeneous set of objects (with and without pk) to be efficiently combined in a single INSERT as demonstrated here

self._for_write = True
fields = [f for f in opts.concrete_fields if not f.generated]
objs = list(objs)
objs_with_pk, objs_without_pk = self._prepare_for_bulk_create(objs)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears that this whole primary key assignment partition logic can also be elided if we'd like to go further

Copy link
Contributor

@sarahboyce sarahboyce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! This looks good to me 👍

@sarahboyce sarahboyce merged commit cd0966c into django:main Aug 15, 2025
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants