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

use named notation for function arguments if any argument is not specified (fix #2730) #2777

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Aug 28, 2019
Merged

use named notation for function arguments if any argument is not specified (fix #2730) #2777

merged 6 commits into from
Aug 28, 2019

Conversation

@rakeshkky
Copy link
Member

@rakeshkky rakeshkky commented Aug 26, 2019 •
edited by 0x777
Loading

Description

The server used to generate SQL for calling functions using positional notation. If any argument is not present then the Postgres query fails as mentioned in #2730. This pull fixes it by using named notation to handle such cases.

Affected components

  • Server
  • Tests

Related Issues

fix #2730

Solution and Design

  • Add named notation syntax for function arguments to SQLExp data type
  • Use named notation if not all arguments are specified in the query

Steps to test and verify

Follow the test case added in this pull

Limitations, known bugs & workarounds

@rakeshkky rakeshkky added s/ok-to-merge Status: This pull request can be merged to master c/server Related to server labels Aug 26, 2019
@rakeshkky rakeshkky requested a review from lexi-lambda as a code owner August 26, 2019 13:03
@rakeshkky rakeshkky self-assigned this Aug 26, 2019
@netlify
Copy link

netlify bot commented Aug 26, 2019 •
edited
Loading

Deploy preview for hasura-docs ready!

Built with commit db927b9

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

@hasura-bot
Copy link
Contributor

hasura-bot commented Aug 26, 2019

Review app for commit 9f0f089 deployed to Heroku: https://hge-ci-pull-2777.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2777-9f0f089c

@0x777 0x777 changed the title use named notation for function arguments if not all of them are specified (fix #2730) use named notation for function arguments if any argument is not specified (fix #2730) Aug 26, 2019
lexi-lambda
lexi-lambda reviewed Aug 27, 2019
View reviewed changes
@hasura-bot
Copy link
Contributor

hasura-bot commented Aug 27, 2019

Review app for commit 08793c2 deployed to Heroku: https://hge-ci-pull-2777.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2777-08793c22

@rakeshkky rakeshkky force-pushed the issue-2730-func-args branch from 08793c2 to 1f32b4e Compare August 27, 2019 10:37
@hasura-bot
Copy link
Contributor

hasura-bot commented Aug 27, 2019

Review app for commit 1f32b4e deployed to Heroku: https://hge-ci-pull-2777.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2777-1f32b4eb

@rakeshkky rakeshkky force-pushed the issue-2730-func-args branch from 1f32b4e to 72cd178 Compare August 27, 2019 10:48
@hasura-bot
Copy link
Contributor

hasura-bot commented Aug 28, 2019

Review app for commit 82f7628 deployed to Heroku: https://hge-ci-pull-2777.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2777-82f76287

lexi-lambda
lexi-lambda suggested changes Aug 28, 2019
View reviewed changes
@rakeshkky
disallow omitting of a non default argument in functions
db927b9 
=> Implement algorithm suggested by @lexi-lambda for parsing function
arguments
=> Bump catalog version to 22
@hasura-bot
Copy link
Contributor

hasura-bot commented Aug 28, 2019

Review app for commit db927b9 deployed to Heroku: https://hge-ci-pull-2777.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2777-db927b90

lexi-lambda
lexi-lambda approved these changes Aug 28, 2019
View reviewed changes
server/src-lib/Hasura/GraphQL/Resolve/Select.hs
Nothing -> throw400 NotSupported
"Only last set of positional arguments can be omitted"
Nothing -> if not hasDefault then
throw400 NotSupported "Non default arguments cannot be omitted"
Copy link
Contributor

@lexi-lambda lexi-lambda Aug 28, 2019

Choose a reason for hiding this comment

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

👍 for plumbing this information through.

@lexi-lambda lexi-lambda merged commit d9fb0f8 into hasura:master Aug 28, 2019
@hasura-bot
Copy link
Contributor

hasura-bot commented Aug 28, 2019

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

polRk pushed a commit to polRk/graphql-engine that referenced this pull request Feb 12, 2020
@rakeshkky @polRk
use named notation for function arguments if any argument is not spec…
24cee1b 
…ified (fix hasura#2730) (hasura#2777)

* use positional arguments in SQL functions
* only allow omitting set of last arguments in functions
* disallow omitting of a non default argument in functions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@lexi-lambda lexi-lambda lexi-lambda approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@rakeshkky rakeshkky

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Mismatch variable binding in multiple default arguments function

3 participants

@rakeshkky @hasura-bot @lexi-lambda