Add configuration property for Hive view security mode#12221
Add configuration property for Hive view security mode#12221findepi merged 1 commit intotrinodb:masterfrom
Conversation
There was a problem hiding this comment.
why does this have a hard coded true but the next line is configurable? If this is correct, add a comment explaining what is happening here
There was a problem hiding this comment.
Also, this appears to be the only caller of this in the PR, and it is hard coded to true. Does this needt to be a parameter at all?
There was a problem hiding this comment.
Had the wrong approach; re-wrote this based on findepi's guidance
There was a problem hiding this comment.
This formatting change doesn't seem right
There was a problem hiding this comment.
Misread this; thought I was fixing the indentation but the original was correct.
There was a problem hiding this comment.
This is a 3rd config toggle pertaining to Hive Views translation.
I think we should group the configs together under one namespace: hive.hive-views.*.
I posted a PR to discuss that: #12238
There was a problem hiding this comment.
I updated the config name to follow what is being proposed in #12238
There was a problem hiding this comment.
per above, pass the configuration to createViewReader and don't pass it to decodeViewData()
There was a problem hiding this comment.
Thanks, re-wrote this based on your guidance.
|
PR has been rebased |
|
(rebased after #12238 merged) |
plugin/trino-hive/src/main/java/io/trino/plugin/hive/ViewReaderUtil.java
Outdated
Show resolved
Hide resolved
|
Documentation was missed. I created a PR for adding that in.. please review #12272 |
| Optional.ofNullable(table.getParameters().get(TABLE_COMMENT)), | ||
| Optional.empty(), | ||
| false); | ||
| hiveViewsRunAsInvoker); |
There was a problem hiding this comment.
Does it work, if owner is always Optional.empty() (line above)?
There was a problem hiding this comment.
This does work; although the ConnectorViewDefinition returns without an owner, that information is then added again in HiveMetadata#getView.
4 participants
Description
This PR provides a configuration property to set the security mode used for Hive views. Hive views are hard-coded to use the DEFINER mode, which may not always be the desired mode for checking user permissions.
Improvement
Hive connector
The configuration property determines whether access to a Hive view is granted based on the permissions of the user who created the view, or the user who is querying the view, assuming that the security permission checks are enabled.
Related issues, pull requests, and links
Documentation
( ) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.
Release notes
( ) No release notes entries required.
( ) Release notes entries required with the following suggested text: