Disallow DQL schema changes for predicates used in GraphQL schema

Issues Dgraph
, ,
#1

I’m using GraphQL and DQL at the same time. Since the GraphQL schema maps to a DQL schema (docs), I can seamlessly run queries between GraphQL and DQL.

But, I can make changes to my DQL schema directly that would break my GraphQL schema. If a predicate is being used in GraphQL it shouldn’t be modifiable via alter. It should only be changed in the GraphQL schema.

What version of Dgraph are you using?

v20.11.3 / latest

Have you tried reproducing the issue with the latest release?

Yes

Steps to reproduce the issue (command/config used to run Dgraph).

  1. Set a GraphQL schema to Dgraph.

    type Product {
  name: String! @search(by: [hash])
}

    This creates a Dgraph schema entry for Product.name: string @index(hash) .

  2. Add data and queries in GraphQL, which work as you expect:

    mutation Product {
  addProduct(input: [{name: "Product A"}]) {
    product {
      name
    }
  }
}

query GetPerson {
  queryProduct(filter: {name: {eq: "Product A"}}) {
    name
  }
}

    {
  "data": {
    "queryProduct": [
      {
        "name": "Product A"
      }
    ]
  }
  ...
}

  3. Update the DQL schema to change the indexing:

    Product.name: string @index(trigram) .

    This makes the DQL schema inconsistent with the GraphQL schema.

  4. Running the same query in #1 returns an error message:

    {
  "errors": [
    {
      "message": "Dgraph query failed because Dgraph execution failed because : Attribute Product.name does not have a valid tokenizer."
    }
  ],
  "data": {
    "queryProduct": []
  }
}

Expected behaviour and actual result.

I shouldn’t be able to change the DQL schema to cause these GraphQL errors.

2 Likes
Filtering by multiple child types slow
#3

What is the suggested workflow then? To limit updates to DQL schema via graphql?

1 Like
#4

The suggested behavior is to disallow DQL schema updates for predicates that are used in the GraphQL schema. If a predicate is defined by the GraphQL schema, it should be changed in the GraphQL schema, not the DQL schema.

Also, this should also work in the case where GraphQL schema predicates are defined by @dgraph. I dunno if that’d be treated any differently.

#5

Predicates annotated with @dgraph should be editable?

#6

@dgraph doesn’t create predicates, hence we don’t need to handle this condition

#7

The question is whether we should allow changing of Dgraph predicates through alter endpoint which have been referenced from GraphQL schema using @dgraph directive.

These changes will also have to be disallowed using the alter endpoint to prevent any inconsistencies with GraphQL schema.

#8

  1. This change would mean, one can no more add extra indexes in addition to the ones defined in the GraphQL schema, without updating the GraphQL schema. For example, if someone only wanted to expose hash index in the GraphQL api, but also wanted DQL to use the trigram index, i.e.:

    Product.name: string @index(hash,trigram) .

    That would no more be possible.

    I am not sure if that was ever a use-case. Could have been a scenario for people who use GraphQL and DQL together.

  2. GraphQL doesn’t have a way to add @count index at present. So, if someone was using custom DQL to make use of count index in their schema for some GraphQL predicate, that won’t work anymore. That would limit the usability of custom DQL.

    Again, not sure if someone was using such a trick.

  3. @dgraph is used for an already existing DQL schema. So I believe in such cases, the DQL schema is more correct than the GraphQL schema. And if the user would have given something wrong in the GraphQL schema for such predicates (like an incompatible type change), then updating the DQL schema generated by the GraphQL layer would have returned an error.

    Also, adding @reverse to DQL schema would be problematic, if we restrict the predicates in @dgraph.

    So for @dgraph, I think we should defer to the DQL schema for correctness. For the rest of the cases, GraphQL schema should be considered for correctness.

I wonder if we should provide this as a configurable change? Also, configurable per Dgraph alpha instance, or per GraphQL user, i.e., as a command-line option to Alpha or as config in the user-provided GraphQL schema?

#9

@dgraph(…) links a predicate to GraphQL. Not sure why it should be treated any differently. In fact, any predicate linked from GraphQL could be assumed to have a @dgraph Directive with their Type.Name in it.

@thiyaga — A predicate linked via @dgraph is being used by GraphQL and must go through GraphQL schema updates for modifications.

#10

IMO, I see community members modifying the DQL schema that gets generated by GraphQL to add:

  • reverse
  • count (not so much now that we have aggregate queries)
  • facets
1 Like
#11

Predicates created by GraphQL interfaces also needs to be handled here

#12

This change is available in master now: feat(GraphQL): Disallow DQL schema changes for predicates used in GraphQL schema (DGRAPH-3245) by abhimanyusinghgaur · Pull Request #7742 · dgraph-io/dgraph · GitHub

From now on:

  • any direct change to DQL schema that can break GraphQL schema will result in an error.
  • changes to DQL schema that don’t affect GraphQL schema, will succeed => one can still add more indexes, or things like @reverse, @count, if they need them in DQL.
  • for the purpose of validation, fields with @dgraph will be treated similar to fields without @dgraph.