You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Thejas Nair <th...@hortonworks.com> on 2014/04/29 04:04:51 UTC
Re: Review Request 19344: Location for new table or partition should be a
write entity
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19344/#review41668
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java
<https://reviews.apache.org/r/19344/#comment75228>
Same issue with this change. It will break Sql std authorization -
in Operation2Privilege -
// in alter-table-add-partition, the table is output, and location is input
op2Priv.put(HiveOperationType.ALTERTABLE_ADDPARTS, new InOutPrivs(OWNER_INS_SEL_DEL_NOGRANT_AR, INS_NOGRANT_AR));
Can we hold off on this part of the changes, that add uri along with other types such as table and database?
Let me take a look at how I can fix the sql std auth to handle this.
ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java
<https://reviews.apache.org/r/19344/#comment75227>
This is already there on line 232
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/19344/#comment75224>
This would break the new sql standard authorization. This will add both URI and database to output.
See Operation2Privilege.java, it uses only one type of privilege check for output. I agree that is a limitation with the implementation (which is similar to the default authorization in this case).
So different kind of check for Database and URI will not be possible.
There was another jira where you also had changes for refactoring ReadEntity/WriteEntity to include more information. But I was unable to find that jira now. I wonder if those changes will help improve the code to avoid this issue.
ql/src/test/results/clientnegative/authorization_uri_create_table1.q.out
<https://reviews.apache.org/r/19344/#comment75225>
This is a strange change in output. Not sure why the changes are resulting in ParseException of all things!
ql/src/test/results/clientnegative/deletejar.q.out
<https://reviews.apache.org/r/19344/#comment75223>
This change looks unrelated.
- Thejas Nair
On March 18, 2014, 7:57 a.m., Navis Ryu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19344/
> -----------------------------------------------------------
>
> (Updated March 18, 2014, 7:57 a.m.)
>
>
> Review request for hive and Thejas Nair.
>
>
> Bugs: HIVE-6692
> https://issues.apache.org/jira/browse/HIVE-6692
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Locations for "create table" and "alter table add partition"should be write entities.
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/common/FileUtils.java 16d7c80
> ql/src/java/org/apache/hadoop/hive/ql/Driver.java 7dbb8be
> ql/src/java/org/apache/hadoop/hive/ql/hooks/Entity.java 2a38aad
> ql/src/java/org/apache/hadoop/hive/ql/hooks/ReadEntity.java 6d7c4f6
> ql/src/java/org/apache/hadoop/hive/ql/hooks/WriteEntity.java 44a3924
> ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java db9fa74
> ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java e642919
> ql/src/java/org/apache/hadoop/hive/ql/parse/ImportSemanticAnalyzer.java 92ec334
> ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java 6c53447
> ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java e1e427f
> ql/src/test/results/clientnegative/archive_multi7.q.out a8eee2f
> ql/src/test/results/clientnegative/authorization_droppartition.q.out 1da250a
> ql/src/test/results/clientnegative/authorization_uri_alterpart_loc.q.out 39a4e4f
> ql/src/test/results/clientnegative/authorization_uri_create_table1.q.out 0b8182a
> ql/src/test/results/clientnegative/authorization_uri_create_table_ext.q.out 0b8182a
> ql/src/test/results/clientnegative/deletejar.q.out 91560ee
> ql/src/test/results/clientnegative/exim_20_managed_location_over_existing.q.out fd4a418
> ql/src/test/results/clientnegative/external1.q.out 696beaa
> ql/src/test/results/clientnegative/external2.q.out a604885
> ql/src/test/results/clientnegative/insertexternal1.q.out 3df5013
>
> Diff: https://reviews.apache.org/r/19344/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Navis Ryu
>
>