You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by pengcheng xiong <px...@hortonworks.com> on 2016/02/22 16:24:41 UTC
Review Request 43834: Support view column authorization
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43834/
-----------------------------------------------------------
Review request for hive and Ashutosh Chauhan.
Repository: hive-git
Description
-------
HIVE-13095
Diffs
-----
ql/src/java/org/apache/hadoop/hive/ql/Driver.java 10bd97b
ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcFactory.java 78bce23
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelFieldTrimmer.java 18145ae
ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 809affb
ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java 642c227
ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java f04b493
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 8a06582
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java fc555ca
ql/src/test/queries/clientnegative/authorization_view_1.q PRE-CREATION
ql/src/test/queries/clientnegative/authorization_view_2.q PRE-CREATION
ql/src/test/queries/clientnegative/authorization_view_disable_cbo_1.q PRE-CREATION
ql/src/test/queries/clientnegative/authorization_view_disable_cbo_2.q PRE-CREATION
ql/src/test/queries/clientnegative/authorization_view_disable_cbo_3.q PRE-CREATION
ql/src/test/queries/clientpositive/authorization_view_1.q PRE-CREATION
ql/src/test/queries/clientpositive/authorization_view_disable_cbo_1.q PRE-CREATION
ql/src/test/results/clientnegative/authorization_view_1.q.out PRE-CREATION
ql/src/test/results/clientnegative/authorization_view_2.q.out PRE-CREATION
ql/src/test/results/clientnegative/authorization_view_disable_cbo_1.q.out PRE-CREATION
ql/src/test/results/clientnegative/authorization_view_disable_cbo_2.q.out PRE-CREATION
ql/src/test/results/clientnegative/authorization_view_disable_cbo_3.q.out PRE-CREATION
ql/src/test/results/clientpositive/authorization_view_1.q.out PRE-CREATION
ql/src/test/results/clientpositive/authorization_view_disable_cbo_1.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/43834/diff/
Testing
-------
Thanks,
pengcheng xiong
Re: Review Request 43834: Support view column authorization
Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43834/#review120371
-----------------------------------------------------------
It will also be good to add true junit tests for this. You may look at TestHiveAuthorizerCheckInvocation for pattern.
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelFieldTrimmer.java (line 403)
<https://reviews.apache.org/r/43834/#comment181831>
Instead of repeating all the logic here, shall we call super() here?
ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java (line 50)
<https://reviews.apache.org/r/43834/#comment181832>
better name: viewAliasToViewSchema?
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java (lines 10439 - 10449)
<https://reviews.apache.org/r/43834/#comment181834>
Do we really need to invoke CP here? When CP gets invoked later on in optimize phase, it will populate column access info correctly, right?
ql/src/test/queries/clientnegative/authorization_view_1.q (line 12)
<https://reviews.apache.org/r/43834/#comment181807>
Add:
grant select(key) on v to user hive_test_user;
Test should still fail, since hive_test_user doesnt have select privs for value.
ql/src/test/queries/clientnegative/authorization_view_2.q (line 11)
<https://reviews.apache.org/r/43834/#comment181808>
Change user to hive_test_user for both statements.
ql/src/test/queries/clientnegative/authorization_view_disable_cbo_1.q (line 12)
<https://reviews.apache.org/r/43834/#comment181809>
Similar comment as above.
ql/src/test/queries/clientnegative/authorization_view_disable_cbo_3.q (line 11)
<https://reviews.apache.org/r/43834/#comment181811>
User name: hive_test_user
- Ashutosh Chauhan
On Feb. 22, 2016, 3:24 p.m., pengcheng xiong wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43834/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2016, 3:24 p.m.)
>
>
> Review request for hive and Ashutosh Chauhan.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-13095
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/Driver.java 10bd97b
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcFactory.java 78bce23
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelFieldTrimmer.java 18145ae
> ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 809affb
> ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java 642c227
> ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java f04b493
> ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 8a06582
> ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java fc555ca
> ql/src/test/queries/clientnegative/authorization_view_1.q PRE-CREATION
> ql/src/test/queries/clientnegative/authorization_view_2.q PRE-CREATION
> ql/src/test/queries/clientnegative/authorization_view_disable_cbo_1.q PRE-CREATION
> ql/src/test/queries/clientnegative/authorization_view_disable_cbo_2.q PRE-CREATION
> ql/src/test/queries/clientnegative/authorization_view_disable_cbo_3.q PRE-CREATION
> ql/src/test/queries/clientpositive/authorization_view_1.q PRE-CREATION
> ql/src/test/queries/clientpositive/authorization_view_disable_cbo_1.q PRE-CREATION
> ql/src/test/results/clientnegative/authorization_view_1.q.out PRE-CREATION
> ql/src/test/results/clientnegative/authorization_view_2.q.out PRE-CREATION
> ql/src/test/results/clientnegative/authorization_view_disable_cbo_1.q.out PRE-CREATION
> ql/src/test/results/clientnegative/authorization_view_disable_cbo_2.q.out PRE-CREATION
> ql/src/test/results/clientnegative/authorization_view_disable_cbo_3.q.out PRE-CREATION
> ql/src/test/results/clientpositive/authorization_view_1.q.out PRE-CREATION
> ql/src/test/results/clientpositive/authorization_view_disable_cbo_1.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43834/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> pengcheng xiong
>
>
Re: Review Request 43834: Support view column authorization
Posted by pengcheng xiong <px...@hortonworks.com>.
> On May 17, 2017, 11:08 p.m., Vineet Garg wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelFieldTrimmer.java
> > Lines 390 (patched)
> > <https://reviews.apache.org/r/43834/diff/2/?file=1267076#file1267076line391>
> >
> > Hi Pengcheng,
> >
> > I know this is pretty old change but I would like to know few things about this change (as I am running into issues while trying to fix HIVE-16689)
> >
> > 1) If I understand this project here corresponds to view and we are trying to populate column access for this view from corresponding table. What is the reason columnAccessInfo is restricted to columns which are being used i.e. only if fieldsUsed.get(ord.id) is true. Is there any side effect of adding all columns from tab?
> > 2) What is the reason columnAccessInfo is populated here? instead of for example durin genLogicalPlan?
> >
> >
> > RelTrimmer expects its plan to not have subquery (this could be bug or expected behavior I am not sure about) so we need to call remove subquery before calling RelTrimmer. But calling remove subquery before RelTrimmer changes the plan and makes 'Project' cached in viewProjectToTableSchema invalid. As a result columnAccessInfo is not populated and view authorization breaks. That is why I am trying to figure out why is it necessary to populate columnAccess.
> > Note that we do not even use the plan after RelTrimmer's column pruning. We discard it and only use columnAccessInfo.
> >
> > Your help will be much appreciated.
> >
> > -Vineet
It was quite a long time ago when i did this patch. I think we need to check fieldsUsed.get(ord.id) is true because in some of the cases, user do not need some column access rights if the column is filtered out. For example, select key from (select key, value from src)sub. User do not need column access for value.
- pengcheng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43834/#review175326
-----------------------------------------------------------
On Feb. 24, 2016, 7:44 a.m., pengcheng xiong wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43834/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2016, 7:44 a.m.)
>
>
> Review request for hive and Ashutosh Chauhan.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-13095
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/Driver.java 10bd97b
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPruner.java c353e3e
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcFactory.java 78bce23
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelFieldTrimmer.java 18145ae
> ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 809affb
> ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java 642c227
> ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java f04b493
> ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 8a06582
> ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java fc555ca
> ql/src/test/queries/clientnegative/authorization_view_1.q PRE-CREATION
> ql/src/test/queries/clientnegative/authorization_view_2.q PRE-CREATION
> ql/src/test/queries/clientnegative/authorization_view_3.q PRE-CREATION
> ql/src/test/queries/clientnegative/authorization_view_4.q PRE-CREATION
> ql/src/test/queries/clientnegative/authorization_view_disable_cbo_1.q PRE-CREATION
> ql/src/test/queries/clientnegative/authorization_view_disable_cbo_2.q PRE-CREATION
> ql/src/test/queries/clientnegative/authorization_view_disable_cbo_3.q PRE-CREATION
> ql/src/test/queries/clientnegative/authorization_view_disable_cbo_4.q PRE-CREATION
> ql/src/test/queries/clientpositive/authorization_view_1.q PRE-CREATION
> ql/src/test/queries/clientpositive/authorization_view_disable_cbo_1.q PRE-CREATION
> ql/src/test/results/clientnegative/authorization_view_1.q.out PRE-CREATION
> ql/src/test/results/clientnegative/authorization_view_2.q.out PRE-CREATION
> ql/src/test/results/clientnegative/authorization_view_3.q.out PRE-CREATION
> ql/src/test/results/clientnegative/authorization_view_4.q.out PRE-CREATION
> ql/src/test/results/clientnegative/authorization_view_disable_cbo_1.q.out PRE-CREATION
> ql/src/test/results/clientnegative/authorization_view_disable_cbo_2.q.out PRE-CREATION
> ql/src/test/results/clientnegative/authorization_view_disable_cbo_3.q.out PRE-CREATION
> ql/src/test/results/clientnegative/authorization_view_disable_cbo_4.q.out PRE-CREATION
> ql/src/test/results/clientpositive/authorization_view_1.q.out PRE-CREATION
> ql/src/test/results/clientpositive/authorization_view_disable_cbo_1.q.out PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/43834/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> pengcheng xiong
>
>
Re: Review Request 43834: Support view column authorization
Posted by Vineet Garg <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43834/#review175326
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelFieldTrimmer.java
Lines 390 (patched)
<https://reviews.apache.org/r/43834/#comment248778>
Hi Pengcheng,
I know this is pretty old change but I would like to know few things about this change (as I am running into issues while trying to fix HIVE-16689)
1) If I understand this project here corresponds to view and we are trying to populate column access for this view from corresponding table. What is the reason columnAccessInfo is restricted to columns which are being used i.e. only if fieldsUsed.get(ord.id) is true. Is there any side effect of adding all columns from tab?
2) What is the reason columnAccessInfo is populated here? instead of for example durin genLogicalPlan?
RelTrimmer expects its plan to not have subquery (this could be bug or expected behavior I am not sure about) so we need to call remove subquery before calling RelTrimmer. But calling remove subquery before RelTrimmer changes the plan and makes 'Project' cached in viewProjectToTableSchema invalid. As a result columnAccessInfo is not populated and view authorization breaks. That is why I am trying to figure out why is it necessary to populate columnAccess.
Note that we do not even use the plan after RelTrimmer's column pruning. We discard it and only use columnAccessInfo.
Your help will be much appreciated.
-Vineet
- Vineet Garg
On Feb. 24, 2016, 7:44 a.m., pengcheng xiong wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43834/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2016, 7:44 a.m.)
>
>
> Review request for hive and Ashutosh Chauhan.
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> HIVE-13095
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/Driver.java 10bd97b
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPruner.java c353e3e
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcFactory.java 78bce23
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelFieldTrimmer.java 18145ae
> ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 809affb
> ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java 642c227
> ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java f04b493
> ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 8a06582
> ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java fc555ca
> ql/src/test/queries/clientnegative/authorization_view_1.q PRE-CREATION
> ql/src/test/queries/clientnegative/authorization_view_2.q PRE-CREATION
> ql/src/test/queries/clientnegative/authorization_view_3.q PRE-CREATION
> ql/src/test/queries/clientnegative/authorization_view_4.q PRE-CREATION
> ql/src/test/queries/clientnegative/authorization_view_disable_cbo_1.q PRE-CREATION
> ql/src/test/queries/clientnegative/authorization_view_disable_cbo_2.q PRE-CREATION
> ql/src/test/queries/clientnegative/authorization_view_disable_cbo_3.q PRE-CREATION
> ql/src/test/queries/clientnegative/authorization_view_disable_cbo_4.q PRE-CREATION
> ql/src/test/queries/clientpositive/authorization_view_1.q PRE-CREATION
> ql/src/test/queries/clientpositive/authorization_view_disable_cbo_1.q PRE-CREATION
> ql/src/test/results/clientnegative/authorization_view_1.q.out PRE-CREATION
> ql/src/test/results/clientnegative/authorization_view_2.q.out PRE-CREATION
> ql/src/test/results/clientnegative/authorization_view_3.q.out PRE-CREATION
> ql/src/test/results/clientnegative/authorization_view_4.q.out PRE-CREATION
> ql/src/test/results/clientnegative/authorization_view_disable_cbo_1.q.out PRE-CREATION
> ql/src/test/results/clientnegative/authorization_view_disable_cbo_2.q.out PRE-CREATION
> ql/src/test/results/clientnegative/authorization_view_disable_cbo_3.q.out PRE-CREATION
> ql/src/test/results/clientnegative/authorization_view_disable_cbo_4.q.out PRE-CREATION
> ql/src/test/results/clientpositive/authorization_view_1.q.out PRE-CREATION
> ql/src/test/results/clientpositive/authorization_view_disable_cbo_1.q.out PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/43834/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> pengcheng xiong
>
>
Re: Review Request 43834: Support view column authorization
Posted by pengcheng xiong <px...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43834/
-----------------------------------------------------------
(Updated Feb. 24, 2016, 7:44 a.m.)
Review request for hive and Ashutosh Chauhan.
Repository: hive-git
Description
-------
HIVE-13095
Diffs (updated)
-----
ql/src/java/org/apache/hadoop/hive/ql/Driver.java 10bd97b
ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPruner.java c353e3e
ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcFactory.java 78bce23
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelFieldTrimmer.java 18145ae
ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 809affb
ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java 642c227
ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java f04b493
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 8a06582
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java fc555ca
ql/src/test/queries/clientnegative/authorization_view_1.q PRE-CREATION
ql/src/test/queries/clientnegative/authorization_view_2.q PRE-CREATION
ql/src/test/queries/clientnegative/authorization_view_3.q PRE-CREATION
ql/src/test/queries/clientnegative/authorization_view_4.q PRE-CREATION
ql/src/test/queries/clientnegative/authorization_view_disable_cbo_1.q PRE-CREATION
ql/src/test/queries/clientnegative/authorization_view_disable_cbo_2.q PRE-CREATION
ql/src/test/queries/clientnegative/authorization_view_disable_cbo_3.q PRE-CREATION
ql/src/test/queries/clientnegative/authorization_view_disable_cbo_4.q PRE-CREATION
ql/src/test/queries/clientpositive/authorization_view_1.q PRE-CREATION
ql/src/test/queries/clientpositive/authorization_view_disable_cbo_1.q PRE-CREATION
ql/src/test/results/clientnegative/authorization_view_1.q.out PRE-CREATION
ql/src/test/results/clientnegative/authorization_view_2.q.out PRE-CREATION
ql/src/test/results/clientnegative/authorization_view_3.q.out PRE-CREATION
ql/src/test/results/clientnegative/authorization_view_4.q.out PRE-CREATION
ql/src/test/results/clientnegative/authorization_view_disable_cbo_1.q.out PRE-CREATION
ql/src/test/results/clientnegative/authorization_view_disable_cbo_2.q.out PRE-CREATION
ql/src/test/results/clientnegative/authorization_view_disable_cbo_3.q.out PRE-CREATION
ql/src/test/results/clientnegative/authorization_view_disable_cbo_4.q.out PRE-CREATION
ql/src/test/results/clientpositive/authorization_view_1.q.out PRE-CREATION
ql/src/test/results/clientpositive/authorization_view_disable_cbo_1.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/43834/diff/
Testing
-------
Thanks,
pengcheng xiong