Skip to content
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

update_*_by_pk mutation with empty body should still return the target row #6061

Closed
carlpaten opened this issue Oct 22, 2020 · 4 comments
Closed

Comments

@carlpaten
Copy link
Contributor

Consider the following GraphQL mutation:

  mutation ExampleMutation {
  update_user_by_pk(pk_columns: {id: 10}, _set: {}) {
    username
  }
}

Today, this returns update_user_by_pk: {} instead of update_user_by_pk: { username: "lilred" }.

This is problematic for my use case. I have a choice of several plug-ins that I can dispatch some request to, and they return a field to put in _set for update_user_by_pk. One of my plug-ins returns the empty object, but this breaks calling code because now when I submit the subsequent mutation I'm not getting the username back.

@tirumaraiselvan
Copy link
Contributor

Not having a _set value means there is no update to be performed and hence there is no way to return target rows from PG.

@carlpaten
Copy link
Contributor Author

I haven't looked into the Hasura internals, so I apologize if I'm making a fool out of myself -

  • How are update_*_by_pk calls translated into SQL? If you could link me to the source I would appreciate it a ton.
  • Hasura could detect _set: {} in update_*_by_pk and execute a SELECT instead of an UPDATE. Is there a reason this couldn't work in principle? I'm not trying to imply that this is the correct thing to do, but it does seem like one possible way of accomplishing it.
  • Do you agree that this is counter-intuitive behaviour? If so, do you think it would be a good idea to "fix" it? I wouldn't mind taking a crack at it if someone can tell me where to look.

@tirumaraiselvan
Copy link
Contributor

How are update_*_by_pk calls translated into SQL?

It's basically using the RETURNING clause to return the updated rows: https://www.postgresql.org/docs/12/sql-update.html

Hasura could detect set: {} in update*_by_pk and execute a SELECT instead of an UPDATE.

I guess it could do it but it would be against the semantics of update which only returns updated rows.

Do you agree that this is counter-intuitive behaviour?

I do not agree that this is counter-intuitive. Since there is nothing to update, hence nothing to return. In fact, it is worth looking into throwing a validation error for an empty _set : {} object for update mutations.

If you need a workaround, then you can set the primary key again:

update_user_by_pk(pk_columns: {id: 10}, _set: {id: 10})

@carlpaten
Copy link
Contributor Author

I guess it could do it but it would be against the semantics of update which only returns updated rows.

I didn't expect "update" to include no-ops, but if Postgres is saying that a no-op counts, then I guess a no-op counts! Thank you for your time. I'll use that work-around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants