FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Reverse relations - `get_related_objects` and `prefetch_related` by regiscamimura · Pull Request #1423 · piccolo-orm/piccolo · GitHub

Reverse relations - get_related_objects and prefetch_related - #1423

Open
regiscamimura wants to merge 1 commit into
piccolo-orm:masterfrom
regiscamimura:reverse-relations
Open

Reverse relations - get_related_objects and prefetch_related#1423
regiscamimura wants to merge 1 commit into
piccolo-orm:masterfrom
regiscamimura:reverse-relations

Conversation

Copy link
Copy Markdown

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.

>>> Manager._meta.reverse_relations
{'band_manager_set': Band.manager, 'band_set': Band.manager}

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

manager = await Manager.objects().where(Manager.name == 'Guido').first()

>>> await manager.get_related_objects(Band.manager)
[<Band: 1>]

It returns a query, not rows, so it composes:

await manager.get_related_objects(Band.manager).where(Band.popularity > 500)

The accessor name works too, which helps when the other table isn't imported —
and it's also available as an attribute:

>>> await manager.get_related_objects('band_set')
>>> await manager.band_set

3. prefetch_related — one query per relation, not one per row

# 2 queries in total, however many managers there are:
managers = await Manager.objects().prefetch_related(Band.manager)

for manager in managers:
    print(await manager.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.

  1. 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.
  2. 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.
  3. 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.
  4. 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.
  5. 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.

$ ./scripts/test-postgres.sh tests/table/test_reverse_relations.py
19 passed

$ ./scripts/test-sqlite.sh tests/table/test_reverse_relations.py
19 passed

Full suite:

$ ./scripts/test-postgres.sh
1 failed, 902 passed, 33 skipped

$ ./scripts/test-sqlite.sh
736 passed, 200 skipped

The one Postgres failure is tests/table/test_update.py::TestOperators::test_operators,
which fails on master for me too.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reverse foreign key lookups

1 participant


Back | FazBrowse Home | New Git URL