You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Navis Ryu <na...@nexr.com> on 2014/03/18 08:57:48 UTC
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/
-----------------------------------------------------------
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
Re: Review Request 19344: Location for new table or partition should be a
write entity
Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
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
>
>