Commit Graph

899 Commits

Author SHA1 Message Date
Karthikeyan Chinnakonda
cb29607833 server: fix mutations bug when inherited roles is enabled
https://github.com/hasura/graphql-engine-mono/pull/1989

GitOrigin-RevId: d4e41431fdf90426651dd1289eb0f0e099de95b9
2021-08-03 11:12:01 +00:00
Karthikeyan Chinnakonda
0a3fd16c35 server: revert the relaxing of unique name constraint in allow-lists
https://github.com/hasura/graphql-engine-mono/pull/1972

GitOrigin-RevId: cb062df7a1ba1c99705f811409e2e4ad42f4b581
2021-08-03 09:23:20 +00:00
Auke Booij
5fc3cf1446 server: make LazyTxT less lazy
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
2021-08-02 19:14:08 +00:00
Swann Moreau
63d5b7ad53 pro, server: improve parameterised query hash handling for batched requests (fix #1767)
https://github.com/hasura/graphql-engine-mono/pull/1768

GitOrigin-RevId: 11615408ee2fb097e68ee4a641e5dc46c1d28795
2021-08-02 16:05:17 +00:00
Evie Ciobanu
e48ccd7fab server: Check session variables for subscriptions
https://github.com/hasura/graphql-engine-mono/pull/1879

GitOrigin-RevId: 78d3384cb21a36e8b8c85c17ae7578ce0b4230f8
2021-07-30 21:42:52 +00:00
Antoine Leblanc
c5face545c Clean some instances
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
2021-07-30 15:43:32 +00:00
David Overton
1abb1dee69 Remote Schema Customization take 2 using parser tranformations
https://github.com/hasura/graphql-engine-mono/pull/1740

GitOrigin-RevId: e807952058243a97f67cd9969fa434933a08652f
2021-07-30 11:33:59 +00:00
Aniket Deshpande
61663ec901 MSSQL: __typename error and json based aggregates (graphql-engine#7130)
https://github.com/hasura/graphql-engine-mono/pull/1930

GitOrigin-RevId: f14df470ffa298c18e006c522d6355298041ae8e
2021-07-30 09:03:57 +00:00
Antoine Leblanc
8adc8abf0f kill Arbitrary (fix #1736)
Delete all weird Arbitrary things from our codebase.

https://github.com/hasura/graphql-engine-mono/pull/1947

GitOrigin-RevId: b6d48db78d2ef4ac2fd232c684ff5039dc345fc4
2021-07-30 07:55:37 +00:00
Auke Booij
3607f472b5 server: build graphql schema without arrows
https://github.com/hasura/graphql-engine-mono/pull/1827

GitOrigin-RevId: 8099ea708283647221973fbfc2c50e89d751efdb
2021-07-29 10:32:02 +00:00
Naveen Naidu
f49e13c890 server: SQL query tags for GQL operations (closes #400)
https://github.com/hasura/graphql-engine-mono/pull/1393

GitOrigin-RevId: f867c16d8281865dac38c91f7dfcbf5815de898c
2021-07-29 08:30:10 +00:00
Rakesh Emmadi
a63fa18d9c server/postgres: Support computed fields in permission check/filter
https://github.com/hasura/graphql-engine-mono/pull/1697

GitOrigin-RevId: 6cdf8acc90d3fd97d20a3ee68c84306c3f589370
2021-07-28 08:10:25 +00:00
Rakesh Emmadi
5cfac6ea87 server/postgres: support computed fields in order by
https://github.com/hasura/graphql-engine-mono/pull/1793

GitOrigin-RevId: e0396c0d4d96fc8f9bdbd7567193933db5b295a6
2021-07-27 16:28:23 +00:00
Anon Ray
e4155e4c5b server: log post drop-source hook errors instead of throwing
https://github.com/hasura/graphql-engine-mono/pull/1724

GitOrigin-RevId: 659d9335861b71ee6ed551eedfb0926d16c9ac3d
2021-07-27 15:15:47 +00:00
Antoine Leblanc
a978b6d683 Remove several outdated backend constraints
## 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
2021-07-27 13:52:20 +00:00
Auke Booij
7bead93827 server: remove remnants of query plan caching (fix #1795)
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
2021-07-27 11:52:43 +00:00
Antoine Leblanc
cc6c86aeab Clean metadata arguments
## 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
2021-07-27 10:42:51 +00:00
Sameer Kolhar
454e6afd1d server: fix pg_invoke_event_trigger regression
https://github.com/hasura/graphql-engine-mono/pull/1785

GitOrigin-RevId: 0addb457c3790f886b4d708ffc08588aebfc6aca
2021-07-27 08:06:25 +00:00
Sameer Kolhar
6d7ec06502 server: update the logging strategy for inconsistent metadata
https://github.com/hasura/graphql-engine-mono/pull/1776

GitOrigin-RevId: 44a8e3b2b8d515aa240905fb07e52add4d565b4d
2021-07-27 05:42:05 +00:00
Evie Ciobanu
4b0f5f09c9 server: add IR support for DB-to-DB joins
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
2021-07-26 13:05:53 +00:00
Antoine Leblanc
49f40a44f0 Enforce that backends use the properly resolved environment variables.
## 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
2021-07-23 12:26:10 +00:00
Antoine Leblanc
0aaf006c25 server: add metadata for DB-DB remote joins
### 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
2021-07-22 23:07:10 +00:00
Vamshi Surabhi
5eb72e202b bigquery: pass the location when fetching a job's status
https://github.com/hasura/graphql-engine-mono/pull/1847

GitOrigin-RevId: ed6c5e0d3caae27f32b97d563873720df77b017a
2021-07-22 15:00:16 +00:00
Antoine Leblanc
8a4caba4e2 [gardening] fix schema cache type instances
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
2021-07-21 23:09:32 +00:00
Abby Sassel
1cd9fc376d server/mysql: integrate MySQL tests into dev.sh workflow
https://github.com/hasura/graphql-engine-mono/pull/1848

GitOrigin-RevId: 2bd7c0a18448f042a1e35d21aaf42232acf48a46
2021-07-21 17:22:56 +00:00
Karthikeyan Chinnakonda
34ccca305d server: relax unique operation name constraint in a query collection
https://github.com/hasura/graphql-engine-mono/pull/1849

GitOrigin-RevId: 56c1737b69511c3f2c4480d22f84828f51d3b543
2021-07-21 12:05:50 +00:00
Sibi Prabakaran
0e6e9deac9 mysql: runSql implementation and python tests leveraging it to enhance the metadata test
https://github.com/hasura/graphql-engine-mono/pull/1606

Co-authored-by: Chris Done <11019+chrisdone@users.noreply.github.com>
Co-authored-by: Aniket Deshpande <922486+aniketd@users.noreply.github.com>
Co-authored-by: Abby Sassel <3883855+sassela@users.noreply.github.com>
GitOrigin-RevId: 2ed37c48c5d1e82e23d691f30a6e870303787388
2021-07-21 10:22:54 +00:00
Lyndon Maydwell
4c4a599373 Committed choice in ReplaceMetadata parsing based on Version
https://github.com/hasura/graphql-engine-mono/pull/1830

GitOrigin-RevId: 1f308ff11c1671ba10a854f221c0912e61b6b114
2021-07-21 05:03:37 +00:00
Antoine Leblanc
83045a4459 server: fix erroneous Eq instance on Metadata Object
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
2021-07-20 14:24:45 +00:00
Philip Lykke Carlsen
2b9e407de5 Server: Index RawFunctionInfo by backend
https://github.com/hasura/graphql-engine-mono/pull/1800

GitOrigin-RevId: 473f72fe47b443a7022c16637a6e3ab9ed016c58
2021-07-19 15:36:09 +00:00
Aniket Deshpande
295f7f3736 bq: fix optional global_select_limit config
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
2021-07-19 11:40:17 +00:00
Karthikeyan Chinnakonda
6bddaa7564 server: inherited roles improvements for select permissions only
https://github.com/hasura/graphql-engine-mono/pull/1539

GitOrigin-RevId: 7444973e9cc7e0598eeb1bd7951ad45dd37ec550
2021-07-16 21:19:58 +00:00
Solomon Bothwell
d88e2bbcce server: add tests ensuring the correct functioning of all endpoints based on user roles
https://github.com/hasura/graphql-engine-mono/pull/1625

Co-authored-by: Sameer Kolhar <6604943+kolharsam@users.noreply.github.com>
GitOrigin-RevId: 6b56efc838d2ed1acc44b2847161fde22d6aee17
2021-07-16 16:09:25 +00:00
Sibi Prabakaran
385d27449e mysql: Metadata awareness
https://github.com/hasura/graphql-engine-mono/pull/1599

Co-authored-by: Chris Done <11019+chrisdone@users.noreply.github.com>
Co-authored-by: Aniket Deshpande <922486+aniketd@users.noreply.github.com>
Co-authored-by: Abby Sassel <3883855+sassela@users.noreply.github.com>
GitOrigin-RevId: 4df4a8ff00fa8ef311a85199d66abe4cc10adc8c
2021-07-15 12:45:51 +00:00
Vamshi Surabhi
9ec40be58d bigquery: 'global_select_limit' is now optional and defaults to 1000
See https://github.com/hasura/graphql-engine-mono/pull/1774. This new branch had to be opened because of an issue with cli test failure if the branch name starts with a number.

https://github.com/hasura/graphql-engine-mono/pull/1784

Co-authored-by: Aniket Deshpande <922486+aniketd@users.noreply.github.com>
GitOrigin-RevId: 25d2d92f57c10d80bc54b47a3f895ab014b8e6fa
2021-07-13 15:50:12 +00:00
Aniket Deshpande
921bdf709b MySQL: Initial connection and corresponding types
https://github.com/hasura/graphql-engine-mono/pull/1517

Co-authored-by: Sibi Prabakaran <737477+psibi@users.noreply.github.com>
Co-authored-by: Chris Done <11019+chrisdone@users.noreply.github.com>
Co-authored-by: Abby Sassel <3883855+sassela@users.noreply.github.com>
GitOrigin-RevId: e0f61664b33d8ca50f223ec7fb76c66940eabc89
2021-07-13 13:33:21 +00:00
Anon Ray
d8fa32776b server: log query in http-log/ws-log only if query-log is enabled
https://github.com/hasura/graphql-engine-mono/pull/1771

GitOrigin-RevId: 1fb8246f6ebff7e114779f284ba36d510b504f29
2021-07-13 12:24:19 +00:00
Rakesh Emmadi
df1efdcd27 server/postgres: fix reload sources and remote schemas if they're inconsistent
https://github.com/hasura/graphql-engine-mono/pull/1709

GitOrigin-RevId: c0684380d0e86dd6f28d6c8a0aebeaa210319272
2021-07-13 07:57:21 +00:00
Rakesh Emmadi
a375f8c105 server/postgres: Support scalar computed fields in remote joins
https://github.com/hasura/graphql-engine-mono/pull/1692

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
GitOrigin-RevId: fcef85910899859f7421cad554c022f8023965ea
2021-07-12 16:04:37 +00:00
Antoine Leblanc
0a9382f2dc [gardening] remove Alias from Backend
### 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
2021-07-09 15:54:56 +00:00
Aniket Deshpande
66f09eeaab MSSQL nodes aggregates & inherited roles
https://github.com/hasura/graphql-engine-mono/pull/1293

Co-authored-by: Chris Done <11019+chrisdone@users.noreply.github.com>
Co-authored-by: Abby Sassel <3883855+sassela@users.noreply.github.com>
GitOrigin-RevId: 776402dbbaf3d8166a62b1aaaf6abc7e584b3eb2
2021-07-08 20:50:09 +00:00
Antoine Leblanc
d91029ad51 [gardening] remove all traverse functions from RQL.IR
### 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
2021-07-08 15:42:53 +00:00
Rakesh Emmadi
e567a096e6 server/postgres: support computed fields in query filters ('where' expression)
https://github.com/hasura/graphql-engine-mono/pull/1677

GitOrigin-RevId: 30175a442237f6ac4b112c652f448a635ad90dc6
2021-07-07 11:59:32 +00:00
Robert
ae6d624441 server: improve handling of HTTP client errors for remote schema calls
https://github.com/hasura/graphql-engine-mono/pull/1719

GitOrigin-RevId: 5ff671bcff5f0559b9821f8359ebee4129f4b664
2021-07-07 10:15:05 +00:00
Antoine Leblanc
80161e4208 server: delegate Metadata API parsing to a new backend class
https://github.com/hasura/graphql-engine-mono/pull/1179

Co-authored-by: Abby Sassel <3883855+sassela@users.noreply.github.com>
Co-authored-by: Tirumarai Selvan <8663570+tirumaraiselvan@users.noreply.github.com>
GitOrigin-RevId: b565de269e215ae8172bddd895f3d057ddcc8695
2021-07-07 01:44:41 +00:00
Chris Done
614c0dab80 Bigquery/fix limit offset for array aggregates
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
2021-07-06 08:29:39 +00:00
Antoine Leblanc
6ed800abaa [gardening] Introduce PartialArbitrary
### 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
2021-07-05 22:04:38 +00:00
Antoine Leblanc
c64f66774b [gardening] remove ToJSON constraint in spockAction (fix #1581)
### 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
2021-07-05 18:47:55 +00:00
Swann Moreau
f6e49500d5 server: log request type (batched/single) in http-log
https://github.com/hasura/graphql-engine-mono/pull/1488

GitOrigin-RevId: 092335d6eb95224b8ed81d853a71fca01379dd7d
2021-07-05 09:46:29 +00:00
Robert
5cc91aebc3 server: Log action and event names (close #1603)
- 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
2021-07-05 07:48:39 +00:00