> On Sept. 13, 2014, 5:28 p.m., Prasad Mujumdar wrote:
> > Couple of high level comments:
> > - I am wondering if we should support creating view with column level
> > permissions. Today it requres select at table level. If I have access to a
> > subset of columns and create privilege on the database, this feature should
> > also allow me to create a view with that subset of columns. Let me know
> > what you think.
> > - We should more end to end tests for column select access paths,
> > + INSERT OVERWRITE into Table or Directory
> > + CREATE and query views (with the current privilege model)
> > + CREATE TABLE AS query
> > -
Thx Prasad! Your comments are very valable, I have a seen to the code again.
1. It may difficult to support creating view with column level permissions.
Here are the reasons:
1) If operation is CREATEVIEW, analyzeInternal in SemanticAnalyzer will return
before compiler.comple, so we couldn't get accessed columns from query.
if (createVwDesc != null) {
saveViewDefinition();
// validate the create view statement
// at this point, the createVwDesc gets all the information for semantic
check
validateCreateView(createVwDesc);
// Since we're only creating a view (not executing it), we
// don't need to optimize or translate the plan (and in fact, those
// procedures can interfere with the view creation). So
// skip the rest of this method.
ctx.setResDir(null);
ctx.setResFile(null);
try {
PlanUtils.addInputsForView(pCtx);
} catch (HiveException e) {
throw new SemanticException(e);
}
return;
}
...
if (!ctx.getExplainLogical()) {
// At this point we have the complete operator tree
// from which we want to create the map-reduce plan
TaskCompiler compiler = TaskCompilerFactory.getCompiler(conf, pCtx);
compiler.init(conf, console, db);
compiler.compile(pCtx, rootTasks, inputs, outputs);
fetchTask = pCtx.getFetchTask();
}
2) When we create view, we can have the columns of view with alias, e.g.
“CREATE VIEW VIEW_1(a1,a2) AS SELECT id,name FROM t1”. But we can just get the
true column names of the view/table from columnAccessInfo, and I also put the
true columns to ReadEntity(HIVE-7730). So we don't support column level
privilege of view.
2. As your suggestions, I added a test case of CreateAsSelect in
TestColumnEndToEnd and found a bug. I have fixed it, thx very much! And we also
has other e2e tests, e.g. TestDbPrivilegesAtColumnScope. This patch is complex
and very important, for reviewing clearly by you, I updated the patch. Soon we
will update the merged patch in https://reviews.apache.org/r/24973/.
> On Sept. 13, 2014, 5:28 p.m., Prasad Mujumdar wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java,
> > line 606
> > <https://reviews.apache.org/r/24963/diff/5/?file=683931#file683931line606>
> >
> > Is it possible to have something other than table or partition ? Should
> > we raise an error in the default case ?
In this switch, I just add column Authorizable to the end of entityHierarchy(I
have put server, db to entityHierarchy before switch). Only table and partition
has columns, if entity type is others, we don't add column Authorizable and
just put entityHierarchy to inputHierarchy like table level security.
- Xiaomeng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24963/#review53269
-----------------------------------------------------------
On Sept. 9, 2014, 11:49 a.m., Xiaomeng Huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24963/
> -----------------------------------------------------------
>
> (Updated Sept. 9, 2014, 11:49 a.m.)
>
>
> Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya
> Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Authorization for column level security. This patch is depends on
> HIVE-7730(https://reviews.apache.org/r/24962/)
>
>
> Diffs
> -----
>
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
> a760516
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java
> 2f97e30
>
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java
> 6e40a5c
>
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
> 51d4248
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java
> PRE-CREATION
>
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java
> 4c66ffe
>
> Diff: https://reviews.apache.org/r/24963/diff/
>
>
> Testing
> -------
>
> test cases are included.
>
>
> Thanks,
>
> Xiaomeng Huang
>
>
