graphql-engine
graphql-engine copied to clipboard
Previous mutations are not rolled back when one fails
Version Information
Server Version: 2.8.3
What is the current behaviour?
When executing a mutation composed of several operations, and when one of them failed, previous ones are not rolled back.
What is the expected behaviour?
https://hasura.io/docs/latest/graphql/core/databases/postgres/mutations/multiple-mutations/#execution
If multiple mutations are part of the same request, they are executed sequentially in a single transaction. If any of the mutations fail, all the executed mutations will be rolled back.
How to reproduce the issue?
Given these three postgres tables:
CREATE TABLE "articles"
(
"identifier" UUID NOT NULL DEFAULT Uuid_generate_v4(),
"title" TEXT NOT NULL,
CONSTRAINT "PK_eccda5e33eeb028ae7adc9ce71e" PRIMARY KEY ("identifier")
)
CREATE TABLE "images"
(
"identifier" UUID NOT NULL DEFAULT Uuid_generate_v4(),
"url" TEXT NOT NULL,
CONSTRAINT "PK_5dd26d55bd7a786386b840469cc" PRIMARY KEY ("identifier")
)
CREATE TABLE "articles_images"
(
"identifier" UUID NOT NULL DEFAULT Uuid_generate_v4(),
"article_identifier" UUID NOT NULL,
"image_identifier" UUID NOT NULL,
CONSTRAINT "PK_7f24e82a97c6c961bef5d97213a" PRIMARY KEY ("identifier")
)
and this graphQL mutation:
mutation updateOneArticleMutation(
$identifier: uuid!,
$fields: articles_set_input!,
$imagesToAdd: [articles_images_insert_input!]!,
$imagesToDelete: [uuid!]!
) {
insert_articles_images(objects: $imagesToAdd) {
affected_rows
}
delete_articles_images(
where: {
article_identifier: {_eq: $identifier},
image_identifier: {_in: $imagesToDelete}
}
) {
affected_rows
}
update_articles_by_pk(
pk_columns: {identifier: $identifier},
_set: $fields
) {
identifier
title
images {
image {
identifier
url
}
}
}
}
When the third operation fails, the two ones before are not rolled back.
What i've done to determine that:
- I've replaced the
identifier
value inupdate_articles_by_pk
>pk_columns
by an invalid UUID string
{
"errors": [
{
"extensions": {
"path": "$",
"code": "data-exception"
},
"message": "invalid input syntax for type uuid: \"NOT_AN_UUID\""
}
]
}
- I've set a SQL constraint that causes the update query to crash at the database level, not graphql engine:
{
"errors": [
{
"extensions": {
"path": "$",
"code": "constraint-violation"
},
"message": "Uniqueness violation. duplicate key value violates unique constraint \"articles_title_key\""
}
]
}
These two cases leads to the same result: insert_articles_images
and delete_articles_images
mutations were commited to the database.
Thanks
I'm facing the same issue
We're also experiencing this on v2.37.0-cloud.2. It's a very serious issue that leaves garbage behind in the database and really breaks the point of using a relational database backend. @0x777 what do you think, since it's a serious data integrity issue vs documented behavior, could this one be prioritized?
On further investigation I've found what is causing the mutation operation to not run in a postgresql transaction, at least in our case. We're using a graphQL client that is code-generated by a framework, and it's adding __typename to each operation in the mutation, as well as one at the end. For example, the mutation looks something like this:
mutation changesomething($a: uuid!, $b: uuid!, $c: String!, $d: String!) {
insert_tablea_one(object: {id: $b, c: $c}, on_conflict: {constraint: tablea_pkey, update_columns: [c]}) {
created_at
updated_at
__typename
}
insert_tablea_tableb_one(object: {a: $a, b: $b, d: $d}, on_conflict: {constraint: tablea_tableb_pkey, update_columns: [d]}) {
created_at
updated_at
__typename
}
__typename
}
It's the presence of that last __typename
that is causing Hasura not to execute the whole thing in a transaction. If you take out the outermast __typename
but leave in __typename
inside of the insert_tablea_one
and insert_tablea_tableb_one
it still executes within a transaction. Hasura is treating the last __typename
as not from the same postgresql source, and thus doesn't execute it in a transaction. In the code that determines if the operation is eligible to run in a transaction, it should ignore meta operations like __typename