Piccolo already records inbound foreign keys on
Table._meta.foreign_key_references, but there's no way to walk them, so
"give me every band this manager manages" means writing the query by hand every
time. This adds three things on top of what's already there, and needs no
change to anyone's schema.
1. Table._meta.reverse_relations
Accessor name → the ForeignKey which points at this table.
Each relation gets a name which includes the column (band_manager_set), and a
shorter one which doesn't (band_set). The shorter name is only added when it's
unambiguous — a table with two foreign keys pointing here (Concert.band_1 /
Concert.band_2) only gets the long names, so nothing silently resolves to the
wrong column.
2. get_related_objects — the reverse of get_related
3. prefetch_related — one query per relation, not one per row
# 2 queries in total, however many managers there are:managers=awaitManager.objects().prefetch_related(Band.manager)
formanagerinmanagers:
print(awaitmanager.band_set)
Measured on 25 managers with a band each, counting calls to
run_querystring:
without prefetch_related: 26 queries
with prefetch_related: 2 queries
Each row keeps its own related rows. Accessing the relation afterwards is
answered from memory; narrowing it in any way goes back to the database,
because the prefetched rows can't answer a different question. That's Django's
rule for .all() vs .filter() on a prefetched related manager.
There's also a standalone prefetch_related(rows, *foreign_keys) for when you
already have the rows.
Design decisions I'd like your call on
I've picked a default for each of these rather than block on them, and none is
hard to change.
Accessor naming. I used Django's <tablename>_set, plus a
column-qualified <tablename>_<column>_set. The alternative is to offer only
the qualified form (never ambiguous, uglier), or nothing automatic at all.
related_name / reverse_lookup. In Reverse foreign key lookups #378 you suggested
ForeignKey(Manager, reverse_lookup='bands'), mainly for tab completion. I've
deliberately left that out — it's a new ForeignKey param, which means
migration serialisation, and it can be added later on top of this without
breaking the derived names. Say the word if you'd rather have it in from the
start.
The __getattr__ accessor. manager.band_set needs Table.__getattr__,
which Piccolo doesn't currently have. It bails out immediately on any name
starting with _, so dunder lookups (pickle, copy) are unaffected, and it
raises AttributeError listing the available relations otherwise. If you'd
rather not have any attribute magic on Table, dropping it leaves
get_related_objects working exactly as it does now.
Prefetch cache semantics. The cache lives on the Objects query
(_prefetched_results) and is discarded by every method which changes the
query. That means one extra line in where, order_by, limit, offset,
prefetch, output, callback, as_of, lock_rows and get — the least
invasive alternative I found. Happy to move it into a subclass of Objects
returned only by get_related_objects if you'd prefer Objects untouched.
Scope. prefetch_related could be a follow-up PR. I've kept it here
because without it the accessor is an N+1 waiting to happen, but I'm happy to
split it.
Relationship to the earlier attempt
#599 approached this from the select side (Manager.select(Manager.artists()),
modelled on M2M) and needed a ReverseLookup declared on the table. This is
the objects-side equivalent instead, derived from the foreign keys which are
already registered — so existing tables get it without any schema change. The
two aren't mutually exclusive.
Testing
New file: tests/table/test_reverse_relations.py (19 tests). It defines its own
tables rather than using the music example app, because
foreign_key_references accumulates globally and several test modules define
throwaway tables with the same tablenames.
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
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.
Closes #378 (if the API looks right to you).
Piccolo already records inbound foreign keys on
Table._meta.foreign_key_references, but there's no way to walk them, so
"give me every band this manager manages" means writing the query by hand every
time. This adds three things on top of what's already there, and needs no
change to anyone's schema.
1. Table._meta.reverse_relations
Accessor name → the ForeignKey which points at this table.
Each relation gets a name which includes the column (band_manager_set), and a
shorter one which doesn't (band_set). The shorter name is only added when it's
unambiguous — a table with two foreign keys pointing here (Concert.band_1 /
Concert.band_2) only gets the long names, so nothing silently resolves to the
wrong column.
2. get_related_objects — the reverse of get_related
It returns a query, not rows, so it composes:
The accessor name works too, which helps when the other table isn't imported —
and it's also available as an attribute:
3. prefetch_related — one query per relation, not one per row
Measured on 25 managers with a band each, counting calls to
run_querystring:
Each row keeps its own related rows. Accessing the relation afterwards is
answered from memory; narrowing it in any way goes back to the database,
because the prefetched rows can't answer a different question. That's Django's
rule for .all() vs .filter() on a prefetched related manager.
There's also a standalone prefetch_related(rows, *foreign_keys) for when you
already have the rows.
Design decisions I'd like your call on
I've picked a default for each of these rather than block on them, and none is
hard to change.
column-qualified <tablename>_<column>_set. The alternative is to offer only
the qualified form (never ambiguous, uglier), or nothing automatic at all.
ForeignKey(Manager, reverse_lookup='bands'), mainly for tab completion. I've
deliberately left that out — it's a new ForeignKey param, which means
migration serialisation, and it can be added later on top of this without
breaking the derived names. Say the word if you'd rather have it in from the
start.
which Piccolo doesn't currently have. It bails out immediately on any name
starting with _, so dunder lookups (pickle, copy) are unaffected, and it
raises AttributeError listing the available relations otherwise. If you'd
rather not have any attribute magic on Table, dropping it leaves
get_related_objects working exactly as it does now.
(_prefetched_results) and is discarded by every method which changes the
query. That means one extra line in where, order_by, limit, offset,
prefetch, output, callback, as_of, lock_rows and get — the least
invasive alternative I found. Happy to move it into a subclass of Objects
returned only by get_related_objects if you'd prefer Objects untouched.
because without it the accessor is an N+1 waiting to happen, but I'm happy to
split it.
Relationship to the earlier attempt
#599 approached this from the select side (Manager.select(Manager.artists()),
modelled on M2M) and needed a ReverseLookup declared on the table. This is
the objects-side equivalent instead, derived from the foreign keys which are
already registered — so existing tables get it without any schema change. The
two aren't mutually exclusive.
Testing
New file: tests/table/test_reverse_relations.py (19 tests). It defines its own
tables rather than using the music example app, because
foreign_key_references accumulates globally and several test modules define
throwaway tables with the same tablenames.
Full suite:
The one Postgres failure is tests/table/test_update.py::TestOperators::test_operators,
which fails on master for me too.