## Description
As the name suggests, `DML.Internal` contains internal implementation details of RQL's DML. However, a lot of unrelated parts of the codebase still use some of the code it contains. This PR fixes this, and removes all imports of `RQL.DML.Internal` from outside of `RQL.DML`. Most of the time, this involves moving a function out of `DML.Internal` to an underlying module (see `getRolePermInfo`) or moving a function _back_ into it (see `checkRetCols`).
This PR also clarifies a bit the situation with `withTyAnn` and `withTypeAnn` by renaming the former into `withScalarTypeAnn` and moving them together. Worth noting: there might be a bug lurking in that function, as it doesn't seem to use the proper type annotations for some extension types!
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4380
GitOrigin-RevId: c8ae5b4e8378fefc0bcccf778d97813df727d3cb
## Description
This PR removes `RQL.Types`, which was now only re-exporting a bunch of unrelated modules.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4363
GitOrigin-RevId: 894f29a19bff70b3dad8abc5d9858434d5065417
## Description
This small PR moves all functions in `RQL.Types.hs` to better locations. Most `askX` functions are moved alongside the `unsafe` functions they use. Several other functions are moved closer to their call site. `MetadataM` is moved alongside `Metadata`. This PR also documents the `ask` functions.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4355
GitOrigin-RevId: 0498a7e8f98e7a94af911dd375cad84ace7ddffa
### Description
`HasSystemDefined` is defined in `RQL.Types`, but only used in one place, `LegacyCatalog`, to avoid passing a boolean around. It is easily replaced by an ad-hoc `ReaderT`.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4337
GitOrigin-RevId: 649d758bb2b18b39533429dda5ab71afde62fb53
### Description
Small PR that moves code out of `RQL.Types.hs`. Specifically, it moves `HasServerConfigCtx` to where `ServerConfigCtx` is defined. This removes code from `RQL.Types`, makes the dependency on `Server.Types` more explicit, and will make some further cleanups easier.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4336
GitOrigin-RevId: 95bb3467d741763892c4e68a38760497157ba1aa
With the current implementation, only the first call to `waitForShutdown` on a given
`ShutdownLatch` will return, while others will block (typically indefinitely). That's not
how one would expect a shutdown latch to work.
This isn't currently a concrete issue because we only wait once on each `ShutdownLatch`.
But in the context of #4154 we'll probably end up wanting to wait for shutdown from
multiple threads.
This adds a number of tests to verify the current behaviour, and adds a test for multiple
`waitForShutdown` calls that fails prior to the functional change.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4162
GitOrigin-RevId: 9a108858d11390b847404f30bc7b93c06fc3f966
In hasura/graphql-engine@7547786b2b, we made the logic that does a specific correctness check on the GraphQL schema runnable _post_-startup. However, in order to error out as early as possible, we explicitly do it on startup anyway.
Unfortunately a few instances of that logic were forgotten: until now we only executed it for non-relay authenticated GraphQL schemas.
This re-implements the "conflicting types" check for both the Relay schema, and for the unauthenticated schemas.
One key point where it does _not_ re-implement it is in `buildFullestDBSchema`, which is a very sketchy method that builds a GraphQL schema that is _only_ used internally in order to mark parts of the schema as inconsistent. There are two reasons I do not add the check here:
- This logic should get replaced entirely - see hasura/graphql-engine-mono#3882.
- Having conflicting types in `buildFullestDBSchema` does not imply there is actually a user-facing issue. The only "conflicting types" which are problematic are the ones which result in illegal _user-facing_ GraphQL schemas. `buildFullestDBSchema` is _never_ user-facing.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3885
GitOrigin-RevId: 49517dcbf51bcd97a3890514ec5e47499f770f32
`encJFromAssocList` is broken since it does not appropriately escape the keys of the JSON dictionaries. Presumably it should.
This is a hotfix for a JSON encoding issue. Longer-term solutions are being discussed in hasura/graphql-engine-mono#3736.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4139
GitOrigin-RevId: 27afff319d268a4ec017c8eafd694cc17c2fd7f0
UPDATE: After testing in CI it turns out that the compile time Improvement is better than expected: even though we always have to recompile the OSS lib (due to Version.hs), downstream packages like Pro and multi-tenant can still benefit from some caching and avoid full recompilation. In the best case this takes us from 22 minutes to 13 minutes total.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4104
GitOrigin-RevId: 76cbfc157064b33856e30f4c2b2ab2366f9c6089
### Motivation
While we strive to write clear code, we have historically struggled at Hasura with having very different styles and standards across the codebase. There's been efforts to standardize our coding style, we have an official styleguide that isn't maintained as closely as it should... We still have some work in front of us.
However, in the last ~year or so, there's been a huge push towards incrementally improving the situation. As part of this we've been blocking PRs that don't add enough comments, or don't improve the files that they touch.
While looking at `Hasura.GraphQL.Analyse`, it became apparent that this file did not meet the engineering standards that I would expect to see addressed during a code review. Some ways in which I think it falls short:
- lack of documentation
- no clear distinction between public / internal components
- "unidiomatic" Haskell code (such as using `Either Result Error`)
While there's no problem with a file looking like this during development, those issues should have been caught at review time. The fact that they weren't indicates a problem in our process that we will need to address: code quality and maintainability is paramount, and we all need to do our part.
### Description
This PR rewrites all of `Hasura.GraphQL.Analyze`, and adapts `Hasura.Server.OpenAPI` accordingly where needed. I've attempted to clarify names and add documentation based on my understanding of the code, and to clean what was unused (such as field variables). I don't think this PR is good enough as is, and I welcome criticism where I got my comments wrong / am happy to help y'all add more.
This PR makes one small change in the way error messages are reported (and adjusts the corresponding test accordingly); each error message is now prefixed with the path within the selection set:
```
⚠️ $.test.foo.bar.baz.mizpelled: field 'mizpelled' not found in object 'Baz'
```
### Note
This PR is currently **on top of #3962**. You can preview the changes in isolation by [diffing the branches](https://github.com/hasura/graphql-engine-mono/compare/nicuveo/clean-rest-endpoint-inconsistency-check..nicuveo/rewrite-analysis).
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3963
Co-authored-by: paritosh-08 <85472423+paritosh-08@users.noreply.github.com>
GitOrigin-RevId: 5ec38e0e753f0c12096a350db0737658495e2f15
## Description
Some of the documentation/organizational changes I was putting into the suggestions for #3624 were a bit too convoluted for GitHub's suggestion interface, so I'm putting them here instead.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3910
Co-authored-by: Solomon <24038+solomon-b@users.noreply.github.com>
GitOrigin-RevId: 06e0cb08bd18e7f8b21452df0697cfd80bc56fde
### Motivation
#2338 introduced a way to validate REST queries against the metadata after a change, to properly report any inconsistency that would emerge from a change in the underlying structure of our schema. However, the way this was done was quite complex and error-prone. Namely: we would use the generated schema parsers to statically execute an introspection query, similar to the one we use for remote schemas, then parse the resulting bytestring as it were coming from a remote schema.
This led to several issues: the code was using remote schema primitives, and was associated with remote schema code, despite being unrelated, which led to absurd situations like creating fake `Variable`s whose type was also their name. A lot of the code had to deal with the fact that we might fail to re-parse our own schema. Additionally, some of it was dead code, that for some reason GHC did not warn about? But more fundamentally, this architecture decision creates a dependency between unrelated pieces of the engine: modifying the internal processing of root fields or the introspection of remote schemas now risks impacting the unrelated `OpenAPI` feature.
### Description
This PR decouples that process from the remote schema introspection logic and from the execution engine by making `Analyse` and `OpenAPI` work on the generic `G.SchemaIntrospection` instead. To accomplish this, it:
- adds `GraphQL.Parser.Schema.Convert`, to convert from our "live" schema back to a flat `SchemaIntrospection`
- persists in the schema cache the `admin` introspection generated when building the schema, and uses it both for validation and for generating the `OpenAPI`.
### Known issues and limitations
This adds a bit of memory pressure to the engine, as we persist the entire schema in the schema cache. This might be acceptable in the short-term, but we have several potential ideas going forward should this be a problem:
- cache the result of `Analyze`: when it becomes possible to build the `OpenAPI` purely with the result of `Analyze` without any additional schema information, then we could cache that instead, reducing the footprint
- caching the `OpenAPI`: if it doesn't need to change every time the endpoint is queried, then it should be possible to cache the entire `OpenAPI` object instead of the schema
- cache a copy of the `FieldParsers` used to generate the schema: as those are persisted through the GraphQL `Context`, and are the only input required to generate the `Schema`, making them accessible in the schema cache would allow us to have the exact same feature with no additional memory cost, at the price of a slightly slower and more complicated process (need to rebuild the `Schema` every time we query the OpenAPI endpoint)
- cache nothing at all, and rebuild the admin schema from scratch every time.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3962
Co-authored-by: paritosh-08 <85472423+paritosh-08@users.noreply.github.com>
GitOrigin-RevId: a8b9808170b231fdf6787983b4a9ed286cde27e0
### Description
This is it! This PR enables the Metadata API for remote relationships from remote schemas, adds tests, ~~adds documentation~~, adds an entry to the Changelog. This is the release PR that enables the feature.
### Checklist
- [ ] Tests:
- [x] RS-to-Postgres (high level)
- [x] RS-to-RS (high level)
- [x] From RS specifically (testing for edge cases)
- [x] Metadata API tests
- [ ] Unit testing the actual engine?
- [x] Changelog entry
- [ ] Documentation?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3974
Co-authored-by: Vamshi Surabhi <6562944+0x777@users.noreply.github.com>
Co-authored-by: Vishnu Bharathi <4211715+scriptnull@users.noreply.github.com>
Co-authored-by: jkachmar <8461423+jkachmar@users.noreply.github.com>
GitOrigin-RevId: c9aebf12e6eebef8d264ea831a327b968d4be9d2
### Description
This very small PR fixes an error introduced in #3811, when changing the collision detection code: we were properly doing collision detection for remote schemas for the unauthenticated context, and also removing remote relationships... but then we were not using the result to build the schema.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3986
GitOrigin-RevId: 26a5553bf82574f2764fd594b0616dfea95a4757
- remove an unused return value
- untangle database query logic slightly
- rename printErrExit functions, and use them more consistently
- simplify the top-level exception handling
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3900
GitOrigin-RevId: a6727c6f899aed00e6a04bd822727341fd51acc4
### Description
This PR extends the `RemoteSchema` parsers to also include remote relationships. This include a significant refactoring of the top level schema building blocks, since remote schemas can no longer be built in isolation: they have to be built within the same run of `MonadSchema` as the sources. It is originally taken from the changes in #3069 and was slightly adapted.
I highly recommend turning OFF whitespace in the Github UI for `Schema.hs`, since I've adjusted the indentation of two large functions.
### Warning
Given the lack of a feature flag, this PR technically **enables the feature**. While the metadata API is not plugged in, a savvy user could use `replace_metadata` to set a metadata that contains remote joins from remote schemas, and they would be enabled. Is this acceptable?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3811
GitOrigin-RevId: a5b00f865cdb8890b0fc02b139c2ebd48929f138
### Description
This small PR improves the representation of an endpoint method from `Text` to an enum of the supported methods. Additionally, it cleans some of the instances defined on surrounding types (such as Postgres-specific instances on Endpoint types).
Due to a name conflict, this makes `RQL.Types.Endpoint` impossible to re-export from `RQL.Types`, which in turn forces several other modules to import it explicitly, which I think is fine since we want to ultimately get rid of `RQL.Types`.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3965
GitOrigin-RevId: 33869007d0d818ddf486fb61d1f6099f9dad7570
<!-- Thank you for ss in the Title above ^ -->
## Description ✍️
<!-- Please fill this se-->
<!-- Describe the changes from a user's perspective -->
Add some documentation on `ActionFieldG` type.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3945
GitOrigin-RevId: d9543ed7a8fe3ccfe9f5267c3a2ac71fb040f4db
### Description
This PR cleans `processRemoteJoins` by splitting the code, introducing comments, and applied the same strategies than #3810 did. Most importantly, it introduces a new module `RemoteJoin.Source`, made to be very similar to `RemoteJoin.RemoteSchema`, that exposes the required tooling to make a join call to a source, which decluters `Join`. Furthermore, this PR uses the same "dependency injection" to make the core of `Join` free from IO: this opens the door to testing the join engine in the unit tests.
None of the functions were modified when moved from their old module to the new one, but there's no way to easily see this in a diff.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3894
GitOrigin-RevId: 1e7c43006f092326e061f9ba12674e207b628bef
### Description
This PR moves Hasura-specific schema functions from `Hasura.GraphQL.Parser.Class` into `Hasura.GraphQL.Schema.Common`. It also removes the two corresponding monad aliases, and consequently harmonizes several parts of the code to use the same common constraint.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3947
GitOrigin-RevId: 40985a7d86da97a311bd480f9a57cc18c350c2a8
## Description
We go through the module `Hasura.Backends.MSSQL.FromIr` and split it into separate self-contained units, which we document.
Note that this PR has a slightly opinionated follow-up PR #3909 .
### Related Issues
Fix#3666
### Solution and Design
The module `FromIr` has given rise to:
* `FromIr.Expression`
* `FromIr.Query`
* `FromIr.Delete`
* `FromIr.Insert`
* `FromIr.Update`
* `FromIr.SelectIntoTempTable`
And `Execute.MutationResponse` has become `FromIr.MutationResponse` (after some slight adaptation of types).
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3908
GitOrigin-RevId: 364acf1bcdf74f2e19464c31cdded12bd8e9aa59
### Description
This PR improves the `Collect` module by re-ordering the functions to make clear what is public API and what is internal implementation. Furthermore, it makes use of `traverseOf` and `traverseFields` to reduce duplication. To do so, it also introduces a few more lenses in the rest of the codebase, and uses this opportunity to harmonize some structures that were not following our naming convention.
While the diff is massive, a lot of it is just code moving around; the file is now divided into separate sections:
- entry points: IR types for which we want to run the collection
- internal monadic structure
- internal traversals: functions that do nothing but drill down further
- actual transformations: the three cases where we do actually have work to do: selection sets on which we do want to insert join columns, extract remote relationships... those functions are left unchanged by this PR
- internal helpers
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3863
GitOrigin-RevId: f7cbecfae9eed9737b62acfa5848bfcf9d4651f6
### Description
#3810 was merged with comments still open; this small PR does a few minute clean-ups to address some remaining nits.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3941
GitOrigin-RevId: 3d15eb399828123640a73247b848bc4ddff02c38
### Description
This very small PR introduces `unionWithM`, to allow hashmap union that might fail, and uses it to transform an `error` into a `throw500`. It also reorders `HashMap.Strict.Extended` to group all "union" functions together.
There is, however, a broader question of whether we should encourage the proliferation of such functions. If so, we might also want to consider:
- `mapWithKeyM`, to remove the `unsafeMkName` of `RemoteJoin.Collect`
- `forWithKey`, as a flipped version of `traverseWithKey`
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3917
GitOrigin-RevId: a488d5bf04a73878b9e42f27ede36199bb4c920a
### Description
This PR adds the ability to perform remote joins from remote schemas in the engine. To do so, we alter the definition of an `ExecutionStep` targeting a remote schema: the `ExecStepRemote` constructor now expects a `Maybe RemoteJoins`. This new argument is used when processing the execution step, in the transport layer (either `Transport.HTTP` or `Transport.WebSocket`).
For this `Maybe RemoteJoins` to be extracted from a parsed query, this PR also extends the `Execute.RemoteJoin.Collect` module, to implement "collection" from a selection set. Not only do those new functions extract the remote joins, but they also apply all necessary transformations to the selection sets (such as inserting the necessary "phantom" fields used as join keys).
Finally in `Execute.RemoteJoin.Join`, we make two changes. First, we now always look for nested remote joins, regardless of whether the join we just performed went to a source or a remote schema; and second we adapt our join tree logic according to the special cases that were added to deal with remote server edge cases.
Additionally, this PR refactors / cleans / documents `Execute.RemoteJoin.RemoteServer`. This is not required as part of this change and could be moved to a separate PR if needed (a similar cleanup of `Join` is done independently in #3894). It also introduces a draft of a new documentation page for this project, that will be refined in the release PR that ships the feature (either #3069 or a copy of it).
While this PR extends the engine, it doesn't plug such relationships in the schema, meaning that, as of this PR, the new code paths in `Join` are technically unreachable. Adding the corresponding schema code and, ultimately, enabling the metadata API will be done in subsequent PRs.
### Keeping track of concrete type names
The main change this PR makes to the existing `Join` code is to handle a new reserved field we sometimes use when targeting remote servers: the `__hasura_internal_typename` field. In short, a GraphQL selection set can sometimes "branch" based on the concrete "runtime type" of the object on which the selection happens:
```graphql
query {
author(id: 53478) {
... on Writer {
name
articles {
title
}
}
... on Artist {
name
articles {
title
}
}
}
}
```
If both of those `articles` are remote joins, we need to be able, when we get the answer, to differentiate between the two different cases. We do this by asking for `__typename`, to be able to decide if we're in the `Writer` or the `Artist` branch of the query.
To avoid further processing / customization of results, we only insert this `__hasura_internal_typename: __typename` field in the query in the case of unions of interfaces AND if we have the guarantee that we will processing the request as part of the remote joins "folding": that is, if there's any remote join in this branch in the tree. Otherwise, we don't insert the field, and we leave that part of the response untouched.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3810
GitOrigin-RevId: 89aaf16274d68e26ad3730b80c2d2fdc2896b96c
…rmance
It makes sense to try to utilize multiple threads for metadata
operations since we expect them to come one at a time (and likely at
lower load periods anyway).
As noted, although we build roles in parallel now, the admin role is
still a bottleneck. For replace_metadata on huge_schema, on my machine
I get:
BEFORE: 22.7 sec
AFTER: 13.5 sec
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3911
GitOrigin-RevId: 4d4ee6ac8b5506603e70e4fc666a3aacc054d493
### Description
Several libraries define `catMaybes` as `mapMaybe id`. We had it defined in `Data.HashMap.Strict.Extended` already. This small PR also defines it in `Extended` modules for other containers and replaces every occurrence of `mapMaybe id` accordingly.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3884
GitOrigin-RevId: d222a2ca2f4eb9b725b20450a62a626d3886dbf4
In hasura/graphql-engine#5144, we noticed that having remote relationships in the schema is problematic for Relay. In particular, we don't support remote schemas in Relay at all, and because of this, remote relationships were also broken.
The fix was easy: when we're building the schema for Relay, whenever we encounter a remote relationship in our configuration, we simply skip building that field. The implementation was easy: (see hasura/graphql-engine#5145)
```diff
- SFRemoteRelationship info -> pure $ mkRemoteRelationshipFld info
+ SFRemoteRelationship info ->
+ -- https://github.com/hasura/graphql-engine/issues/5144
+ if isRelay then [] else pure $ mkRemoteRelationshipFld info
```
A test case was added in that PR to prevent us from accidentally re-including remote relationships in the Relay schema. (However, it now looks like that test case does not function correctly.)
The above code was later refactored in #3037, making use of the `MaybeT` effect. However, this effect was not used correctly, so that the result of the check was ignored.
This fixes the code to use the `MaybeT` effect correctly.
CC @0x777 @rakeshkky
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3868
GitOrigin-RevId: e528303e01eacf60173cba1eec1898986cf12359
### Description
There were several places in the codebase where we would either implement a generic container, or express the need for one. This PR extracts / creates all relevant containers, and adapts the relevant parts of the code to make use of said new generic containers. More specifically, it introduces the following modules:
- `Data.Set.Extended`, for new functions on `Data.Set`
- `Data.HashMap.Strict.Multi`, for hash maps that accept multiple values
- `Data.HashMap.Strict.NonEmpty`, for hash maps that can never be constructed as empty
- `Data.Trie`, for a generic implementation of a prefix tree
This PR makes use of those new containers in the following parts of the code:
- `Hasura.GraphQL.Execute.RemoteJoin.Types`
- `Hasura.RQL.Types.Endpoint*`
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3828
GitOrigin-RevId: e6c1b971bcb3f5ab66bc91d0fa4d0e9df7a0c6c6
The only purpose was enabling the developer API by default. I don't
think that justifies a flag and CPP usage.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3820
GitOrigin-RevId: 058c9a7b03e5e164ef88e35c42f50bae3c42b5b6
### Description
This PR is one further step towards remote joins from remote schemas. It introduces a custom partial AST to represent queries to remote schemas in the IR: we now need to augment what used to be a straightforward GraphQL AST with additional information for remote join fields.
This PR does the minimal amount of work to adjust the rest of the code accordingly, using `Void` in all places that expect a type representing remote relationships.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3794
GitOrigin-RevId: 33fc317731aace71f82ad158a1951ea93350d6cc
No logic in this PR, just tidying things up (renaming the backend from `Experimental` to `DataWrapper`).
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3779
GitOrigin-RevId: f11acf563ccd8b9f16bc23c5e92da392aa4cfb2c
## Description
This PR is in reference to #2449 (support IP blacklisting for multitenant)
*RFC Update: Add support for IPv6 blocking*
### Solution and Design
Using [http-client-restricted](https://hackage.haskell.org/package/http-client-restricted) package, we're creating the HTTP manager with restricting capabilities. The IPs can be supplied from the CLI arguments as `--ipv4BlocklistCidrs cidr1, cidr2...` or `--disableDefaultIPv4Blocklist` for a default IP list. The new manager will block all requests to the provided CIDRs.
We are extracting the error message string to show the end-user that given IP is blocked from being set as a webhook. There are 2 ways to extract the error message "connection to IP address is blocked". Given below are the responses from event trigger to a blocked IP for these implementations:
- 6d74fde316f61e246c861befcca5059d33972fa7 - We return the error message string as a HTTPErr(HOther) from `Hasura/Eventing/HTTP.hs`.
```
{
"data": {
"message": "blocked connection to private IP address "
},
"version": "2",
"type": "client_error"
}
```
- 88e17456345cbb449a5ecd4877c84c9f319dbc25 - We case match on HTTPExceptionContent for InternaException in `Hasura/HTTP.hs` and extract the error message string from it. (this is implemented as it handles all the cases where pro engine makes webhook requests)
```
{
"data": {
"message": {
"type": "http_exception",
"message": "blocked connection to private IP address ",
"request": {
"secure": false,
"path": "/webhook",
"responseTimeout": "ResponseTimeoutMicro 60000000",
"queryString": "",
"method": "POST",
"requestHeaders": {
"Content-Type": "application/json",
"X-B3-ParentSpanId": "5ae6573edb2a6b36",
"X-B3-TraceId": "29ea7bd6de6ebb8f",
"X-B3-SpanId": "303137d9f1d4f341",
"User-Agent": "hasura-graphql-engine/cerebushttp-ip-blacklist-a793a0e41-dirty"
},
"host": "139.59.90.109",
"port": 8000
}
}
},
"version": "2",
"type": "client_error"
}
```
### Steps to test and verify
The restricted IPs can be used as webhooks in event triggers, and hasura will return an error message in reponse.
### Limitations, known bugs & workarounds
- The `http-client-restricted` has a needlessly complex interface, and puts effort into implementing proxy support which we don't want, so we've inlined a stripped down version.
- Performance constraint: As the blocking is checked for each request, if a long list of blocked CIDRs is supplied, iterating through all of them is not what we would prefer. Using trie is suggested to overcome this. (Added to RFC)
- Calls to Lux endpoints are inconsistent: We use either the http manager from the ProServeCtx which is unrestricted, or the http manager from the ServeCtx which is restricted (the latter through the instances for MonadMetadataApiAuthorization and UserAuthentication). (The failure scenario here would be: cloud sets PRO_ENDPOINT to something that resolves to an internal address, and then restricted requests to those endpoints fail, causing auth to fail on user requests. This is about HTTP requests to lux auth endpoints.)
## Changelog
- ✅ `CHANGELOG.md` is updated with user-facing content relevant to this PR.
## Affected components
- ✅ Server
- ✅ Tests
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3186
Co-authored-by: Robert <132113+robx@users.noreply.github.com>
GitOrigin-RevId: 5bd2de2d028bc416b02c99e996c7bebce56fb1e7
Numbers from CI for the new (currently noisy) `replace_metadata` adhoc benchmark:
chinook: 0.19s -> 0.16
huge_schema: 36.98s -> 29.89
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3685
GitOrigin-RevId: be79b666858b03e8407c0d89765e9aac0af8d40a
It was only used for one purpose. This makes the sketchy manager handling in schema cache init a bit more visible.
Should help make the change in #2449 more robust.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3677
GitOrigin-RevId: e34b990bafb4893663ae195d5bf329130056f1ff
I discovered and removed instances of Boolean Blindness about whether json numbers should be stringified or not.
Although quite far-reaching, this is a completely mechanical change and should have no observable impact outside the server code.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3763
GitOrigin-RevId: c588891afd8a6923a135c736f6581a43a2eddbc7
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: https://github.com/hasura/graphql-engine-mono/pull/3543
Co-authored-by: jkachmar <8461423+jkachmar@users.noreply.github.com>
GitOrigin-RevId: eb79619c95f7a571bce99bc144ce42ee65d08505
In order to respond to GraphQL queries that make use of the introspection fields `__type` or `__schema`, we need two things:
- an overview of the relevant GraphQL type information, stored in a `Schema` object, and
- to have included the `__type` and `__schema` fields in the `query_root` that we generate.
It used to be necessary to do the above items in that order, since the `__type` and `__schema` fields (i.e. the respective `FieldParser`s) were generated _from_ a `Schema` object.
Thanks to recent refactorings in `Hasura.GraphQL.Schema.Introspect` (see hasura/graphql-engine-mono#2835 or hasura/graphql-engine@5760d9289c), the introspection fields _themselves_ are now `Schema`-agnostic, and simply return a function that takes a `Schema` object after parsing. For instance, the type of `schema`, corresponding to the `__schema` field, has literally changed as follows:
```diff
-schema :: MonadParse n => Schema -> FieldParser n ( J.Value)
+schema :: MonadParse n => FieldParser n (Schema -> J.Value)
```
This means that the introspection fields can be included in the GraphQL schema *before* we have generated a `Schema` object. In particular, rather than the current architecture of generating `Schema` at startup time for every role, we can instead generate `Schema` ad-hoc at query parsing time, only for those queries that make use of the introspection fields. This avoids us storing a `Schema` for every role for the lifetime of the server.
However: this introduces a functional change, as the code that generates the `Schema` object, and in particular the `accumulateTypeDefinitions` method, also does certain correctness checks, to prevent exposing a spec-incompliant GraphQL schema. If these correctness checks are being done at parsing time rather than startup time, then we catch certain errors only later on. For this reason, this PR adds an explicit run of this type accumulation at startup time. For efficiency reasons, and since this correctness check is not essential for correct operation of HGE, this is done for the admin role only.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3231
GitOrigin-RevId: 23701c548b785929b28667025436b6ce60bfe1cd
There are three minor cleanups here:
- The first argument to the `setMetadataInCatalog` method is always `Just`. It is thus important to avoid `Maybe`, because this means that a crucial piece of code (saving metadata) is completely untested.
- Rather than spelling them out, we can derive the `Semigroup`/`Monoid` instances for `MetadataModifier` through the `Endo` type.
- I've renamed the name of the getter of the `MetadataModifier` newtype to **r**unMetadataModifier. Using record puns, this allows us to write:
```diff
- putMetadata $ unMetadataModifier metadataModifier metadata
+ putMetadata $ runMetadataModifier metadata
```
which is nicer to read.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3703
GitOrigin-RevId: fd36b3c5202017f5afc943c01dfdd7c82c099bdd
#### TODO
- [x] fix `hashable >= 1.3.1` serialization ordering issue [^1]
- `test_graphql_mutations.py::TestGraphQLMutateEnums` was failing
- [x] fix `unordered-containers` serialization ordering issue [^2]
- `test_graphql_queries.py` was failing on Citus
- [ ] verify that no new failures have been introduced
- [ ] open issues to fix the above
- identify test cases that "leak" implementation details by depending on `hashable` instance ordering
- bump `hashable >= 1.3.1` and update test cases with new ordering OR modify them so that ordering is stable
- bump `unordered-containers >= 0.2.15.0` and update test cases with new ordering OR modify them so that ordering is stable
- one of the test cases was failing on string equality comparison for a generated Citus query
- we probably don't want to _actually_ do this unless there are _very specific_ guarantees we want to make about generated query structure
---
Just what it says on the tin.
https://github.com/hasura/graphql-engine-mono/pull/3538 updated the freeze file a few weeks ago, but it looks like the index state hadn't been updated since December so a lot of stuff that had newer versions didn't get updated.
---
EDIT: I should add, the motivation for doing this in the first place is that `hspec > 2.8.4` now supports specifying filtering spec trees based on patterns provided by the `HSPEC_MATCH` environment variable.
For example, one could have a script that executes the following:
```
HSPEC_MATCH="PostgreSQL" \
ghcid \
--command \
'cabal repl graphql-engine:test:tests-hspec \
--repl-option -O0 \
--repl-option -fobject-code' \
--test "main"
```
...which will loop on typechecking the `tests-hspec` component, and then as soon as it passes (i.e. no warnings or errors) will run _only_ the `PostgreSQL` sub-components.
[^1]: `hashable >= 1.3.1.0` [updated its default salts](https://github.com/haskell-unordered-containers/hashable/pull/196), which [broke serialization ordering](https://github.com/haskell/aeson/issues/837)
[^2]: `unordered-containers >= 0.2.16.0` [introduced changes to some of its internal functions](https://hackage.haskell.org/package/unordered-containers-0.2.16.0/changelog) which seem like they could have affected serialization stability
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3672
GitOrigin-RevId: bbd1d48c73db4021913f0b5345b7315a8d6525d3
We build the GraphQL schema by combining building blocks such as `tableSelectionSet` and `columnParser`. These building blocks individually build `{InputFields,Field,}Parser` objects. Those object specify the valid GraphQL schema.
Since the GraphQL schema is role-dependent, at some point we need to know what fragment of the GraphQL schema a specific role is allowed to access, and this is stored in `{Sel,Upd,Ins,Del}PermInfo` objects.
We have passed around these permission objects as function arguments to the schema building blocks since we first started dealing with permissions during the PDV refactor - see hasura/graphql-engine@5168b99e46 in hasura/graphql-engine#4111. This means that, for instance, `tableSelectionSet` has as its type:
```haskell
tableSelectionSet ::
forall b r m n.
MonadBuildSchema b r m n =>
SourceName ->
TableInfo b ->
SelPermInfo b ->
m (Parser 'Output n (AnnotatedFields b))
```
There are three reasons to change this.
1. We often pass a `Maybe (xPermInfo b)` instead of a proper `xPermInfo b`, and it's not clear what the intended semantics of this is. Some potential improvements on the data types involved are discussed in issue hasura/graphql-engine-mono#3125.
2. In most cases we also already pass a `TableInfo b`, and together with the `MonadRole` that is usually also in scope, this means that we could look up the required permissions regardless: so passing the permissions explicitly undermines the "single source of truth" principle. Breaking this principle also makes the code more difficult to read.
3. We are working towards role-based parsers (see hasura/graphql-engine-mono#2711), where the `{InputFields,Field,}Parser` objects are constructed in a role-invariant way, so that we have a single object that can be used for all roles. In particular, this means that the schema building blocks _need_ to be constructed in a role-invariant way. While this PR doesn't accomplish that, it does reduce the amount of role-specific arguments being passed, thus fixing hasura/graphql-engine-mono#3068.
Concretely, this PR simply drops the `xPermInfo b` argument from almost all schema building blocks. Instead these objects are looked up from the `TableInfo b` as-needed. The resulting code is considerably simpler and shorter.
One way to interpret this change is as follows. Before this PR, we figured out permissions at the top-level in `Hasura.GraphQL.Schema`, passing down the obtained `xPermInfo` objects as required. After this PR, we have a bottom-up approach where the schema building blocks themselves decide whether they want to be included for a particular role.
So this moves some permission logic out of `Hasura.GraphQL.Schema`, which is very complex.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3608
GitOrigin-RevId: 51a744f34ec7d57bc8077667ae7f9cb9c4f6c962
- consistent qualified imports
- less convoluted initialization of pro logging HTTP manager
- pass pro HTTP manager directly instead of via Has
- remove some dead healthcheck code
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3639
GitOrigin-RevId: dfa7b9c62d1842a07a8514cdb77f1ed86064fb06
spec: https://github.com/hasura/graphql-engine-mono/pull/2278
Briefly:
- extend metadata so that allowlist entries get a new scope field
- update `add_collection_to_allowlist` to accept this new scope field,
and adds `update_scope_of_collection_in_allowlist` to change the scope
- scope can be global or role-based; a collection is available for every
role if it is global, and available to every listed role if it is role-based
- graphql-engine-oss is aware of role-based allowlist metadata; collections
with non-global scope are treated as if they weren't in the allowlist
To run the tests:
- `cabal run graphql-engine-tests -- unit --match Allowlist`
- py-tests against pro:
- launch `graphql-engine-pro` with `HASURA_GRAPHQL_ADMIN_SECRET` and `HASURA_GRAPHQL_ENABLE_ALLOWLIST`
- `pytest test_allowlist_queries.py --hge-urls=... --pg-urls=... --hge-key=... --test-allowlist-queries --pro-tests`
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/2477
Co-authored-by: Anon Ray <616387+ecthiender@users.noreply.github.com>
Co-authored-by: Robert <132113+robx@users.noreply.github.com>
GitOrigin-RevId: 01f8026fbe59d8701e2de30986511a452fce1a99
## Description
This PR is a subset of #3069, that does roughly that #3031 was aiming to do: add the schema cache building phase for relationships from remote servers. This PR does not change any of the code that *uses* remote relationships, meaning we ignore the added schema cache information. It also contains dependency-tracking code, which was originally missing from #3031; in turn, this pulls some of the metadata API as well, since we identify remote relationships by how they were created.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3540
GitOrigin-RevId: ed962b6d07fd4adbf0a71e0d79736a4e8b422fea
## Description
This PR updates the notes-extracting script, to allow for a stable sort across runs:
- files are now treated in alphabetical order of the full file name:
- within a file, notes are treated in order
- that order is reflected in the generated index file
- within a note, references are sorted by file, then by line number
Additionally it fixes(?) a note whose format was unexpected.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3352
GitOrigin-RevId: 2b2b57ec0aa2657d75a88e4951e6b19bb2d665e3
This change is because in https://github.com/hasura/graphql-engine-mono/pull/2477, errors come out as
```
multiple declarations exist for the following allowlist : [CollectionName {unCollectionName = NonEmptyText {unNonEmptyText = "collection_1"}}]'
```
which is awfully messy. This changes it to
```
multiple declarations exist for the following allowlist: collection_1
```
This should improve error messages across the board -- e.g. `RoleName` has a nice `ToTxt` instance, while we used to use a derived `Show` instances. However, there might just be instances where the `Show` instance was better, bit hard to be sure based on scanning the code since we don't have test coverage for these errors.
Broken out of the allowlist PR since it affects more than just the allowlist.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3470
GitOrigin-RevId: 7a2c29f17d9f15d840cb2f89caefcdd3aae44d25
_(This PR is on top of #3352.)_
## Description
This PR overhauls our documentation CI steps to push all generated server documentation to the `gh-pages` branch of the OSS repo. The goal of this PR is to arrive in the situation where `https://hasura.github.io/graphql-engine/server/` is automatically populated to contain the following:
- all the markdown files from `server/documentation`, copied verbatim, no transformation applied
- all the notes, collected from the code by the `extract-notes.sh` script, in `server/notes`
- the generated haddock documentation for each major release or branch in `server/haddock`.
To do so, this PR does the following:
- it includes the script to extract notes from #3352,
- it rewrites the documentation checking CI step, to generate the notes and publish the resulting "server/documentation" folder,
- it includes a new CI step to deploy the documentation to the `gh-pages` branch
Of note:
- we will generate a different haddock folder for each main branch and release; in practice, that means the _main_, _stable_, _alpha_, _beta_ branches, and every build tagged with a version number
- the step that builds the haddock documentation checks that ALL projects in the repo build, including pro, but the deploy only deploys the graphql-engine documentation, as it pushes it to a publicly-accessible place
## Required work
**DO NOT MERGE THIS PR IT IS NOT READY**. Some work needs to go into this PR before it is ready.
First of all: the `gh-pages` branch of the OSS repo does NOT yet contain the documentation scaffolding that this new process assumes. At the bare minimum, it should be a orphan branch that contains a top-level README.md file, and a _server_ folder. An example of the bare minimum required can be previewed [on my fork](https://nicuveo.github.io/graphql-engine/server/).
The content of the `server/documentation` folder needs to be adjusted to reflect this; at the very least, a `README.md` file needs to be added to do the indexing (again, see the placeholder [on my fork](https://nicuveo.github.io/graphql-engine/server/) for an example).
This way of publishing documentation must be validated against [proposed changes to the documentation](https://github.com/hasura/graphql-engine-mono/pull/3294). @marionschleifer what do you think?
~~The buildkite code in this branch is currently untested, and I am not sure how to test it.~~
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3380
GitOrigin-RevId: b24f6759c64ae29886c1f1b481b172febc512032
### Description
The GraphQL spec has to conflicting requirements:
1. an object must contain at least one field: the schema may not contain empty objects
2. the _query_root_ must always be present
Given _1_, the schema generation code removes from the schema all fields that would result in empty objects, such as a table for which a user does not have select permissions. But, as a result, our code also potentially removes _query_root_ if it is empty, breaking _2_.
This PR introduces a dummy "placeholder" field in the query root if it's empty, to ensure we never remove it from the schema.
### Remaining work
- [x] changelog entry
- [x] tests
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/148
GitOrigin-RevId: bfd6bfcc2f3de92900b6ba566012f093399ca037
### Description
This PR is the result of a discussion in #3363. Namely, we would like to remove all uses of `unsafeMkName`, or at the very least document every single one of them, to avoid similar issues. To do so, this PR does the following:
- it adds a hlint suggestion not to use that function:
- suggestions don't mark the PR as failed, but will be shown at review time
- it is possible to disable that hint with `{- HLINT ignore myFunction "unsafe" -}`
- wherever possible, it removes uses of `unsafeMkName` in favour of `mkName`
- it adds a comment with a tracking issue for the two remaining uses:
- #3478
- #3479
### Remaining work
- discuss whether this hint should make the linter step fail, since the linter step isn't required to merge anyway, and there is a way to disable the hint wherever we think the use of that function is acceptable
- check that none of those uses were load-bearing and result in errors now
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3480
GitOrigin-RevId: 0a7e3e9d1a48185764c04ab61e34b58273af347c
## Description
When setting up a remote relationship to a remote schema, values coming from the left-hand side are given as _arguments_ to the targeted field of the remote schema. In turn, that means we need to adjust the arguments to that remote field; in the case of input objects, it means creating a brand new input object in which the relevant fields have been removed.
To both avoid conflicts, and be explicit, we give a pretty verbose name to such an input object: its original name, followed by "remote_rel", followed by the full name of the field (table name + relationship name). The bug there was introduced when working on extending remote relationships to other backends: we changed the code that translates the table name to a graphql identifier to be generic, and use the table's `ToTxt` instance instead. However, when a table is not in the default schema, the character used by that instance is `.`, which is not a valid GraphQL name.
This PR fixes it, by doing two things:
- it defines a safe function to translate LHS identifiers to graphql names (by replacing all invalid characters by `_`)
- it doesn't use `unsafeMkName` anymore, and checks at validation time that the type name is correct
## Further work
On this PR:
- [x] add a test
- [x] write a Changelog entry
Beyond this PR, we might want to:
- prioritize #1747
- analyze all calls to `unsafeMkName` and remove as many as possible
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3363
GitOrigin-RevId: fe98eb1d34157b2c8323af453f5c369de616af38
This commit introduces an "experimental" backend adapter to the GraphQL Engine.
It defines a high-level interface which will eventually be used as the basis for implementing separate data source query generation & marshaling services that communicate with the GraphQL Engine Server via some protocol.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/2684
Co-authored-by: awjchen <13142944+awjchen@users.noreply.github.com>
Co-authored-by: Chris Parks <592078+cdparks@users.noreply.github.com>
GitOrigin-RevId: 4463b682142ad6e069e223b88b14db511f634768
This PR pretty much does the same thing to remote relationship types in schemacache as what #2979 did to remote relationship types in the IR. On main remote relationships are represented by types of form `T from to`. This PR changes it to `T from` which makes it a lot more reusable.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3037
GitOrigin-RevId: 90a5c9e2346c8dc2da6ec5b8c970d6c863d2afb8
this pr modifies the representation chosen for introspection parsers, "pushing down" the `Schema` input so it is not required to build the parser anymore. instead, the value produced when the parser is evaluated becomes a function that consumes a schema:
```diff
-schema :: MonadParse n => Schema -> FieldParser n ( J.Value)
+schema :: MonadParse n => FieldParser n (Schema -> J.Value)
```
this addresses points (1) and (2) of #2833 and is intended to make #2799 easier: we will need to enforce permissions when generating introspection objects, hiding fields the user is not allowed to see, so if we can pass the schema _later_, we can build this parser once, evaluate it once to (morally) obtain a function `Schema -> Value`, and simply run that single `Schema -> Value` function on different role-based schemas.
(we really need some terminology to be fixed here: "parser" is already not the best name, and then we have parser vs value/function "returned" by parser vs...)
however, we have immediate benefits: we no longer _need_ a `Schema` object to build the introspection parsers! this means we can remove the bogus "degenerate case" schema that is currently constructed in `emptyIntrospection` (and indeed we remove that binding altogether).
(fun fact: the diff for this pull request has a negative line count despite adding a lot of comments. @abooij says i have bragging rights in perpetuity now, à la @nicuveo)
changes:
- internal changes to the operation of the server, invisible outside of a small number of `GraphQL.Schema.*` modules
- no user-facing changes
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/2835
Co-authored-by: Auke Booij <164426+abooij@users.noreply.github.com>
Co-authored-by: Brandon Simmons <210815+jberryman@users.noreply.github.com>
GitOrigin-RevId: 9990f53b8f5c733424c4d71a24d94c13dee842ba
## Remaining Work
- [x] changelog entry
- [x] more tests: `<backend>_delete_remote_relationship` is definitely untested
- [x] negative tests: we probably want to assert that there are some APIs we DON'T support
- [x] update the console to use the new API, if necessary
- [x] ~~adding the corresponding documentation for the API for other backends (only `pg_` was added here)~~
- deferred to https://github.com/hasura/graphql-engine-mono/issues/3170
- [x] ~~deciding which backends should support this API~~
- deferred to https://github.com/hasura/graphql-engine-mono/issues/3170
- [x] ~~deciding what to do about potentially overlapping schematic representations~~
- ~~cf. https://github.com/hasura/graphql-engine-mono/pull/3157#issuecomment-995307624~~
- deferred to https://github.com/hasura/graphql-engine-mono/issues/3171
- [x] ~~add more descriptive versioning information to some of the types that are changing in this PR~~
- cf. https://github.com/hasura/graphql-engine-mono/pull/3157#discussion_r769830920
- deferred to https://github.com/hasura/graphql-engine-mono/issues/3172
## Description
This PR fixes several important issues wrt. the remote relationship API.
- it fixes a regression introduced by [#3124](https://github.com/hasura/graphql-engine-mono/pull/3124), which prevented `<backend>_create_remote_relationship` from accepting the old argument format (break of backwards compatibility, broke the console)
- it removes the command `create_remote_relationship` added to the v1/metadata API as a work-around as part of [#3124](https://github.com/hasura/graphql-engine-mono/pull/3124)
- it reverts the subsequent fix in the console: [#3149](https://github.com/hasura/graphql-engine-mono/pull/3149)
Furthermore, this PR also addresses two other issues:
- THE DOCUMENTATION OF THE METADATA API WAS WRONG, and documented `create_remote_relationship` instead of `<backend>_create_remote_relationship`: this PR fixes this by adding `pg_` everywhere, but does not attempt to add the corresponding documentation for other backends, partly because:
- `<backend>_delete_remote_relationship` WAS BROKEN ON NON-POSTGRES BACKENDS; it always expected an argument parameterized by Postgres.
As of main, the `<backend>_(create|update|delete)_remote_relationship` commands are supported on Postgres, Citus, BigQuery, but **NOT MSSQL**. I do not know if this is intentional or not, if it even should be publicized or not, and as a result this PR doesn't change this.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3157
Co-authored-by: jkachmar <8461423+jkachmar@users.noreply.github.com>
GitOrigin-RevId: 37e2f41522a9229a11c595574c3f4984317d652a
## Description
This PR fixes two issues:
- in [#2903](https://github.com/hasura/graphql-engine-mono/pull/2903), we introduced a new metadata representation of remote relationships, which broke parsing a metadata blob containing an old-style db-to-rs remote relationship
- in [#1179](https://github.com/hasura/graphql-engine-mono/pull/1179), we silently and mistakenly deprecated `create_remote_relationship` in favour of `<backend>_create_remote_relationship`
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3124
Co-authored-by: jkachmar <8461423+jkachmar@users.noreply.github.com>
Co-authored-by: Antoine Leblanc <1618949+nicuveo@users.noreply.github.com>
GitOrigin-RevId: 45481db7a8d42c7612e938707cd2d652c4c81bf8
This PR simplifies the types that represent a remote relationship in IR so that they can be reused in other parts (in remote schema types) which could have remote relationships.
The comments on the PR explain the main changes.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/2979
GitOrigin-RevId: 559c51d9d6ae79e2183ce4347018741b9096ac74
GraphQL types can refer to each other in a circular way. The PDV framework used to use values of type `Unique` to recognize two fragments of GraphQL schema as being the same instance. Internally, this is based on `Data.Unique` from the `base` package, which simply increases a counter on every creation of a `Unique` object.
**NB**: The `Unique` values are _not_ used for knot tying the schema combinators themselves (i.e. `Parser`s). The knot tying for `Parser`s is purely based on keys provided to `memoizeOn`. The `Unique` values are _only_ used to recognize two pieces of GraphQL _schema_ as being identical. Originally, the idea was that this would help us with a perfectly correct identification of GraphQL types. But this fully correct equality checking of GraphQL types was never implemented, and does not seem to be necessary to prevent bugs.
Specifically, these `Unique` values are stored as part of `data Definition a`, which specifies a part of our internal abstract syntax tree for the GraphQL types that we expose. The `Unique` values get initialized by the `SchemaT` effect.
In #2894 and #2895, we are experimenting with how (parts of) the GraphQL types can be hidden behind certain permission predicates. This would allow a single GraphQL schema in memory to serve all roles, implementing #2711. The permission predicates get evaluated at query parsing time when we know what role is doing a certain request, thus outputting the correct GraphQL types for that role.
If the approach of #2895 is followed, then the `Definition` objects, and thus the `Unique` values, would be hidden behind the permission predicates. Since the permission predicates are evaluated only after the schema is already supposed to be built, this means that the permission predicates would prevent us from initializing the `Unique` values, rendering them useless.
The simplest remedy to this is to remove our usage of `Unique` altogether from the GraphQL schema and schema combinators. It doesn't serve a functional purpose, doesn't prevent bugs, and requires extra bookkeeping.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/2980
GitOrigin-RevId: 50d3f9e0b9fbf578ac49c8fc773ba64a94b1f43d
### Description
This PR changes the internal representation of a parsed remote schema. We were still using a list of type definitions, meaning every time we were doing a type lookup we had to iterate through a linked list! 🙀 It was very noticeable on large schemas, that need to do a lot of lookups. This PR consequently changes the internal representation to a HashMap. Building the OneGraph schema on my machine now takes **23 seconds**, compared to **367 seconds** before this patch.
Some important points:
- ~~this PR removes a check for type duplication in remote schemas; it's unclear to me whether that's something we need to add back or not~~ (no longer true)
- this PR makes it obvious that we do not distinguish between "this remote schema is missing type X" and "this remote schema expects type X to be an object, but it's a scalar"; this PR doesn't change anything about it, but adds a comment where we could surface that error (see [2991](https://github.com/hasura/graphql-engine-mono/issues/2991))
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/2963
GitOrigin-RevId: f5c96ad40f4e0afcf8cef635b4d64178111f98d3
Source typename customization (hasura/graphql-engine@aac64f2c81) introduced a mechanism to change certain names in the GraphQL schema that is exposed. In particular it allows last-minute modification of:
1. the names of some types, and
2. the names of some root fields.
The above two items are assigned distinct customization algorithms, and at times both algorithms are in scope. So a need to distinguish them is needed.
In the original design, this was addressed by introducing a newtype wrapper `Typename` around GraphQL `Name`s, dedicated to the names of types. However, in the majority of the codebase, type names are also represented by `Name`. For this reason, it was unavoidable to allow for easy conversion. This was supported by a `HasName Typename` instance, as well as by publishing the constructors of `Typename`.
This means that the type safety that newtypes can add is lost. In particular, it is now very easy to confuse type name customization with root field name customization.
This refactors the above design by instead introducing newtypes around the customization operations:
```haskell
newtype MkTypename = MkTypename {runMkTypename :: Name -> Name}
deriving (Semigroup, Monoid) via (Endo Name)
newtype MkRootFieldName = MkRootFieldName {runMkRootFieldName :: Name -> Name}
deriving (Semigroup, Monoid) via (Endo Name)
```
The `Monoid` instance allows easy composition of customization operations, piggybacking off of the type of `Endo`maps.
This design allows safe co-existence of the two customization algorithms, while avoiding the syntactic overhead of packing and unpacking newtypes.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/2989
GitOrigin-RevId: da3a353a9b003ee40c8d0a1e02872e99d2edd3ca
This is effectively a no-op, the `Left err` case can't actually happen.
- removes some unused logic
- refactors the /healthz endpoint to be clearer
- that includes logging the full QErr if checkMetadataHealth fails,
but it actually can't because the existing Postgres implementation
just lifts
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/2849
GitOrigin-RevId: ac8abf51b6d869ad4048419e36012137c86e5abd
>
High-Level TODO:
* [x] Code Changes
* [x] Tests
* [x] Check that pro/multitenant build ok
* [x] Documentation Changes
* [x] Updating this PR with full details
* [ ] Reviews
* [ ] Ensure code has all FIXMEs and TODOs addressed
* [x] Ensure no files are checked in mistakenly
* [x] Consider impact on console, cli, etc.
### Description
>
This PR adds support for adding set-cookie header on the response from the auth webhook. If the set-cookie header is sent by the webhook, it will be forwarded in the graphQL engine response.
Fixes a bug in test-server.sh: testing of get-webhook tests was done by POST method and vice versa. To fix, the parameters were swapped.
### Changelog
- [x] `CHANGELOG.md` is updated with user-facing content relevant to this PR.
### Affected components
- [x] Server
- [ ] Console
- [ ] CLI
- [x] Docs
- [ ] Community Content
- [ ] Build System
- [x] Tests
- [ ] Other (list it)
### Related Issues
->
Closes [#2269](https://github.com/hasura/graphql-engine/issues/2269)
### Solution and Design
>
### Steps to test and verify
>
Please refer to the docs to see how to send the set-cookie header from webhook.
### Limitations, known bugs & workarounds
>
- Support for only set-cookie header forwarding is added
- the value forwarded in the set-cookie header cannot be validated completely, the [Cookie](https://hackage.haskell.org/package/cookie) package has been used to parse the header value and any unnecessary information is stripped off before forwarding the header. The standard given in [RFC6265](https://datatracker.ietf.org/doc/html/rfc6265) has been followed for the Set-Cookie format.
### Server checklist
#### Catalog upgrade
Does this PR change Hasura Catalog version?
- [x] No
- [ ] Yes
- [ ] Updated docs with SQL for downgrading the catalog
#### Metadata
Does this PR add a new Metadata feature?
- [x] No
#### GraphQL
- [x] No new GraphQL schema is generated
- [ ] New GraphQL schema is being generated:
- [ ] New types and typenames are correlated
#### Breaking changes
- [x] No Breaking changes
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/2538
Co-authored-by: Robert <132113+robx@users.noreply.github.com>
GitOrigin-RevId: d9047e997dd221b7ce4fef51911c3694037e7c3f
We'll see if this improves compile times at all, but I think it's worth
doing as at least the most minimal form of module documentation.
This was accomplished by first compiling everything with
-ddump-minimal-imports, and then a bunch of scripting (with help from
ormolu)
**EDIT** it doesn't seem to improve CI compile times but the noise floor is high as it looks like we're not caching library dependencies anymore
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/2730
GitOrigin-RevId: 667eb8de1e0f1af70420cbec90402922b8b84cb4
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
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
>
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
## 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
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
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
>
### 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
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
>
### 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
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
## Description
This PR removes as many constraints as possible from Backend without refactoring the code. Thanks to #1844, a few ToJSON functions can be removed from the IR, and several constraints were simply redundant.
This PR borrows from similar work done as part of #1666.
## Note
To remove constraints more aggressively, I have explored the possibility of _removing Representable altogether_, in a [separate commit](https://github.com/hasura/graphql-engine-mono/compare/nicuveo/remove-extension-constraints..nicuveo/tentative-remove-representable). I am not convinced it's a good idea in terms of readability of the code, but it's a possibility.
Further work includes deciding what we want to do with `Show` and `ToTxt`; see #1747.
https://github.com/hasura/graphql-engine-mono/pull/1843
GitOrigin-RevId: 337324ad90cb8f86f06e1c5a36aa44bb414e195a
Query plan caching was introduced by - I believe - hasura/graphql-engine#1934 in order to reduce the query response latency. During the development of PDV in hasura/graphql-engine#4111, it was found out that the new architecture (for which query plan caching wasn't implemented) performed comparably to the pre-PDV architecture with caching. Hence, it was decided to leave query plan caching until some day in the future when it was deemed necessary.
Well, we're in the future now, and there still isn't a convincing argument for query plan caching. So the time has come to remove some references to query plan caching from the codebase. For the most part, any code being removed would probably not be very well suited to the post-PDV architecture of query execution, so arguably not much is lost.
Apart from simplifying the code, this PR will contribute towards making the GraphQL schema generation more modular, testable, and easier to profile. I'd like to eventually work towards a situation in which it's easy to generate a GraphQL schema parser *in isolation*, without being connected to a database, and then parse a GraphQL query *in isolation*, without even listening any HTTP port. It is important that both of these operations can be examined in detail, and in isolation, since they are two major performance bottlenecks, as well as phases where many important upcoming features hook into.
Implementation
The following have been removed:
- The entirety of `server/src-lib/Hasura/GraphQL/Execute/Plan.hs`
- The core phases of query parsing and execution no longer have any references to query plan caching. Note that this is not to be confused with query *response* caching, which is not affected by this PR. This includes removal of the types:
- - `Opaque`, which is replaced by a tuple. Note that the old implementation was broken and did not adequately hide the constructors.
- - `QueryReusability` (and the `markNotReusable` method). Notably, the implementation of the `ParseT` monad now consists of two, rather than three, monad transformers.
- Cache-related tests (in `server/src-test/Hasura/CacheBoundedSpec.hs`) have been removed .
- References to query plan caching in the documentation.
- The `planCacheOptions` in the `TenantConfig` type class was removed. However, during parsing, unrecognized fields in the YAML config get ignored, so this does not cause a breaking change. (Confirmed manually, as well as in consultation with @sordina.)
- The metrics no longer send cache hit/miss messages.
There are a few places in which one can still find references to query plan caching:
- We still accept the `--query-plan-cache-size` command-line option for backwards compatibility. The `HASURA_QUERY_PLAN_CACHE_SIZE` environment variable is not read.
https://github.com/hasura/graphql-engine-mono/pull/1815
GitOrigin-RevId: 17d92b254ec093c62a7dfeec478658ede0813eb7
## Description
Thanks to #1664, the Metadata API types no longer require a `ToJSON` instance. This PR follows up with a cleanup of the types of the arguments to the metadata API:
- whenever possible, it moves those argument types to where they're used (RQL.DDL.*)
- it removes all unrequired instances (mostly `ToJSON`)
This PR does not attempt to do it for _all_ such argument types. For some of the metadata operations, the type used to describe the argument to the API and used to represent the value in the metadata are one and the same (like for `CreateEndpoint`). Sometimes, the two types are intertwined in complex ways (`RemoteRelationship` and `RemoteRelationshipDef`). In the spirit of only doing uncontroversial cleaning work, this PR only moves types that are not used outside of RQL.DDL.
Furthermore, this is a small step towards separating the different types all jumbled together in RQL.Types.
## Notes
This PR also improves several `FromJSON` instances to make use of `withObject`, and to use a human readable string instead of a type name in error messages whenever possible. For instance:
- before: `expected Object for Object, but encountered X`
after: `expected Object for add computed field, but encountered X`
- before: `Expecting an object for update query`
after: `expected Object for update query, but encountered X`
This PR also renames `CreateFunctionPermission` to `FunctionPermissionArgument`, to remove the quite surprising `type DropFunctionPermission = CreateFunctionPermission`.
This PR also deletes some dead code, mostly in RQL.DML.
This PR also moves a PG-specific source resolving function from DDL.Schema.Source to the only place where it is used: App.hs.
https://github.com/hasura/graphql-engine-mono/pull/1844
GitOrigin-RevId: a594521194bb7fe6a111b02a9e099896f9fed59c
GJ IR changes cherry-picked from the original GJ branch. There is a separate (can be merged independently) PR for metadata changes (#1727) and there will be a different PR upcoming PR for execution changes.
https://github.com/hasura/graphql-engine-mono/pull/1810
Co-authored-by: Vamshi Surabhi <6562944+0x777@users.noreply.github.com>
GitOrigin-RevId: c31956af29dc9c9b75d002aba7d93c230697c5f4
## Description
This PR fixes an oversight in the implementation of the resolvers of different backends. To implement resolution from environment variables, both MSSQL and BigQuery were directly fetching the process' environment variables, instead of using the careful curated set we thread from main. It was working just fine on OSS, but is failing on Cloud.
This PR fixes this by adding an additional argument to `resolveSourceConfig`, to ensure that backends always use the correct set of variables.
https://github.com/hasura/graphql-engine-mono/pull/1891
GitOrigin-RevId: 58644cab7d041a8bf4235e2acfe9cf71533a92a1
### Description
This PR is the first of several PRs meant to introduce Generalized Joins. In this first PR, we add non-breaking changes to the Metadata types for DB-to-DB remote joins. Note that we are currently rejecting the new remote join format in order to keep folks from breaking their metadata (in case of a downgrade). These issues will be tackled (and JSON changes reverted) in subsequent PRs.
This PR also changes the way we construct the schema cache, and breaks the way we process sources in two steps: we first resolve each source and construct a cache of their tables' raw info, then in a second step we build the source output. This is so that we have access to the target source's tables when building db-to-db relationships.
### Notes
- this PR contains a few minor cleanups of the schema
- it also fixes a bug in how we do renames in remote schema relationships
- it introduces cross-source schema dependencies
https://github.com/hasura/graphql-engine-mono/pull/1727
Co-authored-by: Evie Ciobanu <1017953+eviefp@users.noreply.github.com>
GitOrigin-RevId: f625473077bc5fff5d941b70e9a116192bc1eb22
In the same vein as #1762, but in less critical: we have some old instances that were made manual because of AnyBackend but that don't need to be anymore.
https://github.com/hasura/graphql-engine-mono/pull/1866
GitOrigin-RevId: f1515330859e70531139f8edb21bd016441f8e8d
During the "generalization" of the code, we removed the [generated instances](9d63187803 (diff-7b8382ab20e441fa214586be491eb6f7ca904d8b20ed97894c16a339be45219aL72)) of `Eq` and `Hashable` in favour of manually written ones, because for a time we were struggling with `AnyBackend`.
In the process, we lost one constructor, `MOEndpoint`. It's unlikely to have ever been an issue, since duplicate endpoints are unlikely, but it is nonetheless wrong.
This PR simply goes back to default derived instances, now that we can, fixing the problem and making the code simpler.
---
However, after filing this PR, I realized **it broke rest endpoint tests**. Upon closer inspection, it turns out that while we had proper checks in place, because of this bug we were actually failing _differently_ during metadata checks. This PR actually improves error messages for invalid endpoint configurations?! 🙀
This is a tiny bit scary. ^^'
https://github.com/hasura/graphql-engine-mono/pull/1762
GitOrigin-RevId: c4897d1f3ae92d18c44c87d2f58258c66f716686
The following has been tested locally:
1. [x] Uses the default query limit when `global_select_limit` is not present in the `metadata` configuration
2. [x] Uses the provided number when present in the configuration (tested with `1`)
3. [x] Throws an error if the provided value is not a parseable non-negative number (because bigquery throws an error when limits are negative)
https://github.com/hasura/graphql-engine-mono/pull/1816
Co-authored-by: Abby Sassel <3883855+sassela@users.noreply.github.com>
GitOrigin-RevId: e21157ce9ca8f8130b3b01e91ed048e79f376293
### Description
In our haste to generalize everything for MSSQL, we put every single "suspicious" type in Backend, including ones that weren't required. `Alias` is one of those: it's only used in a type alias, and is actually just an implementation detail of the translation layer. This PR removes it.
https://github.com/hasura/graphql-engine-mono/pull/1759
GitOrigin-RevId: fb348934ec65a51aae7f95d93c83c3bb704587b5
### Description
This PR removes all `fmapX` and `traverseX` functions from RQL.IR, favouring instead `Functor` and `Traversable` instances throughout the code. This was a relatively straightforward change, except for two small pain points: `AnnSelectG` and `AnnInsert`. Both were parametric over two types `a` and `v`, making it impossible to make them traversable functors... But it turns out that in every single use case, `a ~ f v`. By changing those types to take such an `f :: Type -> Type` as an argument instead of `a :: Type` makes it possible to make them functors.
The only small difference is for `AnnIns`, I had to introduce one `Identity` transformation for one of the `f` parameters. This is relatively straightforward.
### Notes
This PR fixes the most verbose BigQuery hint (`let` instead of `<- pure`).
https://github.com/hasura/graphql-engine-mono/pull/1668
GitOrigin-RevId: e632263a8c559aa04aeae10dcaec915b4a81ad1a
Blocked on https://github.com/hasura/graphql-engine-mono/pull/1640.
While fiddling with BigQuery I noticed a severe issue with offset/limit for array-aggregates. I've fixed it now.
The basic problem was that I was using a query like this:
```graphql
query MyQuery {
hasura_Artist(order_by: {artist_self_id: asc}) {
artist_self_id
albums_aggregate(order_by: {album_self_id: asc}, limit: 2) {
nodes {
album_self_id
}
aggregate {
count
}
}
}
}
```
Producing this SQL:
```sql
SELECT `t_Artist1`.`artist_self_id` AS `artist_self_id`,
STRUCT(IFNULL(`aa_albums1`.`nodes`, NULL) AS `nodes`, IFNULL(`aa_albums1`.`aggregate`, STRUCT(0 AS `count`)) AS `aggregate`) AS `albums_aggregate`
FROM `hasura`.`Artist` AS `t_Artist1`
LEFT OUTER JOIN (SELECT ARRAY_AGG(STRUCT(`t_Album1`.`album_self_id` AS `album_self_id`) ORDER BY (`t_Album1`.`album_self_id`) ASC) AS `nodes`,
STRUCT(COUNT(*) AS `count`) AS `aggregate`,
`t_Album1`.`artist_other_id` AS `artist_other_id`
FROM (SELECT *
FROM `hasura`.`Album` AS `t_Album1`
ORDER BY (`t_Album1`.`album_self_id`) ASC NULLS FIRST
-- PROBLEM HERE
LIMIT @param0) AS `t_Album1`
GROUP BY `t_Album1`.`artist_other_id`)
AS `aa_albums1`
ON (`aa_albums1`.`artist_other_id` = `t_Artist1`.`artist_self_id`)
ORDER BY (`t_Artist1`.`artist_self_id`) ASC NULLS FIRST
```
Note the `LIMIT @param0` -- that is incorrect because we want to limit
per artist. Instead, we want:
```sql
SELECT `t_Artist1`.`artist_self_id` AS `artist_self_id`,
STRUCT(IFNULL(`aa_albums1`.`nodes`, NULL) AS `nodes`, IFNULL(`aa_albums1`.`aggregate`, STRUCT(0 AS `count`)) AS `aggregate`) AS `albums_aggregate`
FROM `hasura`.`Artist` AS `t_Artist1`
LEFT OUTER JOIN (SELECT ARRAY_AGG(STRUCT(`t_Album1`.`album_self_id` AS `album_self_id`) ORDER BY (`t_Album1`.`album_self_id`) ASC) AS `nodes`,
STRUCT(COUNT(*) AS `count`) AS `aggregate`,
`t_Album1`.`artist_other_id` AS `artist_other_id`
FROM (SELECT *,
-- ADDED
ROW_NUMBER() OVER(PARTITION BY artist_other_id) artist_album_index
FROM `hasura`.`Album` AS `t_Album1`
ORDER BY (`t_Album1`.`album_self_id`) ASC NULLS FIRST
) AS `t_Album1`
-- CHANGED
WHERE artist_album_index <= @param
GROUP BY `t_Album1`.`artist_other_id`)
AS `aa_albums1`
ON (`aa_albums1`.`artist_other_id` = `t_Artist1`.`artist_self_id`)
ORDER BY (`t_Artist1`.`artist_self_id`) ASC NULLS FIRST
```
That serves both the LIMIT/OFFSET function in the where clause. Then,
both the ARRAY_AGG and the COUNT are correct per artist.
I've updated my Haskell test suite to add regression tests for this. I'll push a commit for Python tests shortly. The tests still pass there.
This just fixes a case that we hadn't noticed.
https://github.com/hasura/graphql-engine-mono/pull/1641
GitOrigin-RevId: 49933fa5e09a9306c89565743ecccf2cb54eaa80
### Context
One of the ways we use the Backend type families is to use `Void` for all types for which a backend has no representation; this allows us to make some branches of our metadata and IR unrepresentable, making some functions total, where they would have to handle those unsupported cases otherwise.
However, one of the biggest features, functions, cannot be cut that way, due to one of the constraints on `FunctionName b`: the metadata generator requires it to have an `Arbitrary` instance, and `Arbitrary` does not have a recovery mechanism which would allow for a `Void` instance...
### Description
This PR solves this problem and removes the `Arbitrary` constraints in `Backend`. To do so, it introduces a new typeclass: `PartialArbitrary`, which is very similar to `Arbitrary`, except that it returns a `Maybe (Gen a)`, allowing for `Void` to have a well-formed instance. An `Arbitrary` instance for `Metadata` can easily be retrieved with `arbitrary = fromJust . partialArbitrary`.
Furthermore, `PartialArbitrary` has a generic implementation, inspired by the one in `generic-arbitrary`, which automatically prunes branches that return `Nothing`, allowing to automatically construct most types. Types that don't have a type parameter and therefore can't contain `Void` can easily get their `PartialArbitrary` instance from `Arbitrary` with `partialArbitrary = Just arbitrary`. This is what a default overlappable instance provides.
In conjunction with other cleanups in #1666, **this allows for Void function names**.
### Notes
While this solves the stated problem, there are other possible solutions we could explore, such as:
- switching from QuickCheck to a library that supports that kind of pruning natively
- removing the test altogether, and dropping all notion of Arbitrary from the code
There are also several things we could do with the Generator module:
- move it out of RQL.DDL.Metadata, to some place that makes more sense
- move ALL Arbitrary instances in the code to it, since nothing else uses Arbitrary
- or, to the contrary, move all those Arbitrary instances alongside their types, to avoid an orphan instance
https://github.com/hasura/graphql-engine-mono/pull/1667
GitOrigin-RevId: 88e304ea453840efb5c0d39294639b8b30eefb81
### Description
The spock handler requires the request type to have a `ToJSON` instance AND a `FromJSON` instance. That's because we parse it from the received bytestring into its proper type.... and call `toJSON` on it to log it. This PR simplifies this, by keeping the intermediate `Value` obtained during parsing, and using it for logging. This has two consequences:
1. it removes the `ToJSON` constraint, which will remove some code down the line (esp. in Metadata)
2. it means we log the actual JSON object query we received, not the result of parsing it, meaning the logged object will contain fields that would have been ignored when parsing the actual value; this is both an upside (more accurate log) and a downside (could be more verbose / more confusing)
### Further work
Should this PR also remove all obsolete ToJSON instances while at it?
How do we test this?
https://github.com/hasura/graphql-engine-mono/pull/1664
GitOrigin-RevId: ae099eea9a671eabadcdf507f993a5ad9433be87
- add name fields to log output in several spots:
- action logs get the action name in detail.action_name
- events (triggered and scheduled) get the trigger name in detail.event_name
- one-off scheduled events don't have a trigger name; instead, they get the
comment if it exists
- remove unused event creation timestamp from ExtraLogContext
https://github.com/hasura/graphql-engine-mono/pull/1712
GitOrigin-RevId: 28907340d4e2d9adc0c48cc5d3010eef1fa902e1
- Add export list to Hasura.Eventing.Common
- Group logging options in one type / argument
- Group request header processing code in one place
This doesn't address the convoluted logic, but should make it a bit easier
to figure out what's going on for the next person.
https://github.com/hasura/graphql-engine-mono/pull/1710
GitOrigin-RevId: 34b0abdd1b86b5836eb512484acb0db8c81f3014
### Description
This PR fixes a major issue in the JSON instances of `AnyBackend`: they were not symmetrical! `FromJSON` always made the assumption that the value was an object, and that it contained a "kind" field if it happened to not be a Postgres value. `ToJSON` did NOT insert said field in the output, and did not enforce that the output was an object.
....however, it worked, because nowhere in the code did we yet rely on those being symmetrical. They are both used only once:
- `parseJSON` was used to decode a `Metadata` object, but the matching `toJSON` instance, which is heavily customized, does insert the "kind" field properly
- `toJSON` was only used on the `SchemaCache`, which has no corresponding `FromJSON` instance, since we only serialize it in debug endpoints
This PR makes no attempt at making the instances symmetrical. Instead, it implements simpler functions, and pushes the problem of identifying the proper backend (if any) to the call sites.
### Notes
Additionally, it cleans up some instances that were manually written where they could be auto-generated. In the process, this PR changes the semantics of `Show`, since the stock derived instance will include the constructor name, where before it was skipped. I think it is preferable.
https://github.com/hasura/graphql-engine-mono/pull/1672
GitOrigin-RevId: 0a1580a0e0f01c25b8c9fee7612dba6e7de055d5
* fix resetting the catalog version to 43 on migration from 1.0 to 2.0
* ci: remove applying patch in test_oss_server_upgrade job
* make the 43 to 46th migrations idempotent
* Set missing HASURA_GRAPHQL_EVENTS_HTTP_POOL_SIZE=8 in upgrade_test
It's not clear why this wasn't caught in CI.
* ci: disable one component of event backpressure test
Co-authored-by: Vishnu Bharathi P <vishnubharathi04@gmail.com>
Co-authored-by: Karthikeyan Chinnakonda <karthikeyan@hasura.io>
Co-authored-by: Brandon Simmons <brandon@hasura.io>
GitOrigin-RevId: c74c6425266a99165c6beecc3e4f7c34e6884d4d
Previous versions of 'pg-client-hs' provided a Template Haskell splice
'sqlFromFile' which returned compile-time embedded PostgreSQL queries.
Rather than returning a concrete 'Query', however, this function
returned the polymorphic 'IsString txt => txt' which allowed the caller
to implicitly convert the result to anything other type with some
'IsString' instance.
https://github.com/hasura/graphql-engine-mono/pull/1570
GitOrigin-RevId: fb4294439148ae8b2762138ece2d59e8e18ef5e0
### Description
This PR adds the required IR for DB to DB joins, based on @paf31 and @0x777 's `feature/db-to-db` branch.
To do so, it also refactors the IR to introduce a new type parameter, `r`, which is used to recursively constructs the `v` parameter of remote QueryDBs. When collecting remote joins, we replace `r` with `Const Void`, indicating at the type level that there cannot be any leftover remote join.
Furthermore, this PR refactors IR.Select for readability, moves some code from IR.Root to IR.Select to avoid having to deal with circular dependencies, and makes it compile by adding `error` in all new cases in the execution pipeline.
The diff doesn't make it clear, but most of Select.hs is actually unchanged. Declarations have just been reordered by topic, in the following order:
- type declarations
- instance declarations
- type aliases
- constructor functions
- traverse functions
https://github.com/hasura/graphql-engine-mono/pull/1580
Co-authored-by: Phil Freeman <630306+paf31@users.noreply.github.com>
GitOrigin-RevId: bbdcb4119cec8bb3fc32f1294f91b8dea0728721
A pull request for cleaning up small issues, bugs, redundancies and missing things in the BigQuery backend.
Summary:
1. Remove duplicate projection fields - BigQuery rejects these.
2. Add order_by to the test suite cases, as it was returning inconsistent results.
3. Add lots of in FromIr about how the dataloader approach is given support.
4. Produce the correct output structure for aggregates:
a. Should be a singleton object for a top-level aggregate query.
b. Should have appropriate aggregate{} and nodes{} labels.
c. **Support for nodes** (via array_agg).
5. Smooth over support of array aggregates by removing the fields used for joining with an explicit projection of each wanted field.
https://github.com/hasura/graphql-engine-mono/pull/1317
Co-authored-by: Vamshi Surabhi <6562944+0x777@users.noreply.github.com>
GitOrigin-RevId: cd3899f4667770a27055f94988ef2a6d5808f1f5
### Description
RunSQL commands are analyzed to detect whether they require a schema cache rebuild; in the case of Citus we were always returning `False`. This PR fixes this, and also removes the catch-all case, to make it explicit / obvious whenever we change this.
https://github.com/hasura/graphql-engine-mono/pull/1549
GitOrigin-RevId: dddaaea868e7b7999bdfe11451032df9d9b44274
Remote relationships are now supported on SQL Server and BigQuery. The major change though is the re-architecture of remote join execution logic. Prior to this PR, each backend is responsible for processing the remote relationships that are part of their AST.
This is not ideal as there is nothing specific about a remote join's execution that ties it to a backend. The only backend specific part is whether or not the specification of the remote relationship is valid (i.e, we'll need to validate whether the scalars are compatible).
The approach now changes to this:
1. Before delegating the AST to the backend, we traverse the AST, collect all the remote joins while modifying the AST to add necessary join fields where needed.
1. Once the remote joins are collected from the AST, the database call is made to fetch the response. The necessary data for the remote join(s) is collected from the database's response and one or more remote schema calls are constructed as necessary.
1. The remote schema calls are then executed and the data from the database and from the remote schemas is joined to produce the final response.
### Known issues
1. Ideally the traversal of the IR to collect remote joins should return an AST which does not include remote join fields. This operation can be type safe but isn't taken up as part of the PR.
1. There is a lot of code duplication between `Transport/HTTP.hs` and `Transport/Websocket.hs` which needs to be fixed ASAP. This too hasn't been taken up by this PR.
1. The type which represents the execution plan is only modified to handle our current remote joins and as such it will have to be changed to accommodate general remote joins.
1. Use of lenses would have reduced the boilerplate code to collect remote joins from the base AST.
1. The current remote join logic assumes that the join columns of a remote relationship appear with their names in the database response. This however is incorrect as they could be aliased. This can be taken up by anyone, I've left a comment in the code.
### Notes to the reviewers
I think it is best reviewed commit by commit.
1. The first one is very straight forward.
1. The second one refactors the remote join execution logic but other than moving things around, it doesn't change the user facing functionality. This moves Postgres specific parts to `Backends/Postgres` module from `Execute`. Some IR related code to `Hasura.RQL.IR` module. Simplifies various type class function signatures as a backend doesn't have to handle remote joins anymore
1. The third one fixes partial case matches that for some weird reason weren't shown as warnings before this refactor
1. The fourth one generalizes the validation logic of remote relationships and implements `scalarTypeGraphQLName` function on SQL Server and BigQuery which is used by the validation logic. This enables remote relationships on BigQuery and SQL Server.
https://github.com/hasura/graphql-engine-mono/pull/1497
GitOrigin-RevId: 77dd8eed326602b16e9a8496f52f46d22b795598
This reverts the remote schema type customisation and namespacing feature temporarily as we test for certain conditions.
GitOrigin-RevId: f8ee97233da4597f703970c3998664c03582d8e7
>
### Description
>
### 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
->
Fixes#1528
### Solution and Design
>
Only replace connection configuration instead of replacing entire metadata with empty one.
GitOrigin-RevId: f9a16dcc7b1219ec1af915bf083622fcb7dde69a
This is a minor refactor (part of `Internal/Parser.hs` is moved into `Internal/Input.hs`) to remove `Collect.hs-boot` and `Directives.hs-boot` files. Without these changes:
1. Most changes would trigger recompilation from the modules with hs-boot files.
1. haskell-language-server fails for some reason in the presence of hs-boot files.
GitOrigin-RevId: 77a2e443417b449c5d7d9d418fc75fcdf076a9ae
This implements the `snakeCaseTableName` method. It's part of the larger BigQuery PR at https://github.com/hasura/graphql-engine-mono/pull/1317, but @0x777 requested a smaller PR with this change.
GitOrigin-RevId: aeb93c3228bb6ba7293cd73b7a34b39c7ed6139a
