Return a `T` instead of a `Result<T, E>` when we never return an error
(`E`) case.
I also enabled some more warnings. `unnecessary_box_returns` has been
suppressed where appropriate, and `unused_async` doesn't seem to be
violated anywhere any more.
I got rid of some calls to `.unwrap()` too.
V3_GIT_ORIGIN_REV_ID: 015ebd05978cf8c2d87474a90e0cd4333779a761
This is Part 3 in a stacked PR set that delivers aggregate root field
support.
* Part 1: OpenDD: https://github.com/hasura/v3-engine/pull/683
* Part 2: Metadata Resolve: https://github.com/hasura/v3-engine/pull/684
JIRA: [V3ENGINE-159](https://hasurahq.atlassian.net/browse/V3ENGINE-159)
## Description
This PR implements the GraphQL API for aggregate root fields. The
GraphQL schema matches the design in the [Aggregate and Grouping
RFC](https://github.com/hasura/v3-engine/blob/main/rfcs/aggregations.md#aggregations-walkthrough).
### Schema Generation
The main new part of the GraphQL schema generation can be found in
`crates/schema/src/aggregates.rs`. This is where we generate the new
aggregate selection types. However, the root field generation can be
found in `crates/schema/src/query_root/select_aggregate.rs`.
The new `filter_input` type generation lives in
`crates/schema/src/model_filter_input.rs`. As this type effectively
encapsulates the existing field arguments used on the Select Many root
field, the code to generate them has moved into `model_filter_input.rs`
and `select_many.rs` simply reuses the functionality from there (without
actually using the filter input type!).
### IR
The main aggregates IR generation for the aggregate root field happens
in `crates/execute/src/ir/query_root/select_aggregate.rs`. It reads all
the input arguments to the root field and then kicks the selection logic
over to `model_aggregate_selection_ir` from
`crates/execute/src/ir/model_selection.rs`.
`crates/execute/src/ir/model_selection.rs` has received some refactoring
to facilitate that new `model_aggregate_selection_ir` function; it
mostly shares functionality with the existing `model_selection_ir`,
except instead of creating fields IR, it creates aggregates IR instead.
The actual reading of the aggregate selection happens in
`crates/execute/src/ir/aggregates.rs`.
The aggregates selection IR captures the nested JSON structure of the
aggregate selection, because NDC does not return aggregates in the same
nested JSON structure as the GraphQL request. NDC takes a flat list of
aggregate operations to run. This captured nested JSON structure is used
during response rewriting to convert NDC's flat list into the nested
structure that matches the GraphQL request.
The aggregate selection IR is placed onto `ModelSelection` alongside the
existing fields IR. Since both fields and aggregates can be put into the
one NDC request (even though they are not right now), this made sense.
They both translate onto one NDC `Query`. This necessitated making the
field selection optional on the `ModelSelection`
(`ModelSelection.selection`), since aggregate requests currently don't
use them.
### Planning
`crates/execute/src/plan/model_selection.rs` takes care of mapping the
aggregates into the NDC request from the generated IR.
There has been a new `ProcessResponseAs` variant added in
`crates/execute/src/plan.rs` to capture how to read and reshape an NDC
aggregates response. This is handled in
`crates/execute/src/process_response.rs` where the captured JSON
structure in the IR is used to restore NDC's flat aggregates list into
the required nested JSON output structure.
### Testing
The Custom Connector has been updated with functionality to allow
aggregates over nested object fields
(`crates/custom-connector/src/query.rs`).
New execution and introspection tests have been added to
`crates/engine/tests/execute/aggregates/` to test aggregates against
Postgres and the Custom Connector.
[V3ENGINE-159]:
https://hasurahq.atlassian.net/browse/V3ENGINE-159?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
V3_GIT_ORIGIN_REV_ID: ff47f13eaca70d10de21e102a6667110f8f8af40
This is Part 2 in a stacked PR set that delivers aggregate root field
support.
* Part 1: OpenDD: https://github.com/hasura/v3-engine/pull/683
* Part 3: GraphQL API: https://github.com/hasura/v3-engine/pull/685
JIRA: [V3ENGINE-159](https://hasurahq.atlassian.net/browse/V3ENGINE-159)
## Description
This PR implements the metadata resolve phase of the engine and adds
support for resolving `AggregateExpression`s and validates their use
when linked to a `Model`.
The bulk of the changes can be found in:
* `crates/metadata-resolve/src/stages/aggregates/*` - This is where the
`AggregateExpression`s are resolved
* `crates/metadata-resolve/src/stages/models/mod.rs` - This is where we
validate the `AggregateExpression` specified for use in the model is
actually compatible with the model and its data connector
The `ndc-spec` version used has been lifted to the latest version that
adds support for aggregates over nested objects
(https://github.com/hasura/ndc-spec/pull/144). This necessitated changes
in the Custom Connector, but actual functionality to implement
aggregation over nested objects is implemented in Part 3.
There are also some changes in
`crates/metadata-resolve/src/types/subgraph.rs` where the `Display`
trait for the various `Qualified<T>`, `QualifiedTypeReference`, etc
types has been reworked so that they print more cleanly, with the
subgraph being put outside the type syntax, and array types getting
formatted correctly. For example, previous an array of Varchars would
have printed as `Varchar (in subgraph default)!`, now it properly
formats as `[Varchar!]! (in subgraph default)`.
This was necessary to make useful error messages using these types.
A tonne of tests have been added in
`crates/engine/tests/validate_metadata_artifacts/aggregate_expressions`
to test every error condition of the metadata resolve process.
[V3ENGINE-159]:
https://hasurahq.atlassian.net/browse/V3ENGINE-159?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
V3_GIT_ORIGIN_REV_ID: ffd859127a3f1560707f06ef01906c9d1b183d31
If a function doesn't return a value, terminate with a semicolon.
I also moved `implicit_hasher` and `return_self_not_must_use` to the
"definitely keep disabling this" list, and installed
[Bacon](https://dystroy.org/bacon/) in the Nix shell to make it easier
to run Clippy.
V3_GIT_ORIGIN_REV_ID: ffb17b42d982518aec433a1676dba0a0dd0ad95d
Add a test case for input type infinite recursion bug (https://github.com/hasura/v3-engine/pull/676). I confirmed the
test failure when the fix was removed.
V3_GIT_ORIGIN_REV_ID: b9d0a9b2a65328aa52f5ec057dcba6470c4e3956
## Description
This PR implements argument presets for `DataConnectorLink`, which can
be used to forward request headers as function/procedure arguments to
NDC. This PR implements the execution part.
**Note**: response header forwarding is not implemented yet.
V3_GIT_ORIGIN_REV_ID: ff69129aee3e3052367ca42acdec3922cbc2cb0c
1. Always name the struct when calling `default()`.
2. Sort construction according to the definitions.
3. Approve allowing `struct_field_names` because it doesn't seem to be
helpful.
4. Enable `manual_string_new`; nothing seems to be triggering it now.
V3_GIT_ORIGIN_REV_ID: 868742114b0bf27bc3ea03cdf1e63a0f710ebe33
Just because it's fewer lines of code.
1. Invert `if`/`else` blocks with negative conditions.
2. Unwrap redundant `else` blocks.
3. Simplify a few branches to `let … else`.
4. Replace a `match` with `if let`.
V3_GIT_ORIGIN_REV_ID: 4f10730b688d21c1fc86a45ee5fb4adf008b3d94
Fix some warnings flagged by Clippy.
1. Elide `.into_iter()` where it's unnecessary.
2. Favor `&` over `.iter()`.
3. Use `.values()` on maps instead of discarding keys by destructuring.
4. Avoid `::from_iter(…)` in favor of `.collect()`.
I also replaced a call to `.cloned()` with `.copied()`.
V3_GIT_ORIGIN_REV_ID: 7d39665b0cd04f5bae9405c0ff5f044f57433f32
Enable HTTP/2 support for Axum, so that we can make use of request
multiplexing from the engine in the future.
I also added an `env_logger` to the custom connector so we can see
what's going on there.
V3_GIT_ORIGIN_REV_ID: 3082b988ae3d149ee548f94638308876734b26df
<!-- Thank you for submitting this PR! :) -->
## Description
This PR enables boolean expressions in command argument presets. This
allows users to provide an expression to do things like "when the `user`
role runs the `deleteUser` command they can only delete users with `id`
== `x-hasura-user-id`".
It's up to the data connector to interpret the expression as it sees
fit.
<!--
Questions to consider answering:
1. What user-facing changes are being made?
2. What are issues related to this PR? (Consider adding `(close
#<issue-no>)` to the PR title)
3. What is the conceptual design behind this PR?
4. How can this PR be tested/verified?
5. Does the PR have limitations?
6. Does the PR introduce breaking changes?
-->
## Changelog
- Add a changelog entry (in the "Changelog entry" section below) if the
changes
in this PR have any user-facing impact. See
[changelog
guide](https://github.com/hasura/graphql-engine-mono/wiki/Changelog-Guide).
- If no changelog is required ignore/remove this section and add a
`no-changelog-required` label to the PR.
### Product
_(Select all products this will be available in)_
- [X] community-edition
- [X] cloud
<!-- product : end : DO NOT REMOVE -->
### Type
<!-- See changelog structure:
https://github.com/hasura/graphql-engine-mono/wiki/Changelog-Guide#structure-of-our-changelog
-->
_(Select only one. In case of multiple, choose the most appropriate)_
- [ ] highlight
- [X] enhancement
- [ ] bugfix
- [ ] behaviour-change
- [ ] performance-enhancement
- [ ] security-fix
<!-- type : end : DO NOT REMOVE -->
### Changelog entry
<!--
- Add a user understandable changelog entry
- Include all details needed to understand the change. Try including
links to docs or issues if relevant
- For Highlights start with a H4 heading (#### <entry title>)
- Get the changelog entry reviewed by your team
-->
Allow using boolean expressions in command arguments.
<!-- changelog-entry : end : DO NOT REMOVE -->
<!-- changelog : end : DO NOT REMOVE -->
V3_GIT_ORIGIN_REV_ID: 302f62a266db9c090ba232fe9ab06db4953f38a6
<!-- Thank you for submitting this PR! :) -->
## Description
We'd like to speed up creation of all these Docker images, so this adds
`dev-auth-webhook` to the Nix flake. Functional no-op.
---------
Co-authored-by: Samir Talwar <samir.talwar@hasura.io>
V3_GIT_ORIGIN_REV_ID: 384eb467b2fe7fba1644f5b4cc6224cdc043ce01
This changes the way we build the custom connector so that the data is
baked in at compile time, not loaded from the file system at runtime.
This simplifies packaging.
I also took the liberty to switch out `.unwrap()` for `anyhow`, and to
avoid parsing a string to construct the socket address.
V3_GIT_ORIGIN_REV_ID: d562e0a9852b4e3ecde45bd6938d8eb1b066211b
When trying to reduce the number of dependencies we use in the engine, I
was blocked by a few `.clone()` calls that, on inspection, turned out to
be completely unnecessary.
I have replaced those with passing by reference, and then gone on a
pedant spree. I enabled the `needless_pass_by_value` Clippy warning and
fixed it everywhere that it highlighted. In most places, this meant
adding `&`, but I also marked some types as `Copy`, which makes
pass-by-value the right move.
In one place, I replaced calls to `async_map` with `if` and `else`, to
avoid constructing closures that capture across async boundaries. This
means I could just delete `async_map`.
V3_GIT_ORIGIN_REV_ID: 6ff71f0c553b707889d89552eff3e8c001e898cc
<!-- Thank you for submitting this PR! :) -->
## Description
Following the approach taken here:
https://github.com/hasura/ndc-postgres/pull/402
This moves the `clippy` settings into the Cargo workspace file instead
of passing them for each invocation.
We enable all pedantic settings, run `cargo clippy --fix` to auto fix a
few things, and then manually disable all other lints.
Plenty of them are worth enabling and fixing in future IMO.
---------
Co-authored-by: Samir Talwar <samir.talwar@hasura.io>
V3_GIT_ORIGIN_REV_ID: aa0e6ccb8d72a7393e14b5c58b82077a67d9cb15
<!-- Thank you for submitting this PR! :) -->
## Description
- Update to `ndc-spec`-`0.1.2`
- Use `ndc_models` since `ndc_client` was removed
- Use `Int32` in `custom_connector` everywhere
<!--
Questions to consider answering:
1. What user-facing changes are being made?
2. What are issues related to this PR? (Consider adding `(close
#<issue-no>)` to the PR title)
3. What is the conceptual design behind this PR?
4. How can this PR be tested/verified?
5. Does the PR have limitations?
6. Does the PR introduce breaking changes?
-->
V3_GIT_ORIGIN_REV_ID: 00c6e7a6c213ab0de31303a93f8446c1d371c538
<!-- Thank you for submitting this PR! :) -->
## Description
Fix a bug which was causing an internal error when `null` was returned
by NDC for a field of array or object type.
### Product
_(Select all products this will be available in)_
- [x] community-edition
- [x] cloud
<!-- product : end : DO NOT REMOVE -->
### Type
<!-- See changelog structure:
https://github.com/hasura/graphql-engine-mono/wiki/Changelog-Guide#structure-of-our-changelog
-->
_(Select only one. In case of multiple, choose the most appropriate)_
- [ ] highlight
- [ ] enhancement
- [x] bugfix
- [ ] behaviour-change
- [ ] performance-enhancement
- [ ] security-fix
<!-- type : end : DO NOT REMOVE -->
### Changelog entry
<!--
- Add a user understandable changelog entry
- Include all details needed to understand the change. Try including
links to docs or issues if relevant
- For Highlights start with a H4 heading (#### <entry title>)
- Get the changelog entry reviewed by your team
-->
Fix a bug which was causing an internal error when `null` was returned
by NDC for a field of array or object type.
<!-- changelog-entry : end : DO NOT REMOVE -->
<!-- changelog : end : DO NOT REMOVE -->
V3_GIT_ORIGIN_REV_ID: 5c935ccd6720b5e5966dfa87c2e21dbb7a2b36f2
<!-- Thank you for submitting this PR! :) -->
## Description
When I run `cargo fmt` on my branches, it makes more diff than I want.
This PR fixes that by adding `just format` / `just fmt`, and adding it
to a CI job.
<!--
Questions to consider answering:
1. What user-facing changes are being made?
2. What are issues related to this PR? (Consider adding `(close
#<issue-no>)` to the PR title)
3. What is the conceptual design behind this PR?
4. How can this PR be tested/verified?
5. Does the PR have limitations?
6. Does the PR introduce breaking changes?
-->
## Changelog
- Add a changelog entry (in the "Changelog entry" section below) if the
changes in this PR have any user-facing impact. See [changelog
guide](https://github.com/hasura/graphql-engine-mono/wiki/Changelog-Guide).
- If no changelog is required ignore/remove this section and add a
`no-changelog-required` label to the PR.
### Product
_(Select all products this will be available in)_
- [ ] community-edition
- [ ] cloud
<!-- product : end : DO NOT REMOVE -->
### Type
<!-- See changelog structure:
https://github.com/hasura/graphql-engine-mono/wiki/Changelog-Guide#structure-of-our-changelog
-->
_(Select only one. In case of multiple, choose the most appropriate)_
- [ ] highlight
- [ ] enhancement
- [ ] bugfix
- [ ] behaviour-change
- [ ] performance-enhancement
- [ ] security-fix
<!-- type : end : DO NOT REMOVE -->
### Changelog entry
<!--
- Add a user understandable changelog entry
- Include all details needed to understand the change. Try including
links to docs or issues if relevant
- For Highlights start with a H4 heading (#### <entry title>)
- Get the changelog entry reviewed by your team
-->
_Replace with changelog entry_
<!-- changelog-entry : end : DO NOT REMOVE -->
<!-- changelog : end : DO NOT REMOVE -->
V3_GIT_ORIGIN_REV_ID: e31e352f27b9ad0129c3759fead051b1a8d86758
<!-- Thank you for submitting this PR! :) -->
## Description
I have upgraded ndc-spec to v0.1.1. This pulls in an upgrade to
opentelemetry v0.22 so I've done that too.
We had issues with the `traceresponse` response header disappearing when
using an opentelemetry version > v0.20. This seems to be resolved here,
probably because we have pinned a single version of opentelemetry rather
than loading multiple at once. I added the `traceresponse` header to
verify this, and have left it in because it doesn't seem to hurt
anything.
I verified that everything works as expected with the OSS components,
but not multitenant.
## Changelog
- Add a changelog entry (in the "Changelog entry" section below) if the
changes in this PR have any user-facing impact. See [changelog
guide](https://github.com/hasura/graphql-engine-mono/wiki/Changelog-Guide).
- If no changelog is required ignore/remove this section and add a
`no-changelog-required` label to the PR.
### Product
_(Select all products this will be available in)_
- [x] community-edition
- [x] cloud
<!-- product : end : DO NOT REMOVE -->
### Type
<!-- See changelog structure:
https://github.com/hasura/graphql-engine-mono/wiki/Changelog-Guide#structure-of-our-changelog
-->
_(Select only one. In case of multiple, choose the most appropriate)_
- [ ] highlight
- [x] enhancement
- [ ] bugfix
- [ ] behaviour-change
- [ ] performance-enhancement
- [ ] security-fix
<!-- type : end : DO NOT REMOVE -->
### Changelog entry
<!--
- Add a user understandable changelog entry
- Include all details needed to understand the change. Try including
links to docs or issues if relevant
- For Highlights start with a H4 heading (#### <entry title>)
- Get the changelog entry reviewed by your team
-->
v3 engine GraphQL queries now include a "traceresponse" header.
<!-- changelog-entry : end : DO NOT REMOVE -->
<!-- changelog : end : DO NOT REMOVE -->
V3_GIT_ORIGIN_REV_ID: f37b6bdb1331b55c49f0bd953f522f5eca298d1d
`cargo machete` is a very useful tool that figures out when you aren't
using a dependency. I have run this locally to remove unused
dependencies.
I've also added a CI job to make sure we catch these in the future.
Sometimes it reports false positives, e.g. when a dependency isn't used
directly but in macro-generated code (e.g. with `strum`). I have added
`"ignored"` clauses to the `Cargo.toml` files where appropriate.
V3_GIT_ORIGIN_REV_ID: ed015089b695cec8eeb03ce455d6dd3cd312a016
- Use `rust-toolchain.toml` to specify tools.
- Drop the unused version field from the compose files.
- Use a modern Docker base for the dev-auth-webhook.
- Handle signals in the test servers so `docker stop` is quick.
- Upgrade Jaeger.
V3_GIT_ORIGIN_REV_ID: b5fee1d5dc953a1fdb0aabd75ba02df6846b7518
An upcoming version of ndc-spec will split out this module as a new
crate named `ndc_models`. This PR gets ahead of this change by aliasing
so that we can simply delete the `use` lines in the future.
This is a very boring, mechanical change that does not modify any
behavior at all.
V3_GIT_ORIGIN_REV_ID: 078d3f5fbc2961fcd93b924134de40ba693f0061
I got frustrated by the lack of useful output when services failed to
start in CI, so I thought I'd mess around.
I made some changes so that:
1. we wait for services with health checks to report as healthy,
2. we do not rebuild the engine or custom connector when they start,
3. the health checks work (apart from ndc-postgres, which will come
later), and
4. we use environment variables rather than command-line arguments where
possible.
I have also renamed the "agent" binary to "custom-connector", matching
its crate, because it was driving me a little crazy.
V3_GIT_ORIGIN_REV_ID: 8d672b0b25438b54d47368ce82cd236cfdd4e554
This means that we can pass arguments to the Criterion benchmarks
because the test binaries do not try and run.
For example, if you run this:
```
cargo bench -- --warm-up-time=1 --measurement-time=1
```
… it will currently fail, because it will also pass these arguments to
the tests. Disabling the benchmarks for the tests fixes this.
V3_GIT_ORIGIN_REV_ID: 10fe0683b82c506e1a11b24ca0a0a4041e37b655
## Description
This PR fixes a bug when remote joins is used within nested selections.
The bug is described in #318
Closes: https://hasurahq.atlassian.net/browse/V3ENGINE-20
The fix is to remove the replacement tokens data type. And instead use
each `Argument` as the key to lookup in RHS response and join with LHS
response.
<!--
Questions to consider answering:
1. What user-facing changes are being made?
2. What are issues related to this PR? (Consider adding `(close
#<issue-no>)` to the PR title)
3. What is the conceptual design behind this PR?
4. How can this PR be tested/verified?
5. Does the PR have limitations?
6. Does the PR introduce breaking changes?
-->
## Changelog
- Add a changelog entry (in the "Changelog entry" section below) if the
changes in this PR have any user-facing impact. See [changelog
guide](https://github.com/hasura/graphql-engine-mono/wiki/Changelog-Guide).
- If no changelog is required ignore/remove this section and add a
`no-changelog-required` label to the PR.
### Product
_(Select all products this will be available in)_
- [x] community-edition
- [x] cloud
<!-- product : end : DO NOT REMOVE -->
### Type
<!-- See changelog structure:
https://github.com/hasura/graphql-engine-mono/wiki/Changelog-Guide#structure-of-our-changelog
-->
_(Select only one. In case of multiple, choose the most appropriate)_
- [ ] highlight
- [ ] enhancement
- [x] bugfix
- [ ] behaviour-change
- [ ] performance-enhancement
- [ ] security-fix
<!-- type : end : DO NOT REMOVE -->
### Changelog entry
<!--
- Add a user understandable changelog entry
- Include all details needed to understand the change. Try including
links to docs or issues if relevant
- For Highlights start with a H4 heading (#### <entry title>)
- Get the changelog entry reviewed by your team
-->
Fix bug when remote joins are used within nested selections
<!-- changelog-entry : end : DO NOT REMOVE -->
<!-- changelog : end : DO NOT REMOVE -->
---------
Co-authored-by: Gil Mizrahi <gil@hasura.io>
V3_GIT_ORIGIN_REV_ID: 3c0648f5fb21eb4f1d6cc004db2f3ee61a731c38
<!-- Thank you for submitting this PR! :) -->
## Description
This moves all the crates into a `/crates` folder. Everything appears to
just work, thanks Cargo!
V3_GIT_ORIGIN_REV_ID: 8e3ef287b1a46cabdb4d919a50e813ab2cddf8b1
