You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Vineet Garg <vg...@hortonworks.com> on 2017/05/17 23:08:58 UTC

Re: Review Request 43834: Support view column authorization

-----------------------------------------------------------
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>.

> 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
> 
>