Commit Graph

71 Commits

Author SHA1 Message Date
Abhinav Gupta
180c1dbc59 Refactor SQL layer to use OpenDD query IR (#925)
As per the multiple frontends RFC:
https://github.com/hasura/v3-engine/blob/vamshi/multiple-frontends/rfcs/multiple-frontends.md

V3_GIT_ORIGIN_REV_ID: 07f7c5323179a62fd08717d6d49f9415da139873
2024-08-05 23:38:19 +00:00
Vamshi Surabhi
4aefdabb65 avoid using raw Strings in more places (#923)
- `DataConnectorAggregationFunctionName` and `AggregateFunctionName` now
use `str_newtype`.
- All usages of `String`s for subgraph names are removed.

(This is part of a larger effort to remove references in
`execute::plan::QueryPlan`).

V3_GIT_ORIGIN_REV_ID: d51f0a2335e8dabbc9efdad1d1efff285ddb74c3
2024-08-05 22:27:47 +00:00
Rakesh Emmadi
9bf0ad967f Query usage analytics | include deprecated info in field usage (#932)
<!-- The PR description should answer 2 important questions: -->

### What

<!-- What is this PR trying to accomplish (and why, if it's not
obvious)? -->

<!-- Consider: do we need to add a changelog entry? -->

<!-- Does this PR introduce new validation that might break old builds?
-->

<!-- Consider: do we need to put new checks behind a flag? -->
When reporting query usage analytics, mention whether a field is
deprecated.

Note: The changelog is not required, as the usage analytics are for
Hasura internal use.

### How

<!-- How is it trying to accomplish it (what are the implementation
steps)? -->
OpenDd allows marking an ObjectType's field as deprecated with an
optional reason. Plumb the deprecation context to the input/output
schema annotation. Report the field usage with a deprecated boolean
field.

V3_GIT_ORIGIN_REV_ID: 430cdcf3e1ff0c43812caecb8d06a64b729665be
2024-08-05 13:13:01 +00:00
Daniel Harvey
e7462f7884 Tidy boolean expression schema generation (#920)
<!-- The PR description should answer 2 important questions: -->

### What

Trying to understand what is going on here. Still no closer, but have
added a test and made some types more specific in order to clarify my
understanding.

<!-- What is this PR trying to accomplish (and why, if it's not
obvious)? -->

<!-- Consider: do we need to add a changelog entry? -->

<!-- Does this PR introduce new validation that might break old builds?
-->

<!-- Consider: do we need to put new checks behind a flag? -->

### How

Add some introspection tests for relationships with
`ObjectBooleanExpressionType`s to ensure they generate. Tried to make
relationship fields disappear to recreate build problems but could not.

Split `BooleanExpressionGraphqlConfig` and
`ObjectBooleanExpressionGraphqlConfig` to make sure we're not mixing
them up. We only want to use `BooleanExpressionGraphqlConfig` in
`metadata_resolve`, this ensures that.

Pushed some partiality in `schema/boolean_expressions.rs` out - a
function was `Option<inputs> -> Option<outputs>` and now it's `inputs ->
outputs`. We use `Option` a lot and it makes reasoning why something
hasn't been added to the schema difficult.

<!-- How is it trying to accomplish it (what are the implementation
steps)? -->

V3_GIT_ORIGIN_REV_ID: 893e6f32bfded14ea724be7eaedc519e264f4c01
2024-08-02 13:04:10 +00:00
Daniel Chambers
63732fe7be Bug fixes around argument presets in the DataConnectorLink (#866)
This PR fixes the following bugs:
- Fixes a bug where models and commands were allowed even though they
did not define arguments to satisfy the underlying data connector
collection/function/procedure. **UPDATE:** This only raises a warning
rather than fails the build, because existing builds on staging and
production have this issue. This will need to be transitioned to an
error once the Compatibility Date plumbing is in place.
- Fixes a bug where argument presets set in the DataConnectorLink were
sent to every connector function/procedure regardless of whether the
function/procedure actually declared that argument
- Fixes a bug where argument presets set in the DataConnectorLink were
not sent to connector collections that backed Models
- Fixes a bug where the type of the argument name in the
DataConnectorLink's argument presets was incorrect in the Open DD
schema. It was `ArgumentName` but should have been
`DataConnectorArgumentName`
- Fixes a bug where the check to ensure that argument presets in the
DataConnectorLink does not overlap with arguments defined on
Models/Commands was comparing against the Model/Command argument name
not the data connector argument name

There are a number of changes that tighten things up in this PR.
Firstly, the custom connector is improved so that it rejects requests
with arguments of the wrong type or unexpected arguments. This causes
tests that should have been failing to actually fail. Then, new tests
have been added to metadata_resolve to cover the untested edge cases
around data connector link argument presets.

Then, metadata resolve is refactored so that the link argument presets
are validated and stored on each command/model source, rather than on
the DataConnectorLink. Extra validation has been added during this
process to fix the above bugs. Any irrelevant argument presets to the
particular command/model are dropped. Then, during execution, we read
the presets from the command/model source instead of from the
DataConnectorLink, which ensures we only send the appropriate arguments.

JIRA: [V3ENGINE-290](https://hasurahq.atlassian.net/browse/V3ENGINE-290)
Fixes
https://linear.app/hasura/issue/APIPG-676/dataconnectorlink-argument-presets-are-always-sent-regardless-of

[V3ENGINE-290]:
https://hasurahq.atlassian.net/browse/V3ENGINE-290?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

V3_GIT_ORIGIN_REV_ID: dd02e52e1ff224760c5f0ed6a73a1ae56779e1f1
2024-08-02 09:25:45 +00:00
Daniel Harvey
66e847bc46 Move "test" job to Github Actions (#872)
<!-- The PR description should answer 2 (maybe 3) important questions:
-->

### What

We've had our CI mixed between Github and Buildkite for a while, it's
time to commit. First step is moving the "tests" step to Github Actions.

<!-- What is this PR trying to accomplish (and why, if it's not
obvious)? -->

<!-- Consider: do we need to add a changelog entry? -->

### How

This PR:

- Moves the `test` step to Github Actions
- Creates a new `custom_connector.Dockerfile` which builds custom
connector only, more quickly.
- Changes the metadata tests to use `localhost` instead of their Docker
internal names (ie `custom_connector` or `postgres_connector`) - this is
because the tests are being run from outside Docker now
- Removes the `test` Buildkite step

It does not:

- Remove the code coverage or benchmarks steps from Buildkite
- Tidy up `justfile` or Dockerfiles

<!-- How is it trying to accomplish it (what are the implementation
steps)? -->

---------

Co-authored-by: Philip Lykke Carlsen <plcplc@gmail.com>
V3_GIT_ORIGIN_REV_ID: a67534ebc1634a24b48d2620c45003221852e199
2024-07-24 13:41:40 +00:00
Daniel Harvey
f2a12f492c Fix manually passed boolean expression arguments (#869)
<!-- The PR description should answer 2 (maybe 3) important questions:
-->

### What

When a boolean expression is passed as an argument it was not being
translated into an `ndc_models::Expression` and so queries failed.

<!-- What is this PR trying to accomplish (and why, if it's not
obvious)? -->

<!-- Consider: do we need to add a changelog entry? -->

### How

Look up the type of an argument in `metadata-resolve`, and mark the
`ArgumentInfo` with a new `ArgumentKind`. Then in IR step we use that to
work out whether to turn the argument to JSON as before, or translate it
into an `Expression` type that will eventually be turned into an
`ndc_models::Expression`.

<!-- How is it trying to accomplish it (what are the implementation
steps)? -->

V3_GIT_ORIGIN_REV_ID: 4da3ce0ae04895c33de2b6bdb6fff1018c39b3ad
2024-07-23 13:36:18 +00:00
Daniel Harvey
8e79e53b63 Raise warnings during metadata-resolve (#859)
<!-- The PR description should answer 2 (maybe 3) important questions:
-->

### What

Sometimes our builds succeed, but we'd like to tell the user how they
could do better. This implements the simplest possible warnings system.

<img width="957" alt="Screenshot 2024-07-18 at 16 06 49"
src="https://github.com/user-attachments/assets/ff91d221-667a-43f9-bc8a-51bf4574a7b8">

<!-- What is this PR trying to accomplish (and why, if it's not
obvious)? -->

<!-- Consider: do we need to add a changelog entry? -->

### How

Warnings are printed to stdout on `v3-engine` startup, and will be
returned to the CLI via `v3-metadata-build-service`. The diff is mostly
updated snapshot tests as we're returning more from
`metadata-resolve::resolve` now.

<!-- How is it trying to accomplish it (what are the implementation
steps)? -->

V3_GIT_ORIGIN_REV_ID: d01520e53f49d9b594e94a4531b6a86e749875c3
2024-07-23 10:01:21 +00:00
Daniel Harvey
ab1d963de0 Split ValueExpression (#812)
### What
`ValueExpression` contained boolean expressions, and was used in places
where boolean expressions were not allowed.

### How
This creates a new type `ValueExpressionOrPredicate`, and uses it in the
places where boolean expressions are allowed.

---------

Co-authored-by: Daniel Chambers <daniel@hasura.io>
V3_GIT_ORIGIN_REV_ID: c0a07c5e0096aeb4369ca7d6b5147451d1ccd14d
2024-07-10 02:30:09 +00:00
Daniel Harvey
83df9e81bb Move GraphqlConfigError to graphql_config stage (#808)
<!-- The PR description should answer 2 (maybe 3) important questions:
-->

### What

We have a giant error file in `metadata-resolve`, and it's really
unclear what can go wrong where. Let's improve it in some small way.

<!-- What is this PR trying to accomplish (and why, if it's not
obvious)? -->

<!-- Consider: do we need to add a changelog entry? -->

### How

Move `GraphqlConfigError` to the `graphql_config` stage, and make that
stage only return that kind of error.

Functional no-op.

<!-- How is it trying to accomplish it (what are the implementation
steps)? -->

---------

Co-authored-by: Samir Talwar <samir.talwar@hasura.io>
V3_GIT_ORIGIN_REV_ID: 9aa58875a24d66e6945c9ead6434fedb124f1dd8
2024-07-09 10:02:40 +00:00
Daniel Harvey
29ccf96784 Skip capability check for local relationships (#805)
<!-- The PR description should answer 2 (maybe 3) important questions:
-->

### What

If the target data connector for a relationship does not have the
`foreach` capability, we threw an error, however we should allow this.

<!-- What is this PR trying to accomplish (and why, if it's not
obvious)? -->

<!-- Consider: do we need to add a changelog entry? -->

### How

<!-- How is it trying to accomplish it (what are the implementation
steps)? -->

Check data connector names for relationship target before throwing
capability error.

V3_GIT_ORIGIN_REV_ID: 157908f0077b7c3af67a77600e5f8c9fc65df7f2
2024-07-08 13:03:13 +00:00
Daniel Chambers
e44589931c Reimplement most OpenDD newtypes using SmolStr and a macro (#797)
This PR replaces most of the string newtypes in the open-dds crate with
new implementations based on `SmolStr`, using a macro adapted from the
macro used in ndc-models. `SmolStr` usage should result in less heap
allocations and the macro ensures all the newtypes get all the same
trait implementations (such as various `From` and `Borrow` instances)
and helper impl functions (such as `as_str`).

The new macro, `str_newtype`, creates a newtype wrapper around `SmolStr`
or another type (typically `Identifier`), and writes out all the various
trait implementations. It also takes a documentation string which
ensures that every newtype has a description in JSON Schema.

The changes in the downstream crates are just adjusting to use the new
newtypes.

Also:
* `QueryGraphqlConfig` used a lot of String typed-properties; these have
been changed for the appropriate `GraphQlTypeName` and
`GraphQlFieldName` types.
* metadata-resolve had a duplicate newtype called
`ConnectorArgumentName`, which duplicated open-dds's
`DataConnectorArgumentName`. This has been removed and replaced.
* A new newtype called `CollectionName` has been added in open-dds to
represent the name of the ndc collection that a model points to. This
was previously just a string.
V3_GIT_ORIGIN_REV_ID: d5a33756ef0e110b2255478358a38e42e6ff89a2
2024-07-05 10:16:33 +00:00
Anon Ray
c1b9592f6b respect relationship comparison capability when generating relationship fields in model filter (#789)
### What

When generating GraphQL schema for relationship fields in model filter,
engine ignores `relation_comparisons` capability of the data connector.
Engine would generate schema for data connectors which don't have this
capability. This PR fixes that.

### How

While generating fields for the filter input type, take the relationship
capabilities into account.

The `ObjectBooleanExpressionType` and `BooleanExpressionType` objects
are quite different, hence their schema generation part is also
different, and is split in two different functions
(`build_comparable_relationships_schema`, and
`build_new_comparable_relationships_schema`). Added checking of
relationship comparison capability in both the functions.

V3_GIT_ORIGIN_REV_ID: dce2b88f7792e01e5bb390ecdb580e223ec80f01
2024-07-02 07:49:10 +00:00
Rakesh Emmadi
5efbb09500 Unify get_underlying_type_name and get_base_type functions (#788)
### What
This is a no-op refactor that involves unifying `get_base_type` and
`get_underlying_type_name` functions, whose motives are the same.

### How
Replace the name of erstwhile `get_base_type` fn with
`get_underlying_type_name`. Remove the latter. Update the rest of the
code to use the new function.

V3_GIT_ORIGIN_REV_ID: 0b0d999670641ba265fde153af9b43b4d865e215
2024-07-01 12:57:50 +00:00
Rakesh Emmadi
9e98ed2310 Implement query usage analytics (#713)
JIRA: https://hasurahq.atlassian.net/browse/V3ENGINE-176

This PR implements usage analytics
([RFC](https://docs.google.com/document/d/1k8_ZniiejxHhD0SphtEJ2mg6MoJON0dkk0q4b5WZWDE/edit))
for a GraphQL query when a request is made. The analytics data JSON is
attached to the span, whose details are provided below:
Span name - `execute`
Attribute - `internal.query_usage_analytics = <JSON-string>` OR
`internal.query_usage_analytics_error = <String>`

Refs:
- [Link](https://github.com/hasura/v3-engine/pull/606) to the PR, where
data types for usage analytics were added.
- [Link](https://github.com/hasura/v3-engine/pull/715) to the PR, where
some improvements are made to the analytics data types.

Note: Changelog entry is added.
V3_GIT_ORIGIN_REV_ID: f4089f3354b638533ff72fbec72951e883c31100
2024-07-01 12:10:12 +00:00
Samir Talwar
291d564d82 Support for partial supergraphs in metadata resolution. (#772)
### What

This adds a flag, `--partial-supergraph`, which instructs the metadata
resolver to prune relationships to unknown subgraphs rather than failing
to resolve.

### How

The flag gets passed through as
`metadata_resolve::configuration::Configuration`, and known subgraphs
are now tracked in `MetadataAccessor`. If the flag is set and a
relationship target refers to an unknown subgraph, we return an empty
list of relationships instead of failing.

Some test infrastructure has been added to set configuration flags per
test.

V3_GIT_ORIGIN_REV_ID: 6f0de2442a3bfc7c7a4c48e3dc7296dc1538cd67
2024-06-27 14:17:31 +00:00
Samir Talwar
eb1dbffbd1 Use SubgraphIdentifier in the metadata accessor, and SmolStr internally. (#768)
### What

Using `String` everywhere to represent subgraph identifiers is
definitely going to cause problems at some point. We can avoid this by
using `open_dds::identifier::SubgraphIdentifier` instead.

`Identifier` and `SubgraphIdentifier` were wrappers around `String`. I
have also changed them to wrap `SmolStr` instead, for better performance
as they're cloned a lot.

### How

First of all, I hid the innards of `Identifier` and
`SubgraphIdentifier`, instead exposing certain behaviors as methods.
Mostly, this means exposing `as_str()` and `to_string()`.

Then I replaced the internals with `SmolStr`. I had to change one place
where `RefCast` was used, but that was pretty much it.

Finally, I switched out `String` for `SubgraphIdentifier` in
`QualifiedObject`. This necessitated a couple of constants for the two
"magic" subgraphs, `__globals` and `__unknown_namespace`, but was
otherwise fine.

I didn't touch `Qualified` for now.

V3_GIT_ORIGIN_REV_ID: 28664609c3173b181c3789093cb9796896642eb7
2024-06-26 22:04:22 +00:00
Daniel Chambers
0624a7553d Aggregate relationships GraphQL schema and execution (#756)
## Description
This PR implements the GraphQL schema and execution for aggregate
relationships.

In the `schema` crate, the new `model_aggregate_relationship_field`
function handles generating schema for ModelAggregateTarget
relationships. It mostly delegates the meat of its implementation to
reused logic; some refactoring has occurred to make this possible.
This involved changes in `select_many`, `select_aggregate` and
`model_arguments`. The creation of the model arguments field argument
now exists in `model_arguments` and is reused by `select_many` and
`select_aggregate`. The creation of all aggregate field arguments is now
in `select_aggregate::generate_select_aggregate_arguments`, and is then
reused when generating the aggregate relationship field. That field is
annotated with the new `RelationshipToModelAggregate` annotation.

In the `execute` crate, the logic around generating an the aggregate
selection IR was moved from `select_aggregate` into `model_selection`.
This was so it can be reused by the logic in `relationship` that now
uses it to generate an aggregate selection when encountering an
`RelationshipToModelAggregate` field.
Inside `relationship` some rearranging was done so that
`build_local_model_relationship` and `build_remote_relationship` could
work with either a normal model selection IR or the new aggregate
selection IR. The necessitated moving the creation of that IR outside
those functions into the caller, so the different callers can create
different IR (normal vs aggregate IR). This also reduced code
duplication.

New tests have been added to `engine` that cover aggregate relationships
and also remote joined aggregate relationships.

This PR also corrects two bugs in metadata resolve revealed by new
testing:
* The filter input field name in `GraphqlConfig` must be specified if
using an aggregate relationship
* The filter input type name defined on a `Model` must be specified if
that model is the target of an aggregate relationship. Conversely, the
filter input type name can be specified if the `Model` itself doesn't
define an aggregate, but is still involved in a aggregate relationship
(this previously produced an error).

This PR completes the feature, but it is still hidden behind the
experimental flag. There will be a follow up PR to remove that and
expose the functionality by default.

JIRA: [V3ENGINE-160](https://hasurahq.atlassian.net/browse/V3ENGINE-160)

[V3ENGINE-160]:
https://hasurahq.atlassian.net/browse/V3ENGINE-160?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

V3_GIT_ORIGIN_REV_ID: d499371906f7af71a4017c7c3ae75b7693cd3fa7
2024-06-26 09:10:33 +00:00
Samir Talwar
8baa50010e Make the internal metadata-resolve flags copyable. (#762)
### What

This just means we can drop a bunch of references and pass by value.

### How

`#[derive(Clone, Copy)]` and fixing lints.

V3_GIT_ORIGIN_REV_ID: e15d323f8232755294d1f7a2c70ccf0de8a1632f
2024-06-26 02:27:41 +00:00
Daniel Harvey
cedd4e56c6 Use new boolean expressions in arguments (#753)
<!-- Thank you for submitting this PR! :) -->

## Description

Adding a test for this and fixing a few missing parts. Mostly threading
`boolean_expression_types` everywhere and adding them to our type
lookups. Behind a feature flag so this is a functional no-op.

V3_GIT_ORIGIN_REV_ID: 5fd6d5b9e06f0216e770b0715c59c0479881017f
2024-06-25 14:35:07 +00:00
Anon Ray
d96bb22844 forward headers from commands as response headers (#707)
## Description

According to NDC headers pass-through spec, commands can include headers
in their responses, which are forwarded as response headers by the
engine to the client. This PR implements it.

V3_GIT_ORIGIN_REV_ID: 4fe458db02c5dd51f4674e4e013312f8e179c087
2024-06-25 07:27:05 +00:00
Samir Talwar
1378730d43 Remove redundant clones. (#752)
I noticed a few extra calls to `.clone()` while working on an unrelated
refactor. I want to remove them for brevity and simplicity; I don't
expect a performance improvement.

This turns on the Clippy warning `redundant_clone`, which detects
unnecessary calls to `.clone()` (and `.to_string()`).

It is an unstable warning and so might reports some false positives. If
we find any, we can suppress the warning there.

V3_GIT_ORIGIN_REV_ID: a713f29cf862d6f4cb40300105c6b9f96df00676
2024-06-24 14:09:02 +00:00
Daniel Harvey
eba79698d5 Ban println! in code with clippy (#711)
<!-- Thank you for submitting this PR! :) -->

## Description

A few debug lines slipped in recently, let's make `clippy` `warn` on
those, so they are kicked out by CI. Functional no-op.

V3_GIT_ORIGIN_REV_ID: 290f6de35f9315b68811eb5f15969fb0333e9d06
2024-06-24 11:46:53 +00:00
Daniel Harvey
995339bbf6 Split models resolve stage (#741)
<!-- Thank you for submitting this PR! :) -->

## Description

To check filter expression types properly, we need the resolved `source`
for all models. Previously we did not have this, as we did all model
resolving in one go. This splits that into stages, the existing `models`
resolves models and their sources, and then `models_graphql` resolves a
Model's filter expressions, aggregations and graphql schema. This means
we can inspect the model sources during the `models_graphql` in the
actual PR we want to do here.

It's massive, but it's a functional no-op, I promise.

V3_GIT_ORIGIN_REV_ID: 6c8fbf0763fb1daa209a4c84855609e8e9372e8b
2024-06-24 02:46:25 +00:00
Daniel Chambers
acf357c687 Refactor object_type_fields and RelationshipTarget (#737)
## Description
This PR refactors the `output_type::object_type_fields` function, which
is a monster, and splits out the handling of relationship fields into a
separate function. This function then itself is broken down into
separate functions that each handle one of the different types of
relationship fields.

This is particularly necessary now, as logic to handle
ModelAggregateRelationshipTargets is about to get added and that would
only grow the monster function more.

The `RelationshipTarget` `metadata_resolve` type has had its enum
variant structs broken out into separate struct types so that they can
be passed around separate to the enum. This makes it easier to write
functions that deal with each enum variant separately (like
`output_type::object_type_fields`).

This PR causes no behavioural changes.

V3_GIT_ORIGIN_REV_ID: 4707f8b07154bd4d503f78392a3d2d8976eab339
2024-06-21 12:09:41 +00:00
Daniel Harvey
b658c97a07 Allow specifying bool exp for relationship (#739)
<!-- Thank you for submitting this PR! :) -->

## Description

When doing relationships across bool exps, by default we use the target
model's `where` clause bool exp. However, we allow the user to provide a
different bool exp instead, which we now pay attention to.

Behind a feature flag so a user-facing no-op.

V3_GIT_ORIGIN_REV_ID: b31157c455d56375c35db28f2418da9d41b68c46
2024-06-21 11:04:55 +00:00
Daniel Harvey
99954495d5 Obey is_null and logical_operators (#733)
<!-- Thank you for submitting this PR! :) -->

## Description

Previously we ignored `isNull` and `logicalOperators`. Now we obey
`isNull` for scalars, and `logicalOperators` for objects.

Both `isNull` on objects and `logicalOperators` on scalars don't exist
as features, so will create follow up tickets for these.

No new tests, but I was able to make both things fail by disabling the
appropriate options:

<img width="655" alt="Screenshot 2024-06-19 at 16 59 33"
src="https://github.com/hasura/v3-engine/assets/4729125/47eb342d-49c1-4ec8-8bfa-5e3f2d286928">

<img width="758" alt="Screenshot 2024-06-19 at 16 30 08"
src="https://github.com/hasura/v3-engine/assets/4729125/5722407c-e12e-429a-ab3a-fc221a8addfa">

Forgive me, I also broke up the `stages/boolean_expressions/object`
module into a few pieces to make this easier to think about.

This is behind a feature flag, no op.

V3_GIT_ORIGIN_REV_ID: b1273d8a9655a21adedc7c7a04a96e0cfe98cc98
2024-06-21 10:01:34 +00:00
Daniel Chambers
b1ee3ae1b8 Metadata resolve for aggregates over relationships (#731)
## Description
This PR continues on from #725 and adds the metadata resolve logic to
validate the usage of aggregates applied to relationships. The metadata
resolve logic is gated behind a new `enable_aggregate_relationships`
flag and disabled by default.

The new resolve logic lives in
`crates/metadata-resolve/src/stages/relationships/mod.rs`, however, most
of the actual logic that validates the usage of the aggregate expression
with the relationship has been reused from the model aggregate resolve
code. Subsequently, that code
(`crates/metadata-resolve/src/stages/models/aggregation.rs`) was
refactored to return a distinct error type
`ModelAggregateExpressionError` that can be composed with the
`RelationshipError` type, so the same errors can be returned with the
additional context of the relationship they were found in.

New metadata resolve error tests have been added in
`crates/metadata-resolve/tests/failing/aggregate_expression_in_relationship/*`.
Also, two new passing metadata resolve tests have been added to cover
the happy case of root field and relationship aggregate expressions:
`crates/metadata-resolve/tests/passing/aggregate_expressions/*`

JIRA: [V3ENGINE-160](https://hasurahq.atlassian.net/browse/V3ENGINE-160)

[V3ENGINE-160]:
https://hasurahq.atlassian.net/browse/V3ENGINE-160?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

V3_GIT_ORIGIN_REV_ID: 8cd7040cc9277940641d5ac239d7b34f8c1462c5
2024-06-21 01:08:54 +00:00
Samir Talwar
783aec942d Move all dependency versions into the workspace file. (#734)
This keeps versions in one place so we can more easily ensure we upgrade
crates together.

V3_GIT_ORIGIN_REV_ID: 6a929bb6196c19a1f66a768585b669127035e9be
2024-06-20 12:25:55 +00:00
paritosh-08
2da4637baa add field arguments in schema and execution (#695)
### TODO
- [x] Validate the presence of arguments in the data connector schema
- [x] Add test for IR
- [x] Add test using custom connector

## Description

JIRA: https://hasurahq.atlassian.net/browse/V3ENGINE-148

This is part 2 of the field arguments.

This will add the field arguments to the graphql schema and also handles
the execution part.

![image](https://github.com/hasura/v3-engine/assets/85472423/c8f55d70-6539-42ef-ab5a-91b74e5699e2)

## 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
-->

Add support for field arguments

<!-- changelog-entry : end : DO NOT REMOVE -->

<!-- changelog : end : DO NOT REMOVE -->

---------

Co-authored-by: Anon Ray <ecthiender@users.noreply.github.com>
Co-authored-by: Samir Talwar <samir.talwar@hasura.io>
V3_GIT_ORIGIN_REV_ID: ea06b896feaca30419f81cd070fd4cfd608ef35b
2024-06-19 08:15:09 +00:00
Samir Talwar
5ad33bd6a5 Avoid returning a Result when it is unnecessary. (#726)
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
2024-06-18 12:03:04 +00:00
Daniel Harvey
7968975c95 Comparable relationships in BooleanExpressionTypes (#717)
<!-- Thank you for submitting this PR! :) -->

## Description

This implements local relationships in the new `BooleanExpressionType`.
We copy all the existing tests for this and reimplement them using the
new types.

Things currently missing:
- a `BooleanExpressionType` can be specified to overwrite the target
model's `where` clause when defining. Currently we ignore this, there is
a follow up ticket for this:
https://hasurahq.atlassian.net/browse/V3ENGINE-209

This work is behind a feature flag, so this is a no-op for the user.

V3_GIT_ORIGIN_REV_ID: 6ae87e1ffb2ad0ebd27c6097e8ea87aca1a4c299
2024-06-18 10:25:04 +00:00
dependabot[bot]
9cf56ca622 Bump serde_json from 1.0.116 to 1.0.117 (#722)
Bumps [serde_json](https://github.com/serde-rs/json) from 1.0.116 to
1.0.117.

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
V3_GIT_ORIGIN_REV_ID: 274e7a3dc975113e95ff3b10ce6b73385f0b6ecc
2024-06-17 05:13:47 +00:00
Daniel Harvey
0f245de566 Defer local relationship check until IR creation (#716)
<!-- Thank you for submitting this PR! :) -->

## Description

Making the schema work for boolean expressions has been challenging as
they are no longer tied to a single data connector, so we cannot do any
checks of whether a relationships is local or remote. We defer this to
the IR step when the source data connector for a relationship is known,
so that we can generate schema for boolean expressions decoupled from
any concept of data connector.

Functional no-op.

V3_GIT_ORIGIN_REV_ID: d2923acedf92b031ba092bb83c515812c4d346f0
2024-06-13 15:26:25 +00:00
Samir Talwar
5df3e6fac4 Inline format arguments into the format string when possible. (#703)
V3_GIT_ORIGIN_REV_ID: 50d6a12eefbfcc6b217d226759856e957fac0f4b
2024-06-12 11:25:38 +00:00
Daniel Chambers
1c5008df7c Aggregates Root Field - Part 3: GraphQL API (#685)
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
2024-06-12 09:02:12 +00:00
Philip Lykke Carlsen
b3641238a9 Simplify namespaced AllowAll (#702)
<!-- Thank you for submitting this PR! :) -->

## Description

It turns out that every usage of `Builder::allow_all_namespaced`
concerned itself with either introspection nodes or (in the GDS case)
`None`.

Combined with the fact that both the SDL and GDS implementations of
`SchemaContext::introspection_namespace_node` were trivial, the
`NamespacedNodeInfo` contained in
`Namespaced::AllowAll(S::NamespacedNodeInfo)` is always functionally
meaningless.

This PR removes the `NamespacedNodeInfo` from `Namespaced::AllowAll` as
well as `SchemaContext::introspection_namespace_node`, thus simplifying
the implementation.

<!--
  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: 1525905488203ff07182aa700c9fff26512743ab
2024-06-11 17:08:41 +00:00
Daniel Harvey
e0b8d67309 Break up src/schema/boolean_expressions.rs somewhat (#701)
<!-- Thank you for submitting this PR! :) -->

## Description

The level of nesting in this file makes it hard to follow, this breaks
the deepest nesting into a function. Functional no-op.

V3_GIT_ORIGIN_REV_ID: 03777bdae0c772754f5ca370dcc3d9577abc9c80
2024-06-11 16:09:09 +00:00
Samir Talwar
0c6d6a67d2 Semicolons, everywhere! (#700)
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
2024-06-11 15:33:32 +00:00
Daniel Harvey
75ced29d11 Resolve nested object boolean expressions (#680)
<!-- Thank you for submitting this PR! :) -->

## Description

This adds the ability to describe nested object boolean expressions,
which become `fieldPath` items in the generated
`ndc_models::ComparisonTarget::Column` items. This allows us to describe
filtering a `User` based on some element in their nested `address` field
(like `postcode`, for example).

Like the other `BooleanExpressionType` work, this remains behind a
feature flag so should make no user-facing changes.

It is also missing a whole heap of metadata resolve checks, going to
follow with these after doing the happy path to unblock other work.

V3_GIT_ORIGIN_REV_ID: c89e2942a651d349fca97706affcf40d91afeefb
2024-06-11 12:57:21 +00:00
Philip Lykke Carlsen
c8d89a8ad8 Remove namespace arguments (#691)
<!-- Thank you for submitting this PR! :) -->

## Description

With the introduction of `NamespacedGetter` as the means deciding how to
interpret what it means to extract "namespaced" data from a schema it is
superfluous to thread along the "namespace" that lookups will be based
on.

This PR removes those namespace arguments.

As a nice side effect this removes the cases where we used to have to
supply a dummy role value when generating a role-agnostic schema.

<!--
  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: 0aca2b3838f1b0d944b3c41ddd54d20dc74503a4
2024-06-10 21:01:34 +00:00
dependabot[bot]
05e13fe566 Bump serde from 1.0.202 to 1.0.203 (#687)
Bumps [serde](https://github.com/serde-rs/serde) from 1.0.202 to 1.0.203.

V3_GIT_ORIGIN_REV_ID: 81b23526c9a289d401260350b8169907eb322bbf
2024-06-10 06:26:06 +00:00
Daniel Harvey
d090331be2 Factor source_data_connector out of relationship (#679)
<!-- Thank you for submitting this PR! :) -->

## Description

We'd like not to tie our boolean expressions to one particular data
connector, we were doing this in relationships and had no need to.
Functional no-op.

V3_GIT_ORIGIN_REV_ID: 0af16ad92117a45ac1f424b49d9f13184787f63f
2024-06-06 15:14:02 +00:00
Daniel Harvey
58f939e605 Use OperatorMappings in boolean expressions (#674)
Previously we required an operator called `_eq` to be called `_eq` in
all data connectors that used it.

Now we can give them nice names, and map each one to a different
operator name per data connector.

V3_GIT_ORIGIN_REV_ID: 1d2f678e25d932e5ea45d34999cf5709ef0038b4
2024-06-06 11:31:36 +00:00
Rakesh Emmadi
49e0c79d9e avoid infinite recursion in building annotations from input types (#676)
## Description
JIRA: https://hasurahq.atlassian.net/browse/V3ENGINE-180

Fix an edge case where the function
`build_annotations_from_input_object_type_permissions` produce infinite
recursions
when an input object type contains fields with self-referential types.

Note: This PR doesn't contain any tests. They'll be added in a separate
PR ([JIRA](https://hasurahq.atlassian.net/browse/V3ENGINE-188)). This
fix needs to be done quickly and merged as other teams (graphql
connector) are blocked by this.
V3_GIT_ORIGIN_REV_ID: 4cc09cb96871804d4daf83d7e49a6f4529c1943d
2024-06-06 08:16:33 +00:00
Daniel Harvey
2cd613077a Make very happy path BooleanExpressionType work (#673)
<!-- Thank you for submitting this PR! :) -->

## Description

This makes the absolute happiest path `BooleanExpressionType` work.

Caveats:

- We ignore operator mappings, so we assume that the OpenDD operator
name (ie, `_eq`) is the same as the data connector's name for it.

- We have not tried to implement `comparisonRelationships` in any way
yet.

- Have not tried nested objects yet.

All functionality here is behind a feature flag so has no user-facing
impact.

V3_GIT_ORIGIN_REV_ID: 5edb706c6d8b03b9fa59433ce24f05f37a69729a
2024-06-05 16:18:28 +00:00
Daniel Harvey
b314d89ff0 Use newtypes for operator names (#672)
<!-- Thank you for submitting this PR! :) -->

## Description

We were using `String` for these, and making some assumptions about NDCs
and OpenDD operator names being the same. This uses newtypes and
documents where these assumptions are being made. Functional no-op.

V3_GIT_ORIGIN_REV_ID: 195e55db6fadf48c956684130e23f647eaa17fb1
2024-06-05 14:46:05 +00:00
Samir Talwar
ca830c05fb Be more explicit about clones, and remove a few. (#671)
Calling `.to_owned()` on a reference, `.to_vec()` on a vector reference,
etc. are just synonyms for `.clone()` which are less explicit about
cloning. Let's be explicit.

This also removes some unnecessary clones.

V3_GIT_ORIGIN_REV_ID: 1bc00c4106f0346303d73e4268c89030c0ce93fc
2024-06-05 13:26:33 +00:00
Anon Ray
8064c292b5 upgrade to ndc v0.1.3 (#629)
Upgrade engine to ndc v0.1.3.

Also updates custom connector.

---------

Co-authored-by: Samir Talwar <samir.talwar@hasura.io>
Co-authored-by: Gil Mizrahi <gil@hasura.io>
V3_GIT_ORIGIN_REV_ID: 80959d4702ef785b7809d6432e092b6477ef88c1
2024-06-05 10:16:11 +00:00
Samir Talwar
7281322f51 Enable a few lint warnings around conditions, and fix the highlighted areas. (#662)
1. Use `map_or(…, …)` instead of `.map(…).unwrap_or(…)`.
2. Use `.is_some_and(…)` instead of `.map(…).unwrap_or_default(…)`.
3. Nest `|` patterns where possible.
4. Be more specific about match patterns.

I found I could also simplify `typecheck_qualified_type_reference`
considerably.

V3_GIT_ORIGIN_REV_ID: 6a3b1a4c525c0187c2fdb6df0c979ca0b7b3016c
2024-06-05 08:42:03 +00:00