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

perf(GraphQL): Part-2: Remove unnecessary dgraph.uid : uid selections for auth queries (GRAPHQL-854) #7105

Merged
merged 1 commit into from Dec 10, 2020

Conversation

abhimanyusinghgaur
Copy link
Contributor

@abhimanyusinghgaur abhimanyusinghgaur commented Dec 10, 2020

We don't need to query uid for auth queries, as they always have at least one field in their selection set. This optimizes the number of touched_uids greatly, while time is not much improved here.


This change is Reviewable

@github-actions github-actions bot added the area/graphql Issues related to GraphQL support on Dgraph. label Dec 10, 2020
Copy link
Contributor

@pawanrawal pawanrawal left a comment

Choose a reason for hiding this comment

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

Reviewed 6 of 6 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @abhimanyusinghgaur and @MichaelJCompton)


graphql/resolve/query_rewriter.go, line 562 at r1 (raw file):

	addArgumentsToField(dgQuery[0], field)
	selectionAuth := addSelectionSetFrom(dgQuery[0], field, authRw)
	// we don't need to query uid for auth queries, as they always have at least one field in their

What happens if the auth rule itself queries for the id, is it added in that case?

Copy link
Contributor Author

@abhimanyusinghgaur abhimanyusinghgaur left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @MichaelJCompton and @pawanrawal)


graphql/resolve/query_rewriter.go, line 562 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

What happens if the auth rule itself queries for the id, is it added in that case?

Yes, that would be added as id: uid.
These dgraph.uid : uid queries were being added in case there was no uid query already existing in that subgraph. This is useful for non-null checks in GraphQL, but not useful for auth queries.

@abhimanyusinghgaur abhimanyusinghgaur merged commit 9baa3c7 into master Dec 10, 2020
@abhimanyusinghgaur abhimanyusinghgaur deleted the abhimanyu/auth-perf-2 branch December 10, 2020 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/graphql Issues related to GraphQL support on Dgraph.
Development

Successfully merging this pull request may close these issues.

None yet

2 participants