-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix delete mutation returning incorrect data (fix #1794) (fix #1763) #1827
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
Conversation
TODO:- -> Add test case for delete mutation with relationships returning -> Update docs
Resolve Conflicts: server/src-lib/Hasura/GraphQL/Resolve/Insert.hs
|
Deploy preview for hasura-docs ready! Built with commit 384ef7b |
…on response (hasura#1776)" This reverts commit 4970fde.
|
Review app for commit 2b771ee deployed to Heroku: https://hge-ci-pull-1827.herokuapp.com |
|
Review app for commit 384ef7b deployed to Heroku: https://hge-ci-pull-1827.herokuapp.com |
|
Review app https://hge-ci-pull-1827.herokuapp.com is deleted |
TL;DR
---
We go from this:
```haskell
(|
withRecordInconsistency
( (|
modifyErrA
( do
(info, dependencies) <- liftEitherA -< buildRelInfo relDef
recordDependencies -< (metadataObject, schemaObject, dependencies)
returnA -< info
)
|) (addTableContext @b table . addRelationshipContext)
)
|) metadataObject
```
to this:
```haskell
withRecordInconsistencyM metadataObject $ do
modifyErr (addTableContext @b table . addRelationshipContext) $ do
(info, dependencies) <- liftEither $ buildRelInfo relDef
recordDependenciesM metadataObject schemaObject dependencies
return info
```
Background
---
We use Haskell's `Arrows` language extension to gain some syntactic sugar when working with `Arrow`s. `Arrow`s are a programming abstraction comparable to `Monad`s.
Unfortunately the syntactic sugar provided by this language extension is not very sweet.
This PR shows how we can sometimes avoid using `Arrow`s altogether, without loss of functionality or correctness. It is a demo of a technique that can be used to cut down the amount of `Arrows`-based code in our codebase by about half.
Approach
---
Although _in general_ not every `Monad` is an `Arrow`, specific `Arrow` instantiations are exactly as powerful as their `Monad` equivalents. Otherwise they wouldn't be very equivalent, would they?
Just like `liftEither` interprets the `Either e` monad into an arbitrary monad implementing `MonadError e`, we add `interpA` which interprets certain concrete monads such as `Writer w` into specific arrows, e.g. ones satisfying `ArrowWriter w`. This means that the part of the code that only uses such interpretable effects can be written _monadically_, and then used in _arrow_ constructions down the line.
This approach cannot be used for arrow effects which do not have a monadic equivalent. In our codebase, the only instance of this is `ArrowCache m`, implemented by the `Rule m` arrow. So code written with `ArrowCache m` in the context cannot be rewritten monadically using this technique.
See also
---
- #1827
- #2210
PR-URL: hasura/graphql-engine-mono#3543
Co-authored-by: jkachmar <8461423+jkachmar@users.noreply.github.com>
GitOrigin-RevId: eb79619c95f7a571bce99bc144ce42ee65d08505
<!-- The PR description should answer 2 important questions: --> ### What 1.86.0 is out now, so it's safe to go up now. [This version of Rust ](https://blog.rust-lang.org/2025/02/20/Rust-1.85.0.html)includes a new `2024` edition, which we do not use yet but will at some point as it enables a bunch of borrow checker improvements etc. ### How Upgrade `rust-toolchain.yaml` plus Dockerfiles, fix any new lints. V3_GIT_ORIGIN_REV_ID: 1c45b2cc9f7a4960ad1c0c7689352a0e855bf4b5
Description
Affected components
Related Issues
fix #1794, #1763
Solution and Design
From
alpha-40we've been using aWHEREclause to fetch required rows and generate mutation response. This has a few limitations like the requirement of a primary key/unique constraint. This also returns inconsistent data ondeletemutation as mentioned in #1794.Now, we're using
VALUES (..)(refer here) expression to form virtual table rows inSQLto generate mutation response.Internal changes:-
ConstraintNamefromTableConstraintinTableInfotypetcColsfield inTableConstrainttypetable_info.sqlandfetchTableMetafunctionSQLdeletemutation and returning relational objects.Steps to test and verify
Perform
deletemutation and query relational field inreturning.Response:-
{ "data": { "delete_article": { "affected_rows": 2, "returning": [ { "id": 4, "title": "Article 4", "content": "Sample article content 4", "author": { "id": 2, "name": "Author 2", "articles_aggregate": { "aggregate": { "count": 0 } }, "articles": [] } }, { "id": 5, "title": "Article 5", "content": "Sample article content 5", "author": { "id": 2, "name": "Author 2", "articles_aggregate": { "aggregate": { "count": 0 } }, "articles": [] } } ] } } }