Fixed #32406 -- Demonstration of how update and bulk_update could take advantage of returning rows. #19298
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
f40fa0d to
3762de9
Compare
charettes
commented
Mar 22, 2025
django/db/models/query.py
Outdated
There was a problem hiding this comment.
Should we take the same approach as Model.save to
- Return the dependent generated fields
- Clear the cached values of fields meant to be returned (generated, expressions) when
not can_return_rows_from_update
56a9d95 to
c323367
Compare
charettes
commented
Mar 24, 2025
django/db/models/query.py
Outdated
There was a problem hiding this comment.
Explicitly not named returning_fields as some backends (e.g. Postgres) allows any expressions to be returned so it could eventually be adapted not only to accept returning: Iterable[str] but also returning: Iterable[str | Resolvable].
50c03c4 to
1aeb4d0
Compare
charettes
commented
Mar 24, 2025
django/db/models/query.py
Outdated
Comment on lines
1308
to
1314
There was a problem hiding this comment.
Suggested change
| if not isinstance(returning, (bool, Iterable)): | |
| raise TypeError( | |
| f"{returning_type} must be either a boolean or an iterable." | |
| ) | |
| if returning is True: | |
| returning_fields = opts.local_concrete_fields | |
| else: | |
| if returning is True: | |
| returning_fields = opts.local_concrete_fields | |
| elif isinstance(returning, Iterable): | |
| ... | |
| else: | |
| raise TypeError( | |
| f"{returning_type} must be True or an iterable." | |
| ) |
charettes
commented
Mar 24, 2025
django/db/models/query.py
Outdated
Comment on lines
1239
to
1313
There was a problem hiding this comment.
These are only added to explain the signature handling bellow better before the docs are adjusted.
1aeb4d0 to
cbbb5c9
Compare
cbbb5c9 to
3e74c53
Compare
Determining if a field is db_returning based on the default connection feature availability prevent the usage of RETURNING for db_default fields in setups where non-default backends do support RETURNING. Whether or not the field should be attempted to be returned is already checked at the compiler level which is backend aware.
…ries. Renamed existing methods and abstractions used for INSERT … RETURNING to be generic enough to be used in the context of UPDATEs as well. Also consolidated SQL compliant implementations on BaseDatabaseOperations.
…date. This required implementing UPDATE RETURNING machinery that heavily borrows from the INSERT one.
…save(). Removed the can_return_columns_from_insert skip gates on existing field_defaults tests to confirm the expected number of queries are performed and that returning field overrides are respected.
Included an @overload to better explify signature.
3e74c53 to
c6995ea
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Trac ticket number
Not meant to be merged, just a demonstration of how the update returning logic proposed in #19285 is future proof.