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
