-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Field naming starting with reserved keywords breaks updates #3597
Comments
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. |
Hi @rstpv could you provide some more detail for how to reproduce the error? I'm trying from the console with the following steps:
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 :) |
Hi @sordina, you can reproduce this as:
5.Rerun the previous query with the renamed column:
The version we are using is the v1.0.0 stable. |
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. |
I've replicated the issue with your instructions. I'll dig in deeper now. |
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 =
[]})]})]}))]})
|
Will attempt to reduce this and then fix the parser. |
Issue created in parser repo: hasura/graphql-parser-hs#20 |
@rstpv I've resolved the issue in the parser and am now testing integration with graphql-engine. Should have a candidate fix ready shortly. |
Pytest added:
|
Created WIP fix: #3927 Still need to finish approval of parser changes |
Parser changes merged. Referencing commit in engine:
|
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:
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.
The text was updated successfully, but these errors were encountered: