Graphql-engine: Field naming starting with reserved keywords breaks updates

[rstpv]

When in a table there is a column that starts with a reserved keyword, for example nullSomeColumnName or trueSomeColumnName, if you try to create an update with an on_conflict and columns, the update will fail like:

extensions": {
        "path": "$.selectionSet.insert_some_table.args.on_conflict.update_columns[indexOfColumnThatStartsWithNull]",
        "code": "validation-failed"
      },
      "message": "null value found for non-nullable type: some_table_update_column!"

If the field starts with true or false the error is almost the same but the message is related to value must be an enum.

It seems like the engine is trying to parse the column names somehow.

[sordina]

Looks very strange! I wonder why the column names might be being parsed? No idea where to start, but my first guess is some kind of derived instance is doing something funky. I'll see if I can figure out how the translation occurs then check out each component of the process.

[sordina]

Hi @rstpv could you provide some more detail for how to reproduce the error? I'm trying from the console with the following steps:

• Create table called "testTable" with id int PK, nullTestColumn int uniq
• Run insert into "testTable" ("nullTestColumn") values (1) on conflict do nothing twice

This seems to succeed. Am I missing something about how this should be expected to be triggered? Otherwise, if this does trigger the bug for you, what version of Hasura did you encounter the bug on?

Apologies if I've missed something obvious! I'm just getting to grips with the codebase :)

[rstpv]

Hi @sordina, you can reproduce this as:

1. Create a table with two columns such as:
   
2. Try to run a query with on_conflict and update_columns such as:

mutation {
  insert_testTable(objects: [{nullColumntwo: "2"}] on_conflict:{constraint:testTable_pkey, update_columns:[nullColumntwo]}){
    affected_rows
  }
}

1. It will throw an error such as:

{
  "errors": [
    {
      "extensions": {
        "path": "$.selectionSet.insert_testTable.args.on_conflict.update_columns[0]",
        "code": "validation-failed"
      },
      "message": "null value found for non-nullable type: testTable_update_column!"
    }
  ]
}

1. Rename the column to anything, for example:
   

5.Rerun the previous query with the renamed column:

mutation {
  insert_testTable(objects: [{changedColumntwo: "2"}] on_conflict:{constraint:testTable_pkey, update_columns:[changedColumntwo]}){
    affected_rows
  }
}

1. It succeeds:

{
  "data": {
    "insert_testTable": {
      "affected_rows": 1
    }
  }
}

The version we are using is the v1.0.0 stable.

[sordina]

Thanks @rstpv that makes total sense. I wasn't running a GraphQL mutation, so of course it wasn't triggering the issue. I'll try to reproduce with your steps.

[sordina]

I've replicated the issue with your instructions. I'll dig in deeper now.

[sordina]

Looks like the issue is in the parser. Initial test-case:

propNullWorks :: Property
propNullWorks = property $ either (fail . Protolude.show) (ast ===) astRoundTrip
  where
    -- Protolude.putStrLn (groom parsed) >> print text >> either (print) (print . PP.TB.renderExecutableDoc) parsed
    -- "mutation  { insert_testTable(on_conflict: {update_columns:[nullColumntwo]}) { affected_rows  } }"
    astRoundTrip = parseExecutableDoc printed
    printed      = PP.TB.renderExecutableDoc ast
    ast          = (ExecutableDocument{getExecutableDefinitions =
                        [ExecutableDefinitionOperation
                           (OperationDefinitionTyped
                              (TypedOperationDefinition{_todType = OperationTypeMutation,
                                                        _todName = Nothing,
                                                        _todVariableDefinitions = [],
                                                        _todDirectives = [],
                                                        _todSelectionSet =
                                                          [SelectionField
                                                             (Field{_fAlias = Nothing,
                                                                    _fName = Name{unName = "insert_testTable"},
                                                                    _fArguments =
                                                                      [Argument{_aName = Name{unName = "on_conflict"},
                                                                                _aValue =
                                                                                  VObject
                                                                                    (ObjectValueG{unObjectValue
                                                                                                    =
                                                                                                    [ObjectFieldG{_ofName = Name{unName = "update_columns"},
                                                                                                                  _ofValue
                                                                                                                    =
                                                                                                                    VList
                                                                                                                      (ListValueG{unListValue
                                                                                                                                    =
                                                                                                                                    [ VEnum
                                                                                                                                       (EnumValue{unEnumValue
                                                                                                                                                    =
                                                                                                                                                    Name{unName = "nullColumntwo"}})]})}]})}],
                                                                    _fDirectives = [],
                                                                    _fSelectionSet =
                                                                      [SelectionField
                                                                         (Field{_fAlias = Nothing,
                                                                                _fName =
                                                                                  Name{unName =
                                                                                         "affected_rows"},
                                                                                _fArguments = [],
                                                                                _fDirectives = [],
                                                                                _fSelectionSet =
                                                                                  []})]})]}))]})

[sordina]

Will attempt to reduce this and then fix the parser.

[sordina]

Issue created in parser repo: https://github.com/hasura/graphql-parser-hs/issues/20

[sordina]

@rstpv I've resolved the issue in the parser and am now testing integration with graphql-engine. Should have a candidate fix ready shortly.

[sordina]

Works manually. Adding automated tests now.

[sordina]

Pytest added:

pytest --hge-urls http://localhost:8080 --pg-urls=postgres://hasura-test\@localhost:5432/hasura-test2  --accept -vv test_graphql_mutations.py::TestGraphqlInsertNullPrefixedColumnOnConflict

test_graphql_mutations.py::TestGraphqlInsertNullPrefixedColumnOnConflict::test_address_not_null_constraint_err[websocket]
[gw0] [100%] PASSED test_graphql_mutations.py::TestGraphqlInsertNullPrefixedColumnOnConflict::test_address_not_null_constraint_err[websocket]

[sordina]

Created WIP fix: https://github.com/hasura/graphql-engine/pull/3927

Still need to finish approval of parser changes

[sordina]

Parser changes merged. Referencing commit in engine:

source-repository-package
  type: git
  location: https://github.com/hasura/graphql-parser-hs.git
  tag: 1380495a7b3269b70a7ab3081d745a5f54171a9c