This is a first step towards clarifying the role of `UnpreparedValue` as part of the IR. It certainly does not belong in the parser framework.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4588
GitOrigin-RevId: d1582a0b266729b79e00d31057178a4099168e6d
### Description
When generalizing the code, back in late 2020, we over-eagerly generalized parts of the code that are specific to RQL's DML. This was in part due to the fact that, at the time, the DML types were all mixed alongside other types in `RQL.Types`. As a result, a lot of `RQL.DML.Internal` was generic over the backend type, instead of being specialized to `'Postgres 'Vanilla`.
A consequence of this is that, before this PR, `DML.Internal` ended up having a dependency on non-Postgres backends, due to the use of `annBoolExp`, which requires a `BackendMetadata` instance. Since the code was written in a generic manner, `DML.Internal` in turn depended on having the metadata instances in scope... This PR changes that to, instead, explicitly import the Postgres instance.
(Note that this module didn't import `RQL.Types.Metadata.Instances`, but depends on a module that imports it, and **orphan instances are transitively imported**, as evidenced by the need for that explicit import in #4568.)
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4573
GitOrigin-RevId: 7b82b5d7c23c03654518a1816802d400f37c3c64
### Description
There were several functions in `GraphQL.Schema.Common` that were unrelated to the schema building process, and were about metadata manipulation or dependency computation. Having those functions in the schema part of the code forces several places in the code to depend on the schema code, despite being completely unrelated.
This PR moves those functions where they make sense: alongside similar functions in `RQL.Types.*`, and rewrites `getRemoteDependencies` for clarity (it was using the term "indirect dependency" in a way that was inconsistent with the rest of the code).
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4568
GitOrigin-RevId: 948a18cebbb337a8bb6367c1f2d2ef5628209d96
### Description
Several places in the code used `a /= []`, which is inelegant. To my surprise, hlint did not warn about this, despite the fact that it forces an `Eq` instance on the elements. This PR replaces all occurrences of that pattern with `not (null a)` and adds a lint warning for it.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4569
GitOrigin-RevId: 6471e75ade9e71e5d583a0dac7815c01870c696b
By generalizing the instances, they can be written as attached instance derivations, rather than standalone ones.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4518
GitOrigin-RevId: 7a387911cf6ad46fe6acd36648275d6c2c68ffe3
A very minor cleanup (came out of documenting the architecture of actions). Does what's mentioned in the title.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4451
GitOrigin-RevId: d480ff438256df468df65b43d15f92a30b14b997
Previously, these were represented with a HashMap, but supposedly that map can never be empty. Now, it uses NEHashMap, which carries the non-empty invariant behind a smart constructor.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4481
GitOrigin-RevId: 93ad9aaa9354f25a1ba10e8207ae19614e1e439e
### Description
As part of the cache building process, we create / update / migrate the catalog that each DB uses as a place to store event trigger information. The function that decides how this should be done was doing an explicit `case ... of` on the backend tag, instead of delegating to one of the backend classes. The downsides of this is that:
- it adds a "friction point" where the backend matters in the core of the engine, which is otherwise written to be almost entirely backend-agnostic
- it creates imports from deep in the engine to the `Backends`, which we try to restrict to a very small set of clearly identified files (the `Instances` files)
- it is currently implemented using a "catch all" default case, which might not always be correct for new backends
This PR makes the catalog updating process a part of `BackendMetadata`, and cleans the corresponding schema cache code.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4457
GitOrigin-RevId: 592f0eaa97a7c38f4e6d4400e1d2353aab12c97e
## 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
`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
### 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 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 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
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
### 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
### 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
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
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
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
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
### 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
## 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
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
<!-- 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
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
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
>
### 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
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
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
This is a follow-up to #1959.
Today, I spent a while in review figuring out that a harmless PR change didn't do anything,
because it was moving from a `runLazy...` to something without the `Lazy`. So let's get
that source of confusion removed.
This should be a bit easier to review commit by commit, since some of the functions had
confusing names. (E.g. there was a misnamed `Migrate.Internal.runTx` before.)
The change should be a no-op.
https://github.com/hasura/graphql-engine-mono/pull/2335
GitOrigin-RevId: 0f284c4c0f814482d7827e7732a6d49e7735b302
### Description
During the PDV refactor that led to 2.0, we broke an undocumented and untested semantic of inserts: accepting _explicit_ null values in nested object inserts.
In short: in the schema, we often distinguish between _explicit_ null values `{id: 3, author: null}` and _implicit_ null values that correspond to the field being omitted `{id: 3}`. In this particular case, we forgot to accept explicit null values. Since the field is optional (meaning we accept implicit null values), it was nullable in the schema, like it was in pre-PDV times. But in practice we would reject explicit nulls.
This PR fixes this, and adds a test. Furthermore, it does a bit of a cleanup of the Mutation part of the schema, and more specifically of all insertion code.
https://github.com/hasura/graphql-engine-mono/pull/2341
GitOrigin-RevId: 895cfeecef7e8e49903a3fb37987707150446eb0
This PR only contains minor changes to documentation that I have collected over some time, revising text as I was passing by.
https://github.com/hasura/graphql-engine-mono/pull/2346
Co-authored-by: Rikin Kachhia <54616969+rikinsk@users.noreply.github.com>
GitOrigin-RevId: f3329f3212b831f1f3c74a299734faff337b1017
>
### Description
>
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
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 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
## 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
### 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
### 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
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
### 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
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
