### 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
When adding object relationships, we set the nullability of the generated GraphQL field based on whether the database backend enforces that the referenced data always exists. For manual relationships (corresponding to `manual_configuration`), the database backend is unaware of any relationship between data, and hence such fields are always set to be nullable.
For relationships generated from foreign key constraints (corresponding to `foreign_key_constraint_on`), we distinguish between two cases:
1. The "forward" object relationship from a referencing table (i.e. which has the foreign key constraint) to a referenced table. This should be set to be non-nullable when all referencing columns are non-nullable. But in fact, it used to set it to be non-nullable if *any* referencing column is non-nullable, which is only correct in Postgres when `MATCH FULL` is set (a flag we don't consider). This fixes that by changing a boolean conjunction to a disjunction.
2. The "reverse" object relationship from a referenced table to a referencing table which has the foreign key constraint. This should always be set to be nullable. But in fact, it used to always be set to non-nullable, as was reported in hasura/graphql-engine#7201. This fixes that.
Moreover, we have moved the computation of the nullability from `Hasura.RQL.DDL.Relationship` to `Hasura.GraphQL.Schema.Select`: this nullability used to be passed through the `riIsNullable` field of `RelInfo`, but for array relationships this information is not actually used, and moreover the remaining fields of `RelInfo` are already enough to deduce the nullability.
This also adds regression tests for both (1) and (2) above.
https://github.com/hasura/graphql-engine-mono/pull/2159
GitOrigin-RevId: 617f12765614f49746d18d3368f41dfae2f3e6ca
In hasura/graphql-engine#7172, an issue was found where under certain conditions a JSON field from Postgres would be parsed as a GraphQL input object, which is not possible in general, and also unnecessary. Luckily, this was already fixed by the time `v2.0.6` got around, presumably thanks to 4a83bb1834. This adds a regression test.
https://github.com/hasura/graphql-engine-mono/pull/2158
GitOrigin-RevId: 1ded1456f6b89726e08f77cf3383ad88c04de451
This removes the module re-exports of [Data.Align](https://hackage.haskell.org/package/semialign-1.2/docs/Data-Align.html) and [Data.These](https://hackage.haskell.org/package/these-1.1.1.1/docs/Data-These.html) from `Hasura.Prelude`. The reasoning being that they're not used widely and reasonably obscure, and that being explicit about the imports makes for an easier to understand codebase.
(I spent longer than I'd have liked earlier today figuring out where `align` in multitenant came from.
The right one not showing up on the first hoogle page doesn't help. Yes, better tool use could have
avoided that, but still...)
Do feel free to shoot this down, I won't insist on the change.
https://github.com/hasura/graphql-engine-mono/pull/2194
GitOrigin-RevId: 10f887b74538b17623bee6d6451c5aba11573fbd
Replaces one instance of `mtl`-style effects with `transformers`-style, as this results in a measurable reduction in memory usage. The change is kept completely within one module.
https://github.com/hasura/graphql-engine-mono/pull/1944
GitOrigin-RevId: 587b8e61725bb4a505404bbe741185759b7bceeb
This should be mostly a no-op change, with one exception:
When limits are not disabled, but neither node nor depth limit
is configured, we no longer count nodes/depth uselessly.
https://github.com/hasura/graphql-engine-mono/pull/2103
GitOrigin-RevId: 9943f89d6b969ca101a9a5601417c5b14a358a10
We also added a missing `--network=host` to the postgres container, which it turns out improves performance numbers a bit (hopefully increases stability a bit too).
https://github.com/hasura/graphql-engine-mono/pull/2149
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: 3fffc8fbfc77606dd26421eed079629306b08d05
This removes the file `bahnql_query.yaml`, which is no longer being used.
a509a86eaa (hasura/graphql-engine#1117) changed the way we test the remote schema feature from using external GraphQL services to running our own mini GraphQL server for testing purposes. This gives us a lot of in-codebase flexibility on the behavior of "remote" GraphQL servers.
During this work, the `bahnql_query.yaml` test was swapped out for the `simple2_query.yaml` test. The former essentially tests if a field from a remote schema can be fetched, whereas the latter tests whether an entry can be fetched from the (non-remote!) database.
It's not clear to me why `bahnql_query.yaml` was no longer used. In any case, the relevant setup code was removed, and this test can no longer be run. Presumably we test such basic functionality already in many other ways.
https://github.com/hasura/graphql-engine-mono/pull/2102
GitOrigin-RevId: c01b7f7ec5c767c874bca2ddad991eb81a0e2809
>
### Description
>
This PR supersedes https://github.com/hasura/graphql-engine-mono/pull/1484. Apply `limit` to the table selection before joining relationship rows to improve query performance.
### 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
### Related Issues
->
Fix https://github.com/hasura/graphql-engine/issues/5745
### Solution and Design
>
Prior to this change, we apply `LIMIT` and `OFFSET` to the outer selection from sub-query which includes joins for relationships. Now, we move `LIMIT` and `OFFSET` (if present) to inner selection of base table. But, this isn't done always! If there are order by relationships' columns we apply at the outer selection. To know more, please refer to [source code note](https://github.com/hasura/graphql-engine-mono/pull/2078/files#diff-46d868ee45d3eaac667cebb34731f573c77d5c9c8097bb9ccf1115fc07f65bfdR652).
```graphql
query {
article(limit: 2){
id
title
content
author{
name
}
}
}
```
Before:
```sql
SELECT
coalesce(json_agg("root"), '[]') AS "root"
FROM
(
SELECT
row_to_json(
(
SELECT
"_4_e"
FROM
(
SELECT
"_0_root.base"."id" AS "id",
"_0_root.base"."title" AS "title",
"_0_root.base"."content" AS "content",
"_3_root.or.author"."author" AS "author"
) AS "_4_e"
)
) AS "root"
FROM
(
SELECT
*
FROM
"public"."article"
WHERE
('true')
) AS "_0_root.base"
LEFT OUTER JOIN LATERAL (
SELECT
row_to_json(
(
SELECT
"_2_e"
FROM
(
SELECT
"_1_root.or.author.base"."name" AS "name"
) AS "_2_e"
)
) AS "author"
FROM
(
SELECT
*
FROM
"public"."author"
WHERE
(("_0_root.base"."author_id") = ("id"))
) AS "_1_root.or.author.base"
) AS "_3_root.or.author" ON ('true')
LIMIT
2
) AS "_5_root"
```
cost
```
Aggregate (cost=0.73..0.74 rows=1 width=32)
-> Limit (cost=0.15..0.71 rows=2 width=32)
-> Nested Loop Left Join (cost=0.15..223.96 rows=810 width=32)
-> Seq Scan on article (cost=0.00..18.10 rows=810 width=72)
-> Index Scan using author_pkey on author (cost=0.15..0.24 rows=1 width=36)
Index Cond: (article.author_id = id)
SubPlan 1
-> Result (cost=0.00..0.01 rows=1 width=32)
SubPlan 2
-> Result (cost=0.00..0.01 rows=1 width=32)
```
After:
```sql
SELECT
coalesce(json_agg("root"), '[]') AS "root"
FROM
(
SELECT
row_to_json(
(
SELECT
"_4_e"
FROM
(
SELECT
"_0_root.base"."id" AS "id",
"_0_root.base"."title" AS "title",
"_0_root.base"."content" AS "content",
"_3_root.or.author"."author" AS "author"
) AS "_4_e"
)
) AS "root"
FROM
(
SELECT
*
FROM
"public"."article"
WHERE
('true')
LIMIT
2
) AS "_0_root.base"
LEFT OUTER JOIN LATERAL (
SELECT
row_to_json(
(
SELECT
"_2_e"
FROM
(
SELECT
"_1_root.or.author.base"."name" AS "name"
) AS "_2_e"
)
) AS "author"
FROM
(
SELECT
*
FROM
"public"."author"
WHERE
(("_0_root.base"."author_id") = ("id"))
) AS "_1_root.or.author.base"
) AS "_3_root.or.author" ON ('true')
) AS "_5_root"
```
cost:
```
Aggregate (cost=16.47..16.48 rows=1 width=32)
-> Nested Loop Left Join (cost=0.15..16.44 rows=2 width=100)
-> Limit (cost=0.00..0.04 rows=2 width=72)
-> Seq Scan on article (cost=0.00..18.10 rows=810 width=72)
-> Index Scan using author_pkey on author (cost=0.15..8.18 rows=1 width=36)
Index Cond: (article.author_id = id)
SubPlan 1
-> Result (cost=0.00..0.01 rows=1 width=32)
SubPlan 2
-> Result (cost=0.00..0.01 rows=1 width=32)
```
https://github.com/hasura/graphql-engine-mono/pull/2078
Co-authored-by: Evie Ciobanu <1017953+eviefp@users.noreply.github.com>
GitOrigin-RevId: 47eaccdbfb3499efd2c9f733f3312ad31c77916f
This is just a one-off fix, based on running ormolu across
the code base, which uses GHC's parser in haddock mode.
### Description
Fixes several instances of illegal haddock comments.
### Related Issues
#1679
### Steps to test and verify
Run ormolu over the codebase. Prior to this change, it complains that it
can't parse certain files due to malformed Haddock comments, after it
doesn't (there are still some other errors).
### Limitations, known bugs & workarounds
This doesn't ensure that we don't introduce similar issues in the future;
that'll be dealt with once we implement #1679.
#### Breaking changes
- [x] No Breaking changes, only touches code comments
https://github.com/hasura/graphql-engine-mono/pull/2010
GitOrigin-RevId: 7fbab0325ce13a16a04ff98d351f1af768e25d7c
## Suggestion: Add fancier trace debugging functions to `Hasura.Prelude`
This PR adds two trace functions, `ltrace` and `ltraceM`, which use the `pretty-simple` package to `show` the input with nice formatting and colors for ease of reading (and comparing using diff tools such as `meld` or `vim-diff`).
I've also added warning pragmas to the functions, which means:
1. Traces will not be left in code, as CI builds with -Werror
2. Developers will have to change the `ghc-options` to `-Wwarn` in their `cabal.project.local` settings to use these functions
### Example
Usage:
```hs
selectFunctionAggregate ... = ... do
ltraceM "functionInfo" function
...
```
Output to terminal looks like this:
<img width="524" alt="Screen Shot 2021-08-12 at 10 33 24" src="https://user-images.githubusercontent.com/8547573/129158878-4a5e96ba-30a5-452c-8f33-9eb4b2cc5e2a.png">
### Dependencies
Requires adding the following dependencies:
- prettyprinter-ansi-terminal-1.1.2 (BSD2)
- pretty-simple-4.0.0.0 (BSD3)
Question: what is the process for adding new dependencies? How does decisions on this matter happen?
https://github.com/hasura/graphql-engine-mono/pull/2075
GitOrigin-RevId: 490b0f0ca595da319b43e92e190ba50c0b132cd5
>
### Description
>
From HGE version 2.0 onwards, all remote relationship fields are generated as plain types without non-nullable and lists. This PR fixes the same.
### 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
->
fix https://github.com/hasura/graphql-engine/issues/7284
### Steps to test and verify
>
- Create a remote relationship to a field in remote schema with non-nullable or list type
- The HGE introspection should give the remote relationship field type correctly as like in the remote schema
https://github.com/hasura/graphql-engine-mono/pull/2071
GitOrigin-RevId: e113f5d17b62bfa0a25028c20260ae1782ae224b
This PR adds source code documentation for metadata versioning.
Note: Adding `force-skip-ci` label to bypass server tests as this PR only has changes in source code comments.
https://github.com/hasura/graphql-engine-mono/pull/2026
GitOrigin-RevId: f24dcc1579f98e59e40677b99890a8af273bb96e
### A long tale about encoding
GraphQL has an [introspection system](http://spec.graphql.org/June2018/#sec-Introspection), which allows its schema to be introspected. This is what we use to introspect [remote schemas](41383e1f88/server/src-rsr/introspection.json). There is one place in the introspection where we might find GraphQL values: the default value of an argument.
```json
{
"fields": [
{
"name": "echo",
"args": [
{
"name": "msg",
"defaultValue": "\"Hello\\nWorld!\""
}
]
}
]
}
```
Note that GraphQL's introspection is transport agnostic: the default value isn't returned as a JSON value, but as a _string-encoded GraphQL Value_. In this case, the value is the GraphQL String `"Hello\nWorld!"`. Embedded into a string, it is encoded as: `"\"Hello\\nWorld!\""`.
When we [parse that value](41383e1f88/server/src-lib/Hasura/GraphQL/RemoteServer.hs (L351)), we first extract that JSON string, to get its content, `"Hello\nWorld!"`, then use our [GraphQL Parser library](21c1ddfb41/src/Language/GraphQL/Draft/Parser.hs (L200)) to interpret this: we find the double quote, understand that the content is a String, unescape the backslashes, and end up with the desired string value: `['H', 'e', 'l', 'l', 'o', '\n', 'W', 'o', 'r', 'l', 'd', '!']`. This all works fine.
However, there was a bug in the _printer_ part of our parser library: when printing back a String value, we would not re-escape characters properly. In practice, this meant that the GraphQL String `"Hello\nWorld"` would be encoded in JSON as `"\"Hello\nWorld!\""`. Note how the `\n` is not properly double-escaped. This led to a variety of problems, as described in #1965:
- we would successfully parse a remote schema containing such characters in its default values, but then would print those erroneous JSON values in our introspection, which would _crash the console_
- we would inject those default values in queries sent to remote schemas, and print them wrong doing so, sending invalid values to remote schemas and getting errors in result
It turns out that this bug had been lurking in the code for a long time: I combed through the history of [the parser library](https://github.com/hasura/graphql-parser-hs), and as far as I can tell, this bug has always been there. So why was it never caught? After all, we do have [round trip tests](21c1ddfb41/test/Spec.hs (L52)) that print + parse arbitrary values and check that we get the same value as a result. They do use any arbitrary unicode character in their generated strings. So... that should have covered it, right?
Well... it turns out that [the tests were ignoring errors](7678066c49/test/Spec.hs (L45)), and would always return "SUCCESS" in CI, even if they failed... Furthermore, the sample size was small enough that, most of the time, _they would not hit such characters_. Running the tests locally on a loop, I only got errors ~10% of the time...
This was all fixed in hasura/graphql-parser-hs#44. This was probably one of Hasura's longest standing bugs? ^^'
### Description
This PR bumps the version of graphql-parser-hs in the engine, and switches some of our own arbitrary tests to use unicode characters in text rather than alphanumeric values. It turns out those tests were much better at hitting "bad" values, and that they consistently failed when generating arbitrary unicode characters.
https://github.com/hasura/graphql-engine-mono/pull/2031
GitOrigin-RevId: 54fa48270386a67336e5544351691619e0684559
### Description
A first PR, #1947, removed all the `Arbitrary` stuff from our codebase. But #1740, merged on the same day, added some tests relying on `Arbitrary`. In the merge process, some unneeded `Arbitrary` code got reintroduced.
This PR removes all `Arbitrary` stuff from `src-lib`, and cleans / refactor `Hasura.Generator` in `src-test` to only reduce it to the bare minimum amount of `Arbitrary` instances.
https://github.com/hasura/graphql-engine-mono/pull/1957
GitOrigin-RevId: 7e76009bb022205e3737fca45749411a266cc08c
The `LazyTxT` type was introduced to avoid connecting to Postgres when a given GraphQL request did not require this. However, through the new way query execution plans are represented, this has now _mostly_ been taken care of in a different way, and so no `LazyTxT` action is generated at all for GraphQL requests that do not fetch data from Postgres.
This removes the laziness of `LazyTxT` by simply making it a newtype wrapper around `Q.TxET`. This simplifies a lot of code in `Hasura.Backends.Postgres.Connection`.
https://github.com/hasura/graphql-engine-mono/pull/1959
GitOrigin-RevId: 58b4d5a05d67bc602b59e02ac338f1c3e63859c7
This PR:
- removes a dependency on Postgres' `ToSQL` in other backends
- removes some usage of `unsafePerformIO` in `FronJSON` / `Arbitrary`
- fixes a `Set` into a `HashSet`
- moves some orphan instances where they belong:
- alongside others in `Hasura.Incremental` for `Cacheable`
- in `Hasura.Base.Instances` for `Hashable`
- introduces a local wrapper around ByteString to avoid unsound UTF8 instances
Some of the weird empty lines come from the fact that this PR is an offshoot of #1947.
https://github.com/hasura/graphql-engine-mono/pull/1949
GitOrigin-RevId: ef9d34452946f8466878d8fdda857b0b43816de7
