{{ message }}
Subqueryload without per-row Row construction — ~26% on subqueryload#13404
Open
ollz272 wants to merge 4 commits into
Open
Subqueryload without per-row Row construction — ~26% on subqueryload#13404ollz272 wants to merge 4 commits into
ollz272 wants to merge 4 commits into
Conversation
_SubqCollections._load iterated the Query, which routes through Query.__iter__ -> result.unique() and builds a Row object plus runs the uniquing filter for every row. Execute the statement directly and group the plain processed tuples from Result._raw_all_tuples() instead. The legacy Query iteration always applied result.unique(), so replicate that dedup explicitly: key the related object on identity (matching the ORM use_id_for_hash uniquing filter) and the remaining columns by value. The Row-based unique() path is retained for the requires_uniquing case (e.g. a joinedload nested inside the subqueryload).
Add SubqueryloadRawTupleTest exercising _SubqCollections._load after it switched from iterating the legacy Query (Row + result.unique()) to consuming Result._raw_all_tuples() with an explicit dedup. Covers: - basic o2m collection grouping and per-collection order - scalar/m2o dedup with distinct_target_key=False, asserting no 'Multiple rows returned with uselist=False' SAWarning is escalated - subqueryload combined with .distinct() - order_by across a join producing duplicate subquery rows - requires_uniquing fallback: joinedload of a collection nested inside a subqueryload (retained Row-based result.unique() path) - populate_existing() - empty collection / None scalar when no related rows - subqueryload -> subqueryload and many-to-many chains - bound-param / .params() filtered queries and param reuse
…/subquery-raw-tuples
Member
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
_SubqCollections._loaditerated theQuery, which routes throughQuery.__iter__ -> result.unique()and builds aRowobject (plus runs theuniquing filter) for every row. Execute the statement directly and group the
plain processed tuples from
Result._raw_all_tuples()instead.The legacy Query iteration always applied
result.unique(), so the dedup isreplicated explicitly: the related object is keyed on identity (matching the ORM
use_id_for_hashuniquing filter) and the remaining key columns by value. TheRow-based
unique()path is retained for therequires_uniquingcase (e.g. ajoinedload nested inside the subqueryload).
Benchmarks
minms, vsmain. Python 3.14, in-memory SQLite, 50 repeats.Risk: low–medium — the explicit dedup must match
unique()semantics; therequires_uniquing fallback covers nested-eager cases.
test_subquery_relationsand
test_recursive_loaderspass.Fixes: #13406
Checklist
This pull request is:
must include a complete example of the issue. one line code fixes without an
issue and demonstration will not be accepted.
Fixes: #<issue number>in the commit messageinclude a complete example of how the feature would look.
Fixes: #13406in the commit messageHave a nice day!
I raised this in a discussion. Still having trouble with gerrit.. sorry!