Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

entc/gen: use join for loading m2m relationship #2417

Merged
merged 2 commits into from Mar 21, 2022
Merged

entc/gen: use join for loading m2m relationship #2417

merged 2 commits into from Mar 21, 2022

Conversation

a8m
Copy link
Member

@a8m a8m commented Mar 20, 2022

Rewrite the way Ent eager-loads M2M edges, by replacing two queries with a single JOIN query. The main reasons are:

  1. This change is necessary for the nested-pagination implementation in entgql in order to use SQL WINDOW functions and limit the number of edges returned for each entity.
  2. In the average case, Ent users limit the number of records they query from the database using entgql/entproto pagination or use limit+offset, and the previous implementation used to scan all edges even though it was not necessary (e.g. With<E> configured with Limit, Order or Where).
  3. If we want to keep the previous implementation and avoid scanning all edges, we should add JOIN to the destination table which makes it irrelevant.
  4. In case there are many edges, the previous implementation is reached the database parameters limit (e.g. 65535 in PostgreSQL).

Test plan

Added 10K users, and connected each one of them to a unique group with attribute active=false. Then, created another 10 groups (with active=true) and connected them to all users.

The results are as follows:

TestMySQL/8 (branch:withm2m)
users->groups(active:false)	104.482914ms
users->groups			394.802676ms
groups(active:false)->users	105.381099ms
groups->users			104.649884ms

TestMySQL/8 (branch:master)
users->groups(active:false)	176.937021ms
users->groups			180.452625ms
groups(active:false)->users	111.331736ms
groups->users			165.322574ms

When querying users->groups, the new JOIN of users.id + groups columns generate 99990 duplicate rows, unlike the previous solution, that used 2 queries. One for getting all edges (without filtering) from join-table, and another one for querying the destination table (the one requested on With).
However, in cases like groups(active:false) when there are no duplicate rows at all, the numbers are much better.

Future work

If we'll feel it is necessary, we can provide users an option to configure the way relationships are queried using "runtime annotation/hints". Some rough ideas I have in mind:

client.User.Query().
    WithGroups(sql.LoadWithJoin()).
    WithFriends(sql.LoadWithSubQuery()).
    All(ctx)

@a8m a8m force-pushed the withm2m branch 3 times, most recently from 78700a6 to 5d93290 Compare March 20, 2022 11:12
@a8m a8m marked this pull request as ready for review March 20, 2022 13:20
@a8m a8m changed the title entc/gen: use join for m2m relationship entc/gen: use join for loading m2m relationship Mar 20, 2022
@a8m a8m requested a review from masseelch March 21, 2022 08:23
Copy link
Collaborator

@masseelch masseelch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests did not pick up the bug, did they? Should we add a test for it?

Copy link
Collaborator

@masseelch masseelch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@a8m a8m merged commit edd9684 into master Mar 21, 2022
@a8m a8m deleted the withm2m branch March 21, 2022 09:37
gitlawr pushed a commit to gitlawr/ent that referenced this pull request Apr 13, 2022
* entc/gen: use join for m2m relationship

* entc/gen: add test for eager-load inverse-m2m
gitlawr pushed a commit to seal-io/ent that referenced this pull request Apr 13, 2022
* entc/gen: use join for m2m relationship

* entc/gen: add test for eager-load inverse-m2m
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.

None yet

2 participants