I was trying to figure out how to pipe some information from query
execution to the http log recently, and once again stumbled over the
mess that is `runGQ`. Here's an attempt to break it apart a little bit.
The result should by no means be considered final, but I hope it makes it
somewhate easier to understand what's going on in this function. E.g. now it's
once again somewhat visible how execution of queries and mutations differs.
Had to stop somewhere...
The PR is intended to have no functional change. It consists of individual
commits which should be "obviously" such.
Some thoughts and possible follow-up:
- It'd be good to get rid of the ad hoc `Result` data type again eventually,
but for the moment I think it's better than the tuples that used to be.
- I think we're quite close to reducing the duplication with WebSocket. E.g.
executeQueryStep and executeMutationStep might be reusable.
- It's tempting to change the caching API slightly, so that the uncached
response headers don't have to be pulled from the cache lookup result.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/2669
GitOrigin-RevId: ea414d24194509ce29469d74c62fd060b750488d
<!-- Thank you for ss in the Title above ^ -->
## Description
<!-- Please fill thier. -->
<!-- Describe the changes from a user's perspective -->
We don't have dependency reporting mechanism for `mssql_run_sql` API i.e when a database object (table, column etc.) is dropped through the API we should raise an exception if any dependencies (relationships, permissions etc.) with the database object exists in the metadata.
This PR addresses the above mentioned problem by
-> Integrating transaction to the API to rollback the SQL query execution if dependencies exists and exception is thrown
-> Accepting `cascade` optional field in the API payload to drop the dependencies, if any
-> Accepting `check_metadata_consistency` optional field to bypass (if value set to `false`) the dependency check
### Related Issues
<!-- Please make surt title -->
<!-- Add the issue number below (e.g. #234) -->
Close#1853
### Solution and Design
<!-- How is this iss -->
<!-- It's better if we elaborate -->
The design/solution follows the `run_sql` API implementation for Postgres backend.
### Steps to test and verify
<!-- If this is a fehis is a bug-fix, how do we verify the fix? -->
- Create author - article tables and track them
- Defined object and array relationships
- Try to drop the article table without cascade or cascade set to `false`
- The server should raise the relationship dependency exists exception
## Changelog
- ✅ `CHANGELOG.md` is updated with user-facing content relevant to this PR.
If no changelog is required, then add the `no-changelog-required` label.
## Affected components
<!-- Remove non-affected components from the list -->
- ✅ Server
- ❎ Console
- ❎ CLI
- ❎ Docs
- ❎ Community Content
- ❎ Build System
- ✅ Tests
- ❎ Other (list it)
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/2636
GitOrigin-RevId: 0ab152295394056c4ca6f02923142a1658ad25dc
While it looks like a lot of work in FromIr.hs, you can rather review the type changes in `Hasura.Backends.MySQL.Types.Internal` and the changes to FromIr are only to reflect that. Essentially we're simplifying the FromIr code to not think about SQL-based joins: instead, FromIr produces fields necessary for the dataloader Plan/Execute to do their job properly.
I've done my best to ensure that all the hunks in the diff in this PR are minimal for slightly easier perusing.
I think future PRs will be more intentionally well structured, rather than created retroactively.
**Preceding PR:** #2549
**Next PR**: #2367
The tests have been run like this on my machine. I don't know more beyond that.
```
docker run -i -e "PYTEST_ADDOPTS=--color=yes" -e "TERM=xterm-256color" --net=host -v`pwd`:`pwd` -w`pwd`/server/tests-py chrisdone/hasura-pytest:b0f26f615 pytest --hge-urls="http://localhost:8080" --pg-urls="postgres://chinook:chinook@localhost:5432/chinook" --backend mysql -k MySQL
```
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/2608
Co-authored-by: Abby Sassel <3883855+sassela@users.noreply.github.com>
GitOrigin-RevId: a6483335c3036963360dde7d7d7eaf10859351cb
This is the next part in the series of MySQL PRs.
**Purpose**: Adds the modules related to the generic backend abstraction. This is the penultimate PR.
The final PR will be the more substantial change made to FromIr (and small tweaks to Execute) that will connect all the things together including Python tests.
**Preceding PR:** #2529
**Next PR**: #2608
After #2529 is merged, this can be repointed to `main`. For now it's aimed at #2529, because then the diff display is simpler.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/2549
Co-authored-by: Abby Sassel <3883855+sassela@users.noreply.github.com>
GitOrigin-RevId: 9d48ac38f4ac7c32c79ba521d3de40805c2a13bc
Prior to this change, the SQL expression that resulted from translating permissions on functions would refer to the table of the function's return type, rather than the set of rows selected from the function being called.
Now the SQL that results from translating permissions correctly refer to the selected rows.
This PR also contains the suggested additions of https://github.com/hasura/graphql-engine-mono/pull/2563#discussion_r726116863, which simplifies the Boolean Expression IR, but in turn makes the Schema Dependency Discovery algorithm work a bit harder.
We are changing the definition of `data OpExpG`, but the format accepted by its JSON parser remains unchanged. While there does exist a generically derived `instance ToJSON OpExpG` this is only used in the (unpublished) `/v1/metadata/dump_internal_state` API.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/2609
Co-authored-by: Gil Mizrahi <8547573+soupi@users.noreply.github.com>
GitOrigin-RevId: bb9a0b4addbc239499dd2268909220196984df72
This is the next part in the series of MySQL PRs.
**Purpose**: Adds the Plan module for the data loader.
**Preceding PR:** #2511
**Next PR:** #2549
After #2511 is merged, this can be repointed to `main`. For now it's aimed at #2511, because then the diff display is simpler.
The `undefined` stubs in this PR have code already written, so they won't introduce a maintenance problem. They're only omitted for digestibility of the PR.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/2529
Co-authored-by: Abby Sassel <3883855+sassela@users.noreply.github.com>
GitOrigin-RevId: 691b35be247531d5e1ac855598e89f6dc1eca0b6
The only real use was for the dubious multitenant option
--consoleAssetsVersion, which actually overrode not just
the assets version. I.e., as far as I can tell, if you pass
--consoleAssetsVersion to multitenant, that version will
also make it into e.g. HTTP client user agent headers as
the proper graphql-engine version.
I'm dropping that option, since it seems unused in production
and I don't want to go to the effort of fixing it, but am happy
to look into that if folks feels strongly that it should be
kept.
(Reason for attacking this is that I was looking into http
client things around blacklisting, and the versioning thing
is a bit painful around http client headers.)
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/2458
GitOrigin-RevId: a02b05557124bdba9f65e96b3aa2746aeee03f4a
The Plan part is missing, because it needs support from FromIr. That'll come in a follow up commit.
**Next PR**: #2529
This is the result of splitting up the mega PR into more digestible chunks. This is the smallest subset I've been able to collect. Missing parts are noted in comments.
The code isn't reachable from Main, so it won't affect the test suite. It just gets compiled for now.
For context, this splits up work from https://github.com/hasura/graphql-engine-mono/pull/2332
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/2511
Co-authored-by: Abby Sassel <3883855+sassela@users.noreply.github.com>
GitOrigin-RevId: 00f30b0f494b56b3b7f8c1b0996377db4874c88d
>
### Description
>
Insert mutations for MSSQL backend. This PR implements execution logic.
### Changelog
- [x] `CHANGELOG.md` is updated with user-facing content relevant to this PR. If no changelog is required, then add the `no-changelog-required` label.
### Affected components
- [x] Server
- [x] Tests
### Related Issues
->
Close https://github.com/hasura/graphql-engine-mono/issues/2114
### Steps to test and verify
>
Track a MSSQL table and perform the generated insert mutation to test.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/2248
Co-authored-by: Abby Sassel <3883855+sassela@users.noreply.github.com>
Co-authored-by: Philip Lykke Carlsen <358550+plcplc@users.noreply.github.com>
GitOrigin-RevId: 936f138c80d7a928180e6e7b0c4da64ecc1f7ebc
>
### Description
>
Add a simple test case to test behavior of computed fields with session argument in filter expression (`where`) of a graphql query.
### Changelog
- [ ] `CHANGELOG.md` is updated with user-facing content relevant to this PR. If no changelog is required, then add the `no-changelog-required` label.
### Affected components
- [x] Tests
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/2465
GitOrigin-RevId: 25e287c7e7826350e93f2bebacd5d877568c9934
### Description
This PR implements operation timeouts, as specced in #1232.
RFC: [rfcs/operation-timeout-api-limits.md](c025a90fe9/rfcs/operation-timeout-api-limits.md)
There's still some things to be done (tests and docs most notably), but apart from that it can
be reviewed. I'd still appreciate feedback on the RFC!
TODO:
- [x] break out the `ApiLimits` refactoring into a separate PR: #2103
- [x] finish the `pg-client-hs` PR: https://github.com/hasura/pg-client-hs/pull/39
- [x] remove configurability, after testing, prior to merging
- [ ] tests: #2390 has some tests that I've run locally to confirm things work on a fundamental level
- [x] changelog
- [x] documentation
- [x] fill in the detailed PR checklist
### Changelog
- [x] `CHANGELOG.md` is updated with user-facing content relevant to this PR. If no changelog is required, then add the `no-changelog-required` label.
### Affected components
- [x] Server
- [ ] Console
- [ ] CLI
- [x] Docs
- [ ] Tests
### Related Issues
Product spec: #1232.
### Solution and Design
Compare `rfcs/operation-timeout-api-limits.md`.
### Steps to test and verify
Configure operation timeouts, e.g. by posting
```
{
"type": "set_api_limits",
"args": {
"operation_timeout": {
"global": 3
}
}
}
```
to `v1/metadata` to set an operation timeout of 3s. Then verify that
1. non-admin queries that take longer than 3s time out with a nice error message
2. that those queries return after ~3s (at least for postgres)
3. also that everything else still works as usual
### Limitations, known bugs & workarounds
- while this will cause slow queries against any backends to fail, it's only verified to actually interrupt queries against postgres
- this will only successfully short-cut (cancel) queries to postgres if the database server is responsive
#### Catalog upgrade
Does this PR change Hasura Catalog version?
- [x] No
#### Metadata
Does this PR add a new Metadata feature?
- [x] Yes
- Does `run_sql` auto manages the new metadata through schema diffing?
- [x] Not required
- Does `run_sql` auto manages the definitions of metadata on renaming?
- [x] Not required
- Does `export_metadata`/`replace_metadata` supports the new metadata added?
- [x] Yes
#### GraphQL
- [x] No new GraphQL schema is generated
#### Breaking changes
- [x] No Breaking changes
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/1593
GitOrigin-RevId: f0582d0be3ed9fadf89e0c4aaf96344d18331dc4
On `main`, currently, haddock generation is broken, due to some unrecognized comments. This PR fixes this, and changes our `build_oss_server` CI job to ensure that future PRs do not break haddock.
https://github.com/hasura/graphql-engine-mono/pull/2222
GitOrigin-RevId: 909bbcdc7b2d31c9a3e947ce6b7691e23f59b916
This commit applies ormolu to the whole Haskell code base by running `make format`.
For in-flight branches, simply merging changes from `main` will result in merge conflicts.
To avoid this, update your branch using the following instructions. Replace `<format-commit>`
by the hash of *this* commit.
$ git checkout my-feature-branch
$ git merge <format-commit>^ # and resolve conflicts normally
$ make format
$ git commit -a -m "reformat with ormolu"
$ git merge -s ours post-ormolu
https://github.com/hasura/graphql-engine-mono/pull/2404
GitOrigin-RevId: 75049f5c12f430c615eafb4c6b8e83e371e01c8e
### Description
- sets up a Makefile target for running ormolu to format and check source code
- updates CI to run ormolu instead of stylish-haskell (and to check instead of format actively)
Compare #1679.
Here's the plan for merging this:
1. merge this PR; at this point, all PRs will fail CI unless they have the `ignore-server-format-checks` label set
2. merge follow-up PR #2404 that does nothing but actually reformats the codebase
3. tag the merge commit as `post-ormolu` (also on `graphql-engine`, for the benefits of community contributors)
4. provide the following script to any devs in order to update their branches:
```
$ git checkout my-feature-branch
$ git merge post-ormolu^
$ make format
$ git commit -a -m "reformat with ormolu"
$ git merge -s ours post-ormolu
```
(I'll put this in the commit message)
https://github.com/hasura/graphql-engine-mono/pull/2020
Co-authored-by: Philip Lykke Carlsen <358550+plcplc@users.noreply.github.com>
Co-authored-by: Swann Moreau <62569634+evertedsphere@users.noreply.github.com>
GitOrigin-RevId: 130f480a6d79967c8d045b7f3a6dec30b10472a7
>
### Description
>
Few improvements to mssql transactions.
### Changelog
- [ ] `CHANGELOG.md` is updated with user-facing content relevant to this PR. If no changelog is required, then add the `no-changelog-required` label.
### Affected components
- [x] Server
- [ ] Console
- [ ] CLI
- [ ] Docs
- [ ] Community Content
- [ ] Build System
- [ ] Tests
- [ ] Other (list it)
https://github.com/hasura/graphql-engine-mono/pull/2324
GitOrigin-RevId: 808947188f5f3d196c7dfc4ebfa661629db5f8f7
### Description
This PR improves error messages in our metadata API by displaying a message with the name of the failing command and a link to our documentation. Furthermore, it harmonizes our internal uses of `withObject`, to respect the convention of using the Haskell type name, now that the Aeson error message is displayed as an "internal error message".
https://github.com/hasura/graphql-engine-mono/pull/1905
GitOrigin-RevId: e4064ba3290306437aa7e45faa316c60e51bc6b6
>
### Description
>
While adding [insert mutation schema parser for MSSQL backend](https://github.com/hasura/graphql-engine-mono/pull/2141) I also included [identity](https://en.wikipedia.org/wiki/Identity_column) notion to table columns across all backends. In MSSQL we cannot insert any value (even `DEFAULT` expression) into Identity columns. This behavior of identity columns is not same in Postgres as we can insert values. This PR drops the notion of identity in the column info. The context of identity columns for MSSQL is carried in `ExtraTableMetadata` type.
### Changelog
- [x] `CHANGELOG.md` is updated with user-facing content relevant to this PR. If no changelog is required, then add the `no-changelog-required` label.
### Affected components
- [x] Server
- [ ] Console
- [ ] CLI
- [ ] Docs
- [ ] Community Content
- [ ] Build System
- [x] Tests
- [ ] Other (list it)
### Related Issues
->
Fix https://github.com/hasura/graphql-engine/issues/7557https://github.com/hasura/graphql-engine-mono/pull/2378
GitOrigin-RevId: c18b5708e2e6107423a0a95a7fc2e9721e8a21a1
Some of our use of CPP causes trouble for ormolu, compare https://github.com/tweag/ormolu/issues/774.
Specifically, for understandable reasons, it can't deal well with `#ifdef` use that is not at the top-level.
This PR removes the problematic usage in ways that I hope are also a net non-loss regardless of helping
out ormolu (or other tooling).
- The default value for enabled APIs moves to the top level, next to the command line help, so
they'll stay in sync more easily.
- All the CPP around using `assertNFHere` is moved to one module.
https://github.com/hasura/graphql-engine-mono/pull/2361
GitOrigin-RevId: ed6e039e6d8960322fd8d1312df762ad197c29b1
This change was prompted by how `make ci-build` in `./server` clobbered my
`cabal.project.local`. Addressing that more directly proved awkward, thus this
change, which makes both `ci-build` targets use a shared `/cabal.project.ci.*`
(via `--with-project-file`).
Also removes some pro targets/scripts which were definitely broken thus unused.
### Affected components
- [x] Build System
### Steps to test and verify
If CI still passes, this should be safe.
https://github.com/hasura/graphql-engine-mono/pull/2244
GitOrigin-RevId: 1494824cabd2fbe6415d050c19e27f37bb51b86b
## Description
Almost all our data structures use strictness annotations, following [our styleguide's principle](https://github.com/hasura/graphql-engine/blob/master/server/STYLE.md#dealing-with-laziness) of "by default, use strict data types and lazy functions". The very few cases where we actually need laziness were already explicitly labelled as lazy with the `~` prefix operator.
This PR simply globally enables `StrictData`, allowing us to express records without `!()` on every field, but makes no attempt at cleaning existing code.
https://github.com/hasura/graphql-engine-mono/pull/1869
Co-authored-by: Philip Lykke Carlsen <358550+plcplc@users.noreply.github.com>
GitOrigin-RevId: e65c6e2f89413188da250122f64c2173615946ec
### Description
We always build a subscription root, even when there was no possible fields. This breaks some third party clients, as the spec does not allow empty types in the schema. This PR fixes this by changing the `buildSubscriptionParser` helper to return a `Maybe` value, and harmonizes / cleans places where we build the subscription root.
https://github.com/hasura/graphql-engine-mono/pull/2357
GitOrigin-RevId: 1aeae25e321eee957e7645c436d17e69207309fd
### Description
The inherited roles integration tests were behind a flag, and its corresponding fixture, presumably to avoid enabling the option globally. However, #2288 introduced a new test using inherited roles that was not gated behind the flag, which fails when run with `dev.sh`. However, that test works on CI... because inherited roles are globally enabled there.
Consequently, this PR:
- globally enables inherited roles in dev.sh
- removes the flag and the associated fixture
https://github.com/hasura/graphql-engine-mono/pull/2358
Co-authored-by: Vishnu Bharathi <4211715+scriptnull@users.noreply.github.com>
GitOrigin-RevId: ebfa6754873324bed15b2cc5e37ec2d8008e8f8d
This is a follow-up to #1959.
Today, I spent a while in review figuring out that a harmless PR change didn't do anything,
because it was moving from a `runLazy...` to something without the `Lazy`. So let's get
that source of confusion removed.
This should be a bit easier to review commit by commit, since some of the functions had
confusing names. (E.g. there was a misnamed `Migrate.Internal.runTx` before.)
The change should be a no-op.
https://github.com/hasura/graphql-engine-mono/pull/2335
GitOrigin-RevId: 0f284c4c0f814482d7827e7732a6d49e7735b302
### Description
During the PDV refactor that led to 2.0, we broke an undocumented and untested semantic of inserts: accepting _explicit_ null values in nested object inserts.
In short: in the schema, we often distinguish between _explicit_ null values `{id: 3, author: null}` and _implicit_ null values that correspond to the field being omitted `{id: 3}`. In this particular case, we forgot to accept explicit null values. Since the field is optional (meaning we accept implicit null values), it was nullable in the schema, like it was in pre-PDV times. But in practice we would reject explicit nulls.
This PR fixes this, and adds a test. Furthermore, it does a bit of a cleanup of the Mutation part of the schema, and more specifically of all insertion code.
https://github.com/hasura/graphql-engine-mono/pull/2341
GitOrigin-RevId: 895cfeecef7e8e49903a3fb37987707150446eb0
This PR only contains minor changes to documentation that I have collected over some time, revising text as I was passing by.
https://github.com/hasura/graphql-engine-mono/pull/2346
Co-authored-by: Rikin Kachhia <54616969+rikinsk@users.noreply.github.com>
GitOrigin-RevId: f3329f3212b831f1f3c74a299734faff337b1017
### Description
Our python test suite has several major problems; one of them being that the tests themselves are not responsible for their own setup. We are therefore using environment variables for all matters of configuration, such as _where the postgres instance is_. This is something that should be changed, but in the meantime, it is the test implementer's responsibility to ensure that tests have a consistent setup in CI and locally, or to to add the proper "skip" annotations.
The recently added `test_pg_add_source_with_source_parameters` fails to do so: as it tests adding a postgres source from hardcoded parameters, rather than relying on environment variables, it only works if the postgres instance is at the matching address, which happens to be the one set in the circle ci config. This is undesirable for two reasons:
- it breaks local tests: running tests locally with `dev.sh` sets postgres up differently, and the test fails;
- a change to the circle config would result in failures in that test.
Sadly, there's no good solution here: our tests do not currently support expanding environment variables in the queries' yaml files, meaning it's not possible to set the values of all those parameters differently in each environment. And we haven't yet started working towards having a unified testing environment setup.
As a result, this PR disables the offending test UNLESS the postgres instance happens to be exactly where the test expects it. This is also very inelegant and adds more tech debt to the pile, but I do not see how to fix this with our current test infrastructure. :(
https://github.com/hasura/graphql-engine-mono/pull/2336
GitOrigin-RevId: 8bc9142075d14acaa48e9c4b20de2527185bc75c
This moves the previous (illegal) `Show` instance for `Hasura.Base.Error.Code` to a `ToJSON`
instance, and uses that in the error `ToJSON` instances.
Addressing https://github.com/hasura/graphql-engine-mono/pull/2277#issuecomment-911557169.
This PR is against #2277.
It adds a replacement derived `Show` instance, which is used:
- in the derived `Show` instance for `QErr`
- in some unit tests
Mostly verified that we didn't otherwise rely on the hand-rolled `Show`
instance by compiling without it (and a faked `QErr` instance), and seeing
that the only compile failures were in tests. (Compare the individual commits.)
https://github.com/hasura/graphql-engine-mono/pull/2279
GitOrigin-RevId: 678fe241a14bd0c9aaf5b267efc510ad9d619dd7
The materialized views cannot be mutated, so this commit removes the option to run mutation on the materialized views via graphql endpoint. Before this, users could have tried running mutation for the materialized views using the graphql endpoint (or from HGE console), which would have resulted in the following error:
``` JSON
{
"errors": [
{
"extensions": {
"internal": {
"statement": "WITH \"articles_mat_view__mutation_result_alias\" AS (DELETE FROM \"public\".\"articles_mat_view\" WHERE (('true') AND (((((\"public\".\"articles_mat_view\".\"id\") = (('20155721-961c-4d8b-a5c4-873ed62c7a61')::uuid)) AND ('true')) AND ('true')) AND ('true'))) RETURNING * ), \"articles_mat_view__all_columns_alias\" AS (SELECT \"id\" , \"author_id\" , \"content\" , \"test_col\" , \"test_col2\" FROM \"articles_mat_view__mutation_result_alias\" ) SELECT json_build_object('affected_rows', (SELECT COUNT(*) FROM \"articles_mat_view__all_columns_alias\" ) ) ",
"prepared": false,
"error": {
"exec_status": "FatalError",
"hint": null,
"message": "cannot change materialized view \"articles_mat_view\"",
"status_code": "42809",
"description": null
},
"arguments": []
},
"path": "$",
"code": "unexpected"
},
"message": "database query error"
}
]
}
```
So, we don't want to generate the mutation fields for the materialized views altogether.
https://github.com/hasura/graphql-engine-mono/pull/2226
GitOrigin-RevId: 4ef441764035a8039e1c780d454569ee1f2febc3
>
### Description
>
Correctly alias the aggregate field projections in site instead of aliasing them later stage.
PS: I discovered this required change while [developing SQL generation for MSSQL inserts](https://github.com/hasura/graphql-engine-mono/pull/2248).
### Changelog
- [ ] `CHANGELOG.md` is updated with user-facing content relevant to this PR. If no changelog is required, then add the `no-changelog-required` label.
### Affected components
- [x] Server
https://github.com/hasura/graphql-engine-mono/pull/2271
GitOrigin-RevId: 0d90fd8d8c0541b18ca9cb1197e413f3454bb227
>
### Description
>
This PR is an incremental work towards [enabling insert mutations on MSSQL](https://github.com/hasura/graphql-engine-mono/pull/1974). In this PR, we generate insert mutation schema parser for MSSQL backend.
### Changelog
- [ ] `CHANGELOG.md` is updated with user-facing content relevant to this PR. If no changelog is required, then add the `no-changelog-required` label.
### Affected components
- [x] Server
https://github.com/hasura/graphql-engine-mono/pull/2141
GitOrigin-RevId: 8595008dece35f7fded9c52e134de8b97b64f53f
