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

Conversation

@rakeshkky
Copy link
Member

@rakeshkky rakeshkky commented Jul 29, 2019

Description

The change related to permission limits in #2027 has slow down the queries. This PR fixes that.

Affected components

  • Server
  • Tests

Related Issues

N/A

Solution and Design

The presence of aggregations in queries affects the usage of a subquery in selecting JSON rows. If aggregations present, then use subquery for permission limit.

The application of limit in BaseNode depends on whether a subquery is being used.
Refer Files changed for in-depth details.

To Reviewer

Only src-lib/Hasura/RQL/DML/Select/Internal.hs module is changed and a utility type is added in src-lib/Hasura/RQL/DML/Select/Types.hs

Steps to test and verify

Define permission to a table with a limit. Make a <table-name>_aggregate query with and without aggregate field. Look for the generated SQL statement using the Analyze button on the console.

Limitations, known bugs & workarounds

@rakeshkky rakeshkky requested a review from lexi-lambda as a code owner July 29, 2019 14:34
@netlify
Copy link

netlify bot commented Jul 29, 2019

Deploy preview for hasura-docs ready!

Built with commit 36c6cbd

https://deploy-preview-2630--hasura-docs.netlify.com

@hasura-bot
Copy link
Contributor

Review app for commit d1cf004 deployed to Heroku: https://hge-ci-pull-2630.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2630-d1cf004c

@ecthiender ecthiender added the c/server Related to server label Jul 30, 2019
@rakeshkky rakeshkky changed the title don't use sub-query for permission limit if aggregations are not present do not use sub-query for permission limit if aggregations are not present Jul 30, 2019
@hasura-bot
Copy link
Contributor

Review app for commit 03e0840 deployed to Heroku: https://hge-ci-pull-2630.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2630-03e08404

@rakeshkky rakeshkky added the s/ok-to-merge Status: This pull request can be merged to master label Jul 30, 2019
@hasura-bot
Copy link
Contributor

Review app for commit 7a8b8e8 deployed to Heroku: https://hge-ci-pull-2630.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2630-7a8b8e85

Copy link
Member

@0x777 0x777 left a comment

Choose a reason for hiding this comment

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

LGTM

@hasura-bot
Copy link
Contributor

Review app for commit 36c6cbd deployed to Heroku: https://hge-ci-pull-2630.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2630-36c6cbd4

@shahidhk shahidhk merged commit 6b8a6ca into hasura:master Aug 1, 2019
@hasura-bot
Copy link
Contributor

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

polRk pushed a commit to polRk/graphql-engine that referenced this pull request Feb 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c/server Related to server s/ok-to-merge Status: This pull request can be merged to master

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants