-
Notifications
You must be signed in to change notification settings - Fork 2.8k
allow session variables in operators which expect array input #2475
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
Conversation
|
Deploy preview for hasura-docs ready! Built with commit 8cca218 |
|
Review app for commit d632830 deployed to Heroku: https://hge-ci-pull-2475.herokuapp.com |
… into arrays-in-sess-vars
|
Review app for commit 1975293 deployed to Heroku: https://hge-ci-pull-2475.herokuapp.com |
rakeshkky
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
server LGTM
|
Review app for commit 66789d3 deployed to Heroku: https://hge-ci-pull-2475.herokuapp.com |
rakeshkky
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Server LGTM
|
Review app for commit 6c6ad53 deployed to Heroku: https://hge-ci-pull-2475.herokuapp.com |
|
Review app for commit 8cca218 deployed to Heroku: https://hge-ci-pull-2475.herokuapp.com |
|
Review app https://hge-ci-pull-2475.herokuapp.com is deleted |
Session variables can now be used with operators like
_in,_nin,_has_keys_anyand_has_keys_all. For example you now now define such a permission:{"org_id": {"_in": "x-hasura-allowed-org-ids"}}and then provide
x-hasura-allowed-org-idsfrom yourauthnlayer in this format. This helps in cases where some of the information used in your authn layer is not part of the database but is derived from an external source.Affected components
Related Issues
#1333
Solution and Design
What we have in #1799 will not work with multiplexed subscriptions. So I had to change the implementation. The SQL generation changes from
col IN (v1, v2 ..)tocol = ANY (ARRAY[v1,v2 ..]::[v]).Breaking changes
_inand_ninoperators is previously[t](tbeing the type of the column). It is now changed to[t!]. Note that even previously when you have a query with say{owner_id: {_in: [1, null]}}, you would only get rows whereowner_idis1and not rows withowner_idset asNULL. This change makes the previous behaviour explicit.{"org_id": {"_in": ["x-hasura-org-id1", "x-hasura-org-id2", ..]}. Such rules don't work as expected any more. The server only expects the session variable to be present only at the top level, for the above mentioned array operators.Steps to test and verify
I'll update this once the console changes are in.
Limitations, known bugs & workarounds
The biggest limitation is that session variables are still a map of strings to strings. i.e, even arrays have to be expressed as strings as per Postgres's literal syntax for arrays (
{1,2,3..}). This is purely an implementation limitation and requires a fair amount of work but will be lifted as soon as possible.Notes to the reviewer
Previously we assumed that the type of a session variable is the type of the column. So, if you have a clause like
{"user_id":{"_eq": "x-hasura-user-id"}, it implies thatx-hasura-user-idshould be of type,type(user_id). This changes. The type of a session variable now need not be that of column, for example,{"user_id":{"_in": "x-hasura-user-ids"}, the type ofx-hasura-user-idsistype(user_id)[].However our modelling of postgres types (
PGColType) is very rudimentary, while it captures all possible types, we can't really construct a type sayint[]. This has to be fixed and will be part of the PR which adds support for all possible types. However till then I introduced a new type calledPgTypewhich is either a simple type or an array type, a gross oversimplification of postgres types but will help us with this feature.