这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@ecthiender
Copy link
Contributor

Description

Correct conversion to internal scalar type while merging to remote schema scalar with hasura scalars.

What component does this PR affect?

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Requires changes from other components? If yes, please mark the components:

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Related Issue

#1244

Solution and Design

Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Docs update
  • Community content

Checklist:

  • I have read the contributing guide and my code conforms to the guidelines.
  • This change requires a change in the documentation.
  • I have updated the documentation accordingly.
  • I have added required tests.

@ecthiender ecthiender requested a review from 0x777 January 28, 2019 14:08
@ecthiender ecthiender added the c/server Related to server label Jan 28, 2019
@hasura-bot
Copy link
Contributor

Review app for commit 7edf82a deployed to Heroku: https://hge-ci-pull-1497.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1497-7edf82a

@shahidhk shahidhk merged commit 39bc3ac into hasura:master Jan 28, 2019
@hasura-bot
Copy link
Contributor

Review app https://hge-ci-pull-1497.herokuapp.com is deleted

hasura-bot pushed a commit that referenced this pull request Jun 11, 2021
Remote relationships are now supported on SQL Server and BigQuery. The major change though is the re-architecture of remote join execution logic. Prior to this PR, each backend is responsible for processing the remote relationships that are part of their AST.

This is not ideal as there is nothing specific about a remote join's execution that ties it to a backend. The only backend specific part is whether or not the specification of the remote relationship is valid (i.e, we'll need to validate whether the scalars are compatible).

The approach now changes to this:

1. Before delegating the AST to the backend, we traverse the AST, collect all the remote joins while modifying the AST to add necessary join fields where needed.

1. Once the remote joins are collected from the AST, the database call is made to fetch the response. The necessary data for the remote join(s) is collected from the database's response and one or more remote schema calls are constructed as necessary.

1. The remote schema calls are then executed and the data from the database and from the remote schemas is joined to produce the final response.

### Known issues

1. Ideally the traversal of the IR to collect remote joins should return an AST which does not include remote join fields. This operation can be type safe but isn't taken up as part of the PR.

1. There is a lot of code duplication between `Transport/HTTP.hs` and `Transport/Websocket.hs` which needs to be fixed ASAP. This too hasn't been taken up by this PR.

1. The type which represents the execution plan is only modified to handle our current remote joins and as such it will have to be changed to accommodate general remote joins.

1. Use of lenses would have reduced the boilerplate code to collect remote joins from the base AST.

1. The current remote join logic assumes that the join columns of a remote relationship appear with their names in the database response. This however is incorrect as they could be aliased. This can be taken up by anyone, I've left a comment in the code.

### Notes to the reviewers

I think it is best reviewed commit by commit.

1. The first one is very straight forward.

1. The second one refactors the remote join execution logic but other than moving things around, it doesn't change the user facing functionality.  This moves Postgres specific parts to `Backends/Postgres` module from `Execute`. Some IR related code to `Hasura.RQL.IR` module.  Simplifies various type class function signatures as a backend doesn't have to handle remote joins anymore

1. The third one fixes partial case matches that for some weird reason weren't shown as warnings before this refactor

1. The fourth one generalizes the validation logic of remote relationships and implements `scalarTypeGraphQLName` function on SQL Server and BigQuery which is used by the validation logic. This enables remote relationships on BigQuery and SQL Server.

hasura/graphql-engine-mono#1497

GitOrigin-RevId: 77dd8ee
hasura-bot pushed a commit that referenced this pull request Jan 8, 2025
<!-- The PR description should answer 2 important questions: -->

### What
- Support custom aggregation functions in OpenDd query planning
- Fix GraphQL to OpenDd IR conversion for aggregate selection set and
arguments. Enable few aggregation tests to use the OpenDd execution
pipeline.

V3_GIT_ORIGIN_REV_ID: ae83e5c10fd25cf349123c8d54e756602e2874ec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c/server Related to server

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants