Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add aggregate expression types to fixture metadata #95

Merged
merged 8 commits into from
Aug 13, 2024

Conversation

hallettj
Copy link
Collaborator

@hallettj hallettj commented Aug 8, 2024

Once again, this only affects integration tests and the local dev environment.

Adds aggregation expression types for everything that looks it should be aggregatable. The connector isn't advertising support for nested field aggregates yet so the nested field aggregates are commented out for now. I filed NDC-386 to follow up on that. Everything looks good in ad-hoc testing. I'm planning to follow up with integration tests shortly.

Completes NDC-381

@hallettj hallettj self-assigned this Aug 8, 2024
Comment on lines +71 to +74
aggregatedType: Int
aggregationFunctions:
- name: _avg
returnType: Int
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return type for averages over Int should be Float. We need to fix that in the connector capabilities. I filed an issue to follow up on this, https://linear.app/hasura/issue/NDC-385/return-type-for-average-of-ints-should-be-float-not-int

Copy link
Contributor

@daniel-chambers daniel-chambers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me!

You should also consider adding aggregates to your relationships as well, so you can get Customer.Invoices_aggregate-style fields.

@hallettj hallettj merged commit 838ed55 into main Aug 13, 2024
1 check passed
@hallettj hallettj deleted the jesse/aggregate-configs-in-fixtures branch August 13, 2024 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants