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

Fix(GraphQL): Add filter in DQL query in case of reverse predicate #7728

Merged
merged 4 commits into from
Apr 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions graphql/resolve/query_rewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@ func aggregateQuery(query schema.Query, authRw *authRewriter) []*gql.GraphQuery
// var(func: type(Tweets)) {
// scoreVar as Tweets.score
// }

mainQuery.Children = append(mainQuery.Children, child)
isAggregateVarAdded[constructedForField] = true
}
Expand Down Expand Up @@ -1148,6 +1149,12 @@ func buildAggregateFields(
fieldFilter, _ := f.ArgValue("filter").(map[string]interface{})
_ = addFilter(mainField, constructedForType, fieldFilter)

// Add type filter in case the Dgraph predicate for which the aggregate
// field belongs to is a reverse edge
if strings.HasPrefix(constructedForDgraphPredicate, "~") {
addTypeFilter(mainField, f.ConstructedFor())
}

// isAggregateVarAdded is a map from field name to boolean. It is used to
// ensure that a field is added to Var query at maximum once.
// Eg. Even if scoreMax and scoreMin are queried, the corresponding field will
Expand All @@ -1165,6 +1172,13 @@ func buildAggregateFields(
}
// Add filter to count aggregation field.
_ = addFilter(aggregateChild, constructedForType, fieldFilter)

// Add type filter in case the Dgraph predicate for which the aggregate
// field belongs to is a reverse edge
if strings.HasPrefix(constructedForDgraphPredicate, "~") {
addTypeFilter(aggregateChild, f.ConstructedFor())
}

aggregateChildren = append(aggregateChildren, aggregateChild)
continue
}
Expand Down Expand Up @@ -1363,6 +1377,12 @@ func addSelectionSetFrom(
if includeField := addFilter(child, f.Type(), filter); !includeField {
continue
}

// Add type filter in case the Dgraph predicate is a reverse edge
if strings.HasPrefix(f.DgraphPredicate(), "~") {
addTypeFilter(child, f.Type())
}

addOrder(child, f)
addPagination(child, f)
addCascadeDirective(child, f)
Expand Down
50 changes: 49 additions & 1 deletion graphql/resolve/query_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2527,7 +2527,7 @@
query {
queryMovie(func: type(Movie)) {
Movie.name : Movie.name
Movie.director : ~directed.movies {
Movie.director : ~directed.movies @filter(type(MovieDirector)) {
MovieDirector.name : MovieDirector.name
dgraph.uid : uid
}
Expand Down Expand Up @@ -3410,3 +3410,51 @@
dgraph.uid : uid
}
}

-
name: "Query fields linked to reverse predicates in Dgraph"
gqlquery: |
query {
queryLinkX(filter:{f9:{eq: "Alice"}}) {
f1(filter: {f6: {eq: "Eve"}}) {
f6
}
f2(filter: {f7: {eq: "Bob"}}) {
f7
}
f1Aggregate(filter: {f6: {eq: "Eve"}}) {
count
f6Max
}
f2Aggregate(filter: {f7: {eq: "Bob"}}) {
count
f7Min
}
}
}
dgquery: |-
query {
queryLinkX(func: type(LinkX)) @filter(eq(LinkX.f9, "Alice")) {
LinkX.f1 : ~link @filter((eq(LinkY.f6, "Eve") AND type(LinkY))) {
LinkY.f6 : LinkY.f6
dgraph.uid : uid
}
LinkX.f2 : ~link @filter((eq(LinkZ.f7, "Bob") AND type(LinkZ))) {
LinkZ.f7 : LinkZ.f7
dgraph.uid : uid
}
LinkX.f1Aggregate : ~link @filter((eq(LinkY.f6, "Eve") AND type(LinkY))) {
LinkX.f1Aggregate_f6Var as LinkY.f6
dgraph.uid : uid
}
LinkYAggregateResult.count_LinkX.f1Aggregate : count(~link) @filter((eq(LinkY.f6, "Eve") AND type(LinkY)))
LinkYAggregateResult.f6Max_LinkX.f1Aggregate : max(val(LinkX.f1Aggregate_f6Var))
LinkX.f2Aggregate : ~link @filter((eq(LinkZ.f7, "Bob") AND type(LinkZ))) {
LinkX.f2Aggregate_f7Var as LinkZ.f7
dgraph.uid : uid
}
LinkZAggregateResult.count_LinkX.f2Aggregate : count(~link) @filter((eq(LinkZ.f7, "Bob") AND type(LinkZ)))
LinkZAggregateResult.f7Min_LinkX.f2Aggregate : min(val(LinkX.f2Aggregate_f7Var))
dgraph.uid : uid
}
}
14 changes: 14 additions & 0 deletions graphql/resolve/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -451,3 +451,17 @@ type Friend1 {
type Friend {
id: String! @id
}

type LinkX {
f9: String! @id
f1: [LinkY] @dgraph(pred: "~link")
f2: [LinkZ] @dgraph(pred: "~link")
}
type LinkY {
f6: String! @id
f3: [LinkX] @dgraph(pred: "link")
}
type LinkZ {
f7: String! @id
f4: [LinkX] @dgraph(pred: "link")
}
13 changes: 13 additions & 0 deletions graphql/schema/gqlschema_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3431,3 +3431,16 @@ valid_schemas:
addressHi: String @dgraph(pred: "Person.address@hi")
professionEn: String @dgraph(pred: "Person.profession@en")
}

- name: "Same reverse dgraph predicate can be used by two different GraphQL fields"
input: |
type X {
f1: [Y] @dgraph(pred: "~link")
f2: [Z] @dgraph(pred: "~link")
}
type Y {
f3: [X] @dgraph(pred: "link")
}
type Z {
f4: [X] @dgraph(pred: "link")
}
5 changes: 5 additions & 0 deletions graphql/schema/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,11 @@ func dgraphDirectivePredicateValidation(gqlSch *ast.Schema, definitions []string
isSecret: false,
}

// Skip the checks related to same Dgraph predicates being used twice with
// different types in case it is an inverse edge.
if strings.HasPrefix(fname, "~") || strings.HasPrefix(fname, "<~") {
continue
}
if pred, ok := preds[fname]; ok {
if pred.isSecret {
errs = append(errs, secretError(pred, thisPred))
Expand Down