这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@rakeshkky
Copy link
Member

Description

Affected components

  • Server
  • Tests

Related Issues

fix #1794, #1763

Solution and Design

From alpha-40 we've been using a WHERE clause 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 on delete mutation as mentioned in #1794.
Now, we're using VALUES (..) (refer here) expression to form virtual table rows in SQL to generate mutation response.

Internal changes:-

  • Not to use primary key/unique constraint columns:-
    • Revert back to ConstraintName from TableConstraint in TableInfo type
    • Remove tcCols field in TableConstraint type
    • Modify table_info.sql and fetchTableMeta function SQL
  • A test case to perform delete mutation and returning relational objects.

Steps to test and verify

Perform delete mutation and query relational field in returning.

mutation DeleteAuthor2Articles {
  delete_article(where: {author: {id: {_eq: 2}}}) {
    affected_rows
    returning {
      id
      title
      content
      author {
        id
        name
        articles_aggregate{
          aggregate{count}
        }
        articles{
          id
          title
        }
      }
    }
  }
}

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": []
          }
        }
      ]
    }
  }
}

TODO:-
-> Add test case for delete mutation with relationships returning
-> Update docs
Resolve Conflicts:
	server/src-lib/Hasura/GraphQL/Resolve/Insert.hs
@rakeshkky rakeshkky added the c/server Related to server label Mar 20, 2019
@rakeshkky rakeshkky self-assigned this Mar 20, 2019
@rakeshkky rakeshkky requested a review from 0x777 as a code owner March 20, 2019 09:30
@netlify
Copy link

netlify bot commented Mar 20, 2019

Deploy preview for hasura-docs ready!

Built with commit 384ef7b

https://deploy-preview-1827--hasura-docs.netlify.com

@rakeshkky rakeshkky requested a review from rikinsk-zz as a code owner March 20, 2019 09:44
@hasura-bot
Copy link
Contributor

Review app for commit 2b771ee deployed to Heroku: https://hge-ci-pull-1827.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1827-2b771ee

@hasura-bot
Copy link
Contributor

Review app for commit 384ef7b deployed to Heroku: https://hge-ci-pull-1827.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1827-384ef7b

@shahidhk shahidhk changed the title fix delete mutation returning incorrect data fix delete mutation returning incorrect data (fix #1794) (fix #1763) Mar 22, 2019
@shahidhk shahidhk merged commit 5bafdce into hasura:master Mar 22, 2019
@hasura-bot
Copy link
Contributor

Review app https://hge-ci-pull-1827.herokuapp.com is deleted

hasura-bot pushed a commit that referenced this pull request Feb 22, 2022
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
hasura-bot pushed a commit that referenced this pull request Apr 8, 2025
<!-- 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c/server Related to server

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Relation in Mutations do not return correct data since alpha40

4 participants