GraphQL naming conventions

[rodyvansambeek]

I am using DAB to connect to a MSSQL database, and in MSSQL it is common for Tables and Properties to be PascalCase.
However in the GraphQL naming conventions which is actually referenced from the source code I would think that the resulting GraphQL fields should also be camelCase.

In my MSSQL database I have a table called Properties, with columns: "Id" and "Description".

The graphql query to fetch these is:

query {
    properties {
        items {
            Id
            Description
        }
    }
}

Wheren Properties is nicely camelCased, but shouldn't Id and Description be camelCased as well?

[severussundar]

Hi @rodyvansambeek, thanks for spotting this.

At the moment, the fields generated does have the same styling/casing as the column names by default. If a column name is camel Cased, it does show up like that in the graphql field as well.
For example, when a table is constructed with the below schema,

CREATE TABLE stocks(
    categoryid int NOT NULL,
    pieceid int NOT NULL,
    categoryName varchar(100) NOT NULL,
    piecesAvailable int DEFAULT 0,
    piecesRequired int DEFAULT 0 NOT NULL,
    PRIMARY KEY(categoryid,pieceid)
);

the fields for the graphql query are as follows

{
  stocks {
    items{
      pieceid
      categoryName
      piecesAvailable
      piecesRequired
    }
  }
}

Notice that the fields categoryName, piecesAvailable and piecesRequired are camelCased.

However, by making use of the mapping option, it is possible to expose a column with a customized name for graphQL.

In the config file, under the Stock entity, adding the below mapping lets the pieceid column in the table be exposed as pieceId for graphQL.

"mappings": {
        "pieceid": "pieceId"
      }

GraphQL query:

{
  stocks {
    items{
      pieceId
      categoryName
      piecesAvailable
      piecesRequired
    }
  }
}

Please refer to Mappings documentation for more information on mappings

[rodyvansambeek]

Hi @severussundar thanks for reaching out. I understand the mappings features and can use that indeed.

However I think this should be the default behavior right to automatically map the column names to correct GraphQL fields? Or at least make it configurable to allow somebody to do that, because it's a breaking change of course due to GraphQL being case sensitive.

Do you agree? If so, I could write up a PR when I have some spare time to implement this.

[severussundar]

Hi @rodyvansambeek, here is the idea behind this behavior.

The graphQL query and mutation names are generated by Data API builder and the query/mutation names are created as per the referenced naming convention. The field names, however, are derived from the user's tables. So, the default behavior is to use the same name with which the columns are created. Additionally, the users are provided with the mappings feature to expose the columns with a different name for graphQL if they choose to.

[rodyvansambeek]

Hi @severussundar, thanks for the info. I understand that this is the idea behind te behavior, clear.

I encountered this situation because I have a EF Core code-first database which defaults to the exact same casing as the properties in the Model classes in C# (which are PascalCased). When using the GraphQL endpoint in a typescript frontend, this generates results for the fields (properties) which are also PascalCased, as in front-end code, the convention is to use lowercases (see TS Style guid).

The mappings features is useful, but a bit cumbersome to have to change all fields manually, for now, I ended up using the EFCore.NamingConventions to override the column names in the MSSQL database as a simple solution.

[aaronpowell]

Something doesn't seem right with the GraphQL that is being generated.

As you pointed out, we reference the GraphQL naming style guide in the source code, here to be exact: https://github.com/Azure/data-api-builder/blob/main/src/Service.GraphQLBuilder/GraphQLNaming.cs#L188-L193

Now that method, when executed like FormatNameForField("Id") will return id, which is the expected casing.

If you're seeing a generated GraphQL graph that doesn't match that, then I think we have a bug in how the names are generated.

[rodyvansambeek]

Hi @aaronpowell, thanks for your reply. I think this is a bug too.
I believe the code does not call the FormatNameForField method for the fields, but only for the objects and entities. Let me check it for sure with the latest source code and see if I can debug it.
Will come back to you.

[aaronpowell]

Actually, I stand corrected, it doesn't call that - https://github.com/Azure/data-api-builder/blob/main/src/Service.GraphQLBuilder/Sql/SchemaConverter.cs#L93-L108

And I was sure that it did, I wrote the SchemaConverter, so after some investigation I found that it use to but was changed with #609.

I'll tag @seantleonard who authored the PR to see if he remembers why the name formatting was removed from fields of the ObjectType's (it also suggests we might have a gap in our test coverage).

[Aniruddh25]

This is a known issue #692 which was closed until we receive an actual customer request. We can reopen it now :)

As @severussundar pointed out, this was done because we didn't want to modify customer provided Entity or column names.

So, the default behavior is to use the same name with which the columns are created.

@rodyvansambeek, I like this idea:

Or at least make it configurable to allow somebody to do that, because it's a breaking change of course due to GraphQL being case sensitive.

If you are interested, would you like to pick up #692? For the configuration option, I think we can add it under the graphql section of the respective entity. Thanks for bringing this up!

[rodyvansambeek]

If you are interested, would you like to pick up #692? For the configuration option, I think we can add it under the graphql section of the respective entity. Thanks for bringing this up!

@Aniruddh25 Yes ofcourse, let me find some time next week, I'll give it a shot!

[rodyvansambeek]

I have opened the Draft Pull Request #1528, let's continue our conversion there.