feat(graphql): Introducing Nested Filtering for Dgraph GraphQL API with Optimized Query Support #9100

iyinoluwaayoola · 2024-06-09T15:09:43Z

Description:

This merge request introduces Nested Filtering to the Dgraph GraphQL API. It allows for comprehensive filtering capabilities that span across queries, aggregations, and authorization functions.

How Does It Work?

Nested Filtering leverages Dgraph's var block to filter nested objects and selects parent objects through inverse predicates. To enable nested filtering, you need to add the @search directive to the nested field (edge) without any arguments.

Example:

type User {
    name: String @search(by: [hash])
    friends: [User] @hasInverse(field: friends) @search
}

Given the schema above, you can query users who have friends named "Ben" using the following query:

query {
    queryUser(filter: { friends: { name: { eq: "Ben" } } }) {
        name
    }
}

Schema Validation:

If the @search directive is added to an edge, the @hasInverse directive must also be set on one of the referenced fields. This validation process includes handling interface fields by checking the implemented types for reverse fields.

Testing:

Testing has been conducted locally, showing approximately 50% fewer scanned nodes

Docs
RFC -> https://discuss.dgraph.io/t/rfc-nested-filters-in-graphql/13560/37

…t filtering ♻️ (query_rewriter.go): Refactor to store field definition in a variable for reuse ✨ (wrappers.go): Add HasSearchDirective method to FieldDefinition interface and its implementation to check for search directive in a field

CLAassistant · 2024-06-09T15:09:48Z

All committers have signed the CLA.

CLAassistant · 2024-06-09T15:09:49Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

harshil-goel · 2024-06-10T04:11:21Z

Can you add some test cases in e2e folder, and one in query_rewriter?

✨ (query_rewriter.go): Remove unnecessary assignment of aggFilterQueries 💡 (query_rewriter.go): Add comments to explain nested object filtering 🐛 (query_rewriter.go): Fix nested object filtering to handle nested fields correctly

…tainability ✨ (query_test.yaml, schema.graphql, gqlschema_test.yml): Add tests and schema for nested filtering 🐛 (gqlschema.go, rules.go): Fix validation for @search directive to require @hasInverse directive when necessary ♻️ (wrappers.go): Rename function 'hasInverseReference' to 'hasInverse' for clarity 💡 (wrappers.go): Remove unnecessary semicolon and improve code formatting ✨ (wrappers.go): Extend 'isCustomType' function to include 'ast.Interface' kind 🐛 (wrappers.go): Add additional check for inverse type when querying from an interface, not the implemented type

…for readability ✨ (query_test.yaml): Add test cases for deeply nested object queries and AND/OR conditions 🐛 (gqlschema.go, rules.go, wrappers.go): Fix custom type detection by passing type instead of field definition

…unction" to increase test coverage and ensure the aggregate function works as expected in nested queries.

…port nested filtering ✨ (auth_query_test.yaml): Add test case for nested filter query 🐛 (mutation_rewriter.go): Update rewriteAsQueryByIds and addFilter functions to include mutation alias for better query identification ♻️ (query_rewriter.go): Refactor filterQueries to varQry for better semantics ✨ (query_rewriter.go): Add queryName parameter to rewriteAsQuery and addArgumentsToField functions to support query aliasing 🐛 (query_rewriter.go): Fix issue where only the first query was returned in rewriteRuleNode, now returns all queries including nested var queries

…ules for more test coverage ✨ (auth_delete_test.yaml): Add new test case for delete selection by nested filter ✨ (auth_query_test.yaml): Update existing test case and add new ones for positive, negative, and uncertain auth rules with nested filter 🐛 (mutation_rewriter.go): Fix addFilter function call to include authRw parameter ♻️ (query_rewriter.go): Refactor addFilter and buildFilter functions to include authRewriter parameter for better access control 💡 (query_rewriter.go): Update function calls to addFilter and buildFilter to pass authRewriter parameter for improved security ♻️ (query_rewriter.go): Refactor variable name from 'queries' to 'varQry' for better readability ✨ (query_rewriter.go): Add 'auth' parameter to 'buildFilter' and 'buildUnionFilter' functions to support authorization rewriting in queries

iyinoluwaayoola · 2024-06-18T16:04:57Z

Hi @harshil-goel , added auth to the nested query filters along with a few test cases.

iyinoluwaayoola · 2024-06-18T16:21:19Z

I see an opportunity to generally improve the auth queries by storing them per type and reusing them each time. This way, if a type is selected multiple times within the same request, we can reuse the auth variable and reduce the number of scans.

iyinoluwaayoola · 2024-06-22T12:18:23Z

Hi @harshil-goel , can you please provide feedback or review in getting this merged? I see some CI failures, but unsure if they are related to the change.

…tter readability

✨ (query.go): Add timer to track query execution time for performance monitoring

harshil-goel

Sorry for the time in review.
The diff looks great, I have minor changes here and there based on the examples.
Please take a look.

harshil-goel · 2024-07-08T14:53:49Z

graphql/resolve/schema.graphql

@@ -522,3 +522,19 @@ type ProjectDotProduct {
  title: String
  description_v: [Float!] @embedding @search(by: ["hnsw(metric: dotproduct, exponent: 4)"]) 
 }
+
+"""


Is this required here? This makes it such that generated schema would contain this. From what I understand, you only need this in the tests?

Yes, only required for the test cases. Not sure if you require this to be removed, in which case the tests will fail.

harshil-goel · 2024-07-08T14:56:26Z