server/bug: conflicts on tables that are already tracked should not cause the server to fail

[sassela]

Relates https://github.com/hasura/graphql-engine/issues/8844. Suggesting a 1-day timebox

[ajohnson1200]

I thought that product platform team had an issue for this filed for something similar already, no?

[sassela]

not sure, so I've asked them directly.

[sassela]

confirmed in this thread the product platform team's not actively working on a fix, so we're good to pick it up. Consider requesting a review from them as there might be overlap with an upcoming RFC of theirs

[danieljharvey]

It seemed like there was an opportunity for a fix in the schema-parsers library here: https://github.com/hasura/graphql-engine-mono/blob/main/server/lib/schema-parsers/src/Hasura/GraphQL/Parser/Internal/Parser.hs#L189. However on closer inspection it seems more complicated than that.

Currently we collect together all the FieldParsers that are generated and group any duplicates by name and using them to show an errors. One solution could be to look at the sets of duplicates and decide if there's one we'd rather keep, removing the others. To do so though, we need to decide on a way to identify which is the "better" parser.

For example, if a user is tracking both authors and authors_by_pk tables, and then a new version of HGE adds the x_by_pk feature (pretend for a moment it's not already there).

Our first conflict will be between versions of the hasura_author_by_pk parser. One is from author, the other from author_by_pk. The parser that is named after the table should be more important, as we checked for conflicts before allowing HGE to track it. Therefore Hasura adding this new feature must be at fault, and we should prefer the parser that matches the table name.

(How could we do this? Currently we have MetadataObjId for looking at the source of FieldParsers, and although it does not disambiguate the ideas of "created from table" and "derived from table" it doesn't seem a stretch to add it.)

However, if we resolve the first conflict, then we have a second conflict between which version of update_hasura_author_by_pk wins - should it be the one from author or author_by_pk? This gets a little trickier to do automatically as they're both created by Hasura.

There are a few hacks like choosing the origin with the longer name, but that seems like it will come back to haunt us as another bug in the future.

[abooij]

One solution could be to look at the sets of duplicates and decide if there's one we'd rather keep, removing the others. To do so though, we need to decide on a way to identify which is the "better" parser.

I think this kind of approach is within the realm of what is already being explored by the Product Platform team as part of the work that's specified in #4998. So I'd like to request a change of direction here. The Native Databases team should work on finding a way to build a schema that avoids triggering this error in the first place, i.e. to avoid generating duplicate field names. I'm requesting this also because the more general solution for this kind of problem, that you're exploring in your comment, will take some time to materialize.

[abooij]

Put differently: we're approaching this from two angles:

1. To avoid conflicts in the first place by generating better field names. (Native Databases)
2. By figuring out a better way to deal with conflicts once they arise. (Product Platform)

[sassela]

discussing this in standup, here's the related Product Platform team RFC FYI https://github.com/hasura/graphql-engine-mono/blob/vamshi/rfc/graphql-schema-validation/rfcs/validating-graphql-schema.md

[danieljharvey]

Going to reduce the scope of this to begin with - it occurred because of the addition of the <tablename>_updates fields recently. We can solve the concrete problem with a feature flag for disabling this feature. The the workflow for unblocking users is:

Users run HGE with DISABLE_UPDATES_FIELD feature flag enabled (so it now starts up) Users renames the conflicting <tablename>_updates field to <tablename>_do_updates or something Users restart HGE with DISABLE_UPDATES_FIELD feature flag disabled and now it starts up with the updates feature enabled.

(feature flag name subject to change)

This also reflects how we should release features like this in future:

Release 1:

• new field behind a feature flag
• console / CLI warns users that they have a conflict and how to fix it

Release 2:

• feature flag removed
• users can rectify any conflicts before upgrading to this

[danieljharvey]

We'll also need to fix this in the console so users are able to do the renaming: https://github.com/hasura/graphql-engine-mono/issues/5887

[ejkkan]

We'll also need to fix this in the console so users are able to do the renaming: hasura/graphql-engine-mono#5887

https://github.com/hasura/graphql-engine-mono/issues/5887#issuecomment-1267981055