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

functions can access session info via input arg (close #2322) #3143

Merged
merged 26 commits into from Nov 20, 2019

Conversation

rakeshkky
Copy link
Member

@rakeshkky rakeshkky commented Oct 15, 2019

Description

We can access session information (in JSON) in a transaction using current_setting('hasura.user') when queries are run in a transaction. But subscriptions are not run in a transaction any more. This PR supports tracking SQL functions by configuring an input session argument so as to access the session variables JSON through it.

Metadata exported by the server (through export_metadata) is versioned via a top-level field version whose value is either 1 or 2. If version field is absent (metadata exported by previous server versions) then it is considered as version: 1. In version 2, the exported metadata functions are a list of JSON objects with function and configuration field.

A sample metadata yaml exported.

version: 2
functions:
- function: search_posts
  configuration: {}
- function: get_test
  configuration: {}
- function: my_add
  configuration: {}
- function: get_users
  configuration: {}
- function: get_session_var
  configuration:
    session_argument: hasura_session
# Other metadata objects come below

See preview docs to learn more about accessing session variables in SQL functions.

Affected components

  • Server
  • Docs
  • Tests

Related Issues

close #2322

Solution and Design

  • A new version 2 track_function query type is defined to facilitate users to specify the configuration with session argument. An example request as follows
   POST /v1/query HTTP/1.1
   Content-Type: application/json
   X-Hasura-Role: admin

   {
       "type": "track_function",
       "version": 2,
       "args": {
           "function": {
               "schema": "public",
               "name": "search_articles"
           },
           "configuration": {
               "session_argument": "hasura_session"
           }
       }
   }
  • The server executes the SQL function with session variable JSON object.
  • The export/apply metadata JSON document has a new optional version key. The server exports metadata with version 2 and accepts metadata of versions 1 and 2.

Steps to test and verify

Follow the example here

Limitations, known bugs & workarounds

  • Older versions cannot import metadata exported by this.

@netlify
Copy link

netlify bot commented Oct 15, 2019

Deploy preview for hasura-docs ready!

Built with commit 99664ce

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

@rakeshkky rakeshkky changed the title support session information argument in SQL functions (close #2322) allow SQL functions to access session information through an input argument (close #2322) Oct 15, 2019
@rakeshkky rakeshkky self-assigned this Oct 15, 2019
@rakeshkky rakeshkky added the c/server Related to server label Oct 15, 2019
@hasura-bot
Copy link
Contributor

Review app for commit 14c58e3 deployed to Heroku: https://hge-ci-pull-3143.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3143-14c58e33

@hasura-bot
Copy link
Contributor

Review app for commit 322b814 deployed to Heroku: https://hge-ci-pull-3143.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3143-322b814d

Resolve Conflicts:
	server/src-lib/Hasura/GraphQL/Resolve/Select.hs
	server/src-lib/Hasura/GraphQL/Resolve/Types.hs
	server/src-lib/Hasura/GraphQL/Schema.hs
	server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs
	server/src-lib/Hasura/RQL/DDL/Schema/Function.hs
	server/src-lib/Hasura/RQL/Types/Catalog.hs
	server/src-lib/Hasura/RQL/Types/Common.hs
	server/src-lib/Hasura/RQL/Types/SchemaCache.hs
	server/src-rsr/catalog_metadata.sql
	server/src-rsr/migrations/25_to_26.sql
@hasura-bot
Copy link
Contributor

Review app for commit afbe0eb deployed to Heroku: https://hge-ci-pull-3143.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3143-afbe0eb0

Resolve Conflicts:
	server/src-lib/Hasura/RQL/DDL/Metadata.hs
	server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs
	server/src-lib/Hasura/RQL/DDL/Schema/Function.hs
	server/src-lib/Hasura/RQL/Types/Catalog.hs
	server/src-rsr/catalog_metadata.sql
@hasura-bot
Copy link
Contributor

Review app for commit b3bff20 deployed to Heroku: https://hge-ci-pull-3143.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3143-b3bff20a

@hasura-bot
Copy link
Contributor

Review app for commit 110f0d2 deployed to Heroku: https://hge-ci-pull-3143.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3143-110f0d2c

@hasura-bot
Copy link
Contributor

Review app for commit 18d6625 deployed to Heroku: https://hge-ci-pull-3143.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3143-18d6625c

@rakeshkky rakeshkky marked this pull request as ready for review October 23, 2019 06:28
@lexi-lambda
Copy link
Contributor

Cross-posting a comment I made on slack, since that discussion never really went anywhere:

It’s unfortunate that Postgres has no notion of dynamically scoped variables or something like that so that we could preserve the current API. I do think this could be an area where convention over configuration feels better to me, though—why not just automatically use an argument named hasura_session? That seems pretty unlikely to ever conflict with anything else, and it seems silly to track this information for every function.

@rakeshkky, do you have any thoughts on whether or not doing something like that would work/is a good idea? The overhead of configuring this for every function individually seems a little high to me, but maybe I’m overreacting.

@rakeshkky
Copy link
Member Author

Cross-posting a comment I made on slack, since that discussion never really went anywhere:

It’s unfortunate that Postgres has no notion of dynamically scoped variables or something like that so that we could preserve the current API. I do think this could be an area where convention over configuration feels better to me, though—why not just automatically use an argument named hasura_session? That seems pretty unlikely to ever conflict with anything else, and it seems silly to track this information for every function.

@rakeshkky, do you have any thoughts on whether or not doing something like that would work/is a good idea? The overhead of configuring this for every function individually seems a little high to me, but maybe I’m overreacting.

@lexi-lambda Now, we may avoid configuring the SQL functions and auto derive the session argument using hasura_session argument name, but, in future, we may need to configure the SQL function definitely if a feature does not allow us to auto derive things. Already, we are configuring the tracked tables/views. I don't feel it is not good to introduce configuration for SQL functions too.

As a developer, IMO, I'd like to define things explicitly. While tracking a SQL functions I'd always like to specify what argument in my function accepts the session information JSON 🙂. Auto deriving may affect the functions which are already tracked (if a function already has hasura_session as an argument).

@hasura-bot
Copy link
Contributor

Review app for commit ba1b335 deployed to Heroku: https://hge-ci-pull-3143.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3143-ba1b3355

@rakeshkky rakeshkky dismissed stale reviews from shahidhk and lexi-lambda via ef9ca75 November 18, 2019 07:52
@hasura-bot
Copy link
Contributor

Review app for commit ef9ca75 deployed to Heroku: https://hge-ci-pull-3143.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3143-ef9ca752

@hasura-bot
Copy link
Contributor

Review app for commit 42ec624 deployed to Heroku: https://hge-ci-pull-3143.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3143-42ec624c

lexi-lambda
lexi-lambda previously approved these changes Nov 18, 2019
shahidhk
shahidhk previously approved these changes Nov 19, 2019
Copy link
Contributor

@marionschleifer marionschleifer left a comment

Choose a reason for hiding this comment

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

I left one comment. Otherwise, it looks good 🙂

The session argument will be a JSON object where keys are session variable names (in lower case) and values are strings.
Use the ``->>`` JSON operator to fetch the value of a session variable as shown in the following example.

.. note::
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have the note in the end of this section? When we say in the following example, then the example should follow immediately.

Copy link
Contributor

@marionschleifer marionschleifer left a comment

Choose a reason for hiding this comment

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

💯

@shahidhk shahidhk changed the title allow SQL functions to access session information through an input argument (close #2322) allow sql functions to access session info via an input argument (close #2322) Nov 20, 2019
@shahidhk shahidhk changed the title allow sql functions to access session info via an input argument (close #2322) allow sql functions to access session info via input argument (close #2322) Nov 20, 2019
@hasura-bot
Copy link
Contributor

Review app for commit 99664ce deployed to Heroku: https://hge-ci-pull-3143.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3143-99664ce4

@shahidhk shahidhk changed the title allow sql functions to access session info via input argument (close #2322) functions can access session info via input arg (close #2322) Nov 20, 2019
@shahidhk shahidhk merged commit 9b8e6b4 into hasura:master Nov 20, 2019
@hasura-bot
Copy link
Contributor

Review app https://hge-ci-pull-3143.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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

allow functions to use session information through an argument
5 participants