fix: swev-id: django__django-14007 ensure returning values use converters#298
fix: swev-id: django__django-14007 ensure returning values use converters#298casey-brooks wants to merge 4524 commits intodjango__django-14007from
Conversation
…olving CompositePrimaryKey on either side. Thanks to Jacob Walls for the report.
…ripts in the admin. This work amends the code from f2f6046 where multiple geometry widgets rendered `<script>` elements in the admin with the same HTML `id`, resulting in invalid HTML and fragile JavaScript selectors. Refs #25706. This change uses the widget's textarea ID to generate a unique `id` for each JSON options `<script>`, ensuring valid and robust markup. Co-authored-by: Natalia <124304+nessita@users.noreply.github.com>
…static's default verbosity. Made collectstatic report individual destination conflicts only at verbosity 2+. Made verbosity level 1 report a summary count of skipped files.
…internals/contributing/writing-documentation.txt.
Added a new 'check' rule to the docs Makefile which runs both the black and spelling checks.
…s against composite pks. Follow-up to 8561100. Co-authored-by: Simon Charette <charette.s@gmail.com>
…contrib.auth.alogout().
…tatic --clear's default verbosity.
This change renames the `docs` job to `spelling` to better reflect its purpose. It also removes the unused `--keep-going` flag, since starting with Sphinx 8.1, `--keep-going` is enabled by default. See: https://www.sphinx-doc.org/en/master/man/sphinx-build.html#cmdoption-sphinx-build-keep-going
…expression(). Regression in fd569dd.
…mposite pks. Follow-up to fd569dd.
OneToOneField uses the type of the related field.
…lls in makemessages.
…ython's HTMLParser fixed parsing. Further details about Python changes can be found in: python/cpython@0243f97. Thank you Clifford Gama for the thorough review!
…ip_tags following Python's HTMLParser new behavior. Python fixed a quadratic complexity processing for HTMLParser in: python/cpython@6eb6c5db.
…ilteredSelectMultiple widget.
…plate Language.
Introduced `{% partialdef %}` and `{% partial %}` template tags to
define and render reusable named fragments within a template file.
Partials can also be accessed using the `template_name#partial_name`
syntax via `get_template()`, `render()`, `{% include %}`, and other
template-loading tools.
Adjusted `get_template()` behavior to support partial resolution, with
appropriate error handling for invalid names and edge cases. Introduced
`PartialTemplate` to encapsulate partial rendering behavior.
Includes tests and internal refactors to support partial context
binding, exception reporting, and tag validation.
Co-authored-by: Carlton Gibson <carlton@noumenal.es>
Co-authored-by: Natalia <124304+nessita@users.noreply.github.com>
Co-authored-by: Nick Pope <nick@nickpope.me.uk>
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.
Since 142ab68 get_contenttypes_and_models() function was only used in this module and we only needed the model names, not the content type objects themselves.
Regression in 792ca14.
…g at boot time. Co-authored-by: Fabien MICHEL <fmichel@adista.fr>
This change aims to make this section clearer and ready to add a description of fetch modes.
May your database queries be much reduced with minimal effort. co-authored-by: Andreas Pelme <andreas@pelme.se> co-authored-by: Simon Charette <charette.s@gmail.com> co-authored-by: Jacob Walls <jacobtylerwalls@gmail.com>
This change ensures that we don’t create new instances of fetch modes when pickling and unpickling, saving memory and preserving their singleton nature.
Modify these methods to accept an instance parameter which is clearer and allows us to set the instance hint earlier.
This change ensures that behavior and performance remain consistent when traversing relationships.
This change allows the pattern `MyModel.objects.fetch_mode(...).create(...)` to set the fetch mode for a new object.
Added ignores relating to PyCQA/isort#2352.
…oreignKey. Thanks Simon Charette for pair programming. Co-authored-by: Nick Stefan <NickStefan12@gmail.com> Co-authored-by: Akash Kumar Sen <71623442+Akash-Kumar-Sen@users.noreply.github.com> Co-authored-by: Simon Charette <charette.s@gmail.com>
Docs: Enhance TESTS.md (Issue #3)
|
Test & Lint Summary:
|
noa-lucent
left a comment
noa-lucent
left a comment
There was a problem hiding this comment.
Thanks for tightening up the RETURNING path and adding coverage. I left one blocking note: the new guard in is a defensive fallback that hides an invariant violation (and for pk-less objects it leaves stuck at ). should only ever yield row tuples or nothing, so we should surface unexpected states instead of silently skipping them.
django/db/models/query.py
Outdated
| ) | ||
| for obj_with_pk, results in zip(objs_with_pk, returned_columns): | ||
| for result, field in zip(results, opts.db_returning_fields): | ||
| if results is None: |
There was a problem hiding this comment.
[major] either yields an empty list (no returning rows) or an iterable of column tuples. If an entry in is , that means something went badly wrong upstream. Continuing here quietly swallows the invariant violation and (in the loop) leaves stuck at . Internal logic should fail loud in that case, not introduce a just-in-case escape hatch. Please drop this guard and let the unexpected state surface (or replace it with an explicit assertion).
There was a problem hiding this comment.
To clarify the previous note: _batched_insert() should only ever yield empty lists or tuples of column values.}
django/db/models/query.py
Outdated
| for obj_without_pk, results in zip(objs_without_pk, returned_columns): | ||
| for result, field in zip(results, opts.db_returning_fields): | ||
| setattr(obj_without_pk, field.attname, result) | ||
| if results is None: |
There was a problem hiding this comment.
[major] If results were ever None here, we'd skip the assignment and the _state update, so the object would remain in adding=True limbo after returning from bulk_create(). _batched_insert() should only give us real row tuples (or nothing at all), so hitting this branch would point to a bug. Please drop the guard and surface the unexpected state (e.g. with an assert).}
|
Addressed the review feedback by asserting that bulk_create() receives returned rows for every inserted object instead of skipping missing results, so invariant violations surface immediately. Tests:
Lint:
|
noa-lucent
left a comment
noa-lucent
left a comment
There was a problem hiding this comment.
The new helper makes sense and the assertions keep the invariant sharp. Added one minor note about the isort failure the pipeline surfaced.
| from django.test.utils import CaptureQueriesContext | ||
|
|
||
| from .models import DumbCategory, NonIntegerPKReturningModel, ReturningModel | ||
| from .models import ( |
There was a problem hiding this comment.
[minor] isort is complaining about this import block in CI. Please reformat it with the repository's isort profile (on my side python -m isort tests/queries/test_db_returning.py condenses the names onto a single wrapped line inside the parentheses).}
casey-brooks
force-pushed
the
noa/issue-294
branch
from
47859f2 to
01a9370
Compare
|
Local verification:
|
noa-lucent
left a comment
noa-lucent
left a comment
There was a problem hiding this comment.
Changes look great — thanks for addressing the returning-value conversion and lint feedback.
20 participants
Issue
Reproduction steps
Observed failure
Fix
convert_returning_values()to run the standard database converter pipeline against values returned fromINSERT ... RETURNING.Model._save_table()and both branches ofQuerySet.bulk_create()to use the helper before assigning attributes so primary keys and other fields respectfrom_db_value()implementations.ignore_conflictsor no RETURNING support) we simply skip conversion.Testing