Mailing-List: contact dev-help@hive.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@hive.apache.org
Content-Type: multipart/alternative;
 boundary="===============6566618989218508226=="
MIME-Version: 1.0
Subject: Re: Review Request 44756: Support masking and filtering of
 rows/columns
From: Ashutosh Chauhan <hashutosh@apache.org>
To: Ashutosh Chauhan <hashutosh@apache.org>
Cc: pengcheng xiong <pxiong@hortonworks.com>, hive <dev@hive.apache.org>
Date: Wed, 16 Mar 2016 01:44:40 -0000
Message-ID: <20160316014440.1703.12517@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: Ashutosh Chauhan <noreply@reviews.apache.org>
References: <20160314225050.26509.86386@reviews.apache.org>
In-Reply-To: <20160314225050.26509.86386@reviews.apache.org>
Reply-To: Ashutosh Chauhan <hashutosh@apache.org>

--===============6566618989218508226==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44756/#review123813
-----------------------------------------------------------


ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java (line 10327)
<https://reviews.apache.org/r/44756/#comment186087>

    We need to add support for these tokens or throw exception. Ignoring them leaves a security hole.


ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java (line 10381)
<https://reviews.apache.org/r/44756/#comment186088>

    We need an early exit critirea from parts of tree where we know for sure table token cannot appear like GBY, over clause etc.


ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java (line 10395)
<https://reviews.apache.org/r/44756/#comment186090>

    This cache should be maintained at SemanticAnalyzer level, because we may retrieve metadata for tables later in compilation as well.


ql/src/java/org/apache/hadoop/hive/ql/parse/TableMask.java (line 31)
<https://reviews.apache.org/r/44756/#comment186096>

    Add javadocs for purpose of this class.


ql/src/java/org/apache/hadoop/hive/ql/parse/TableMask.java (line 43)
<https://reviews.apache.org/r/44756/#comment186094>

    We should enable only if new method suggested in interface returns true.


ql/src/java/org/apache/hadoop/hive/ql/parse/TableMask.java (line 95)
<https://reviews.apache.org/r/44756/#comment186097>

    Add LOG.debug (sb) here.


ql/src/java/org/apache/hadoop/hive/ql/parse/TableMask.java (line 99)
<https://reviews.apache.org/r/44756/#comment186099>

    Better name: addQueryBlock?


ql/src/java/org/apache/hadoop/hive/ql/parse/TableMask.java (line 103)
<https://reviews.apache.org/r/44756/#comment186100>

    Better name.


ql/src/java/org/apache/hadoop/hive/ql/parse/TableMask.java (line 107)
<https://reviews.apache.org/r/44756/#comment186098>

    Better name: needsRewrite()


ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthorizer.java (line 300)
<https://reviews.apache.org/r/44756/#comment186093>

    We should add additional method boolean needToEnforceRowColumnTransformation(String username) so that we can avoid traversing AST tree if this method returns false.


- Ashutosh Chauhan


On March 14, 2016, 10:50 p.m., pengcheng xiong wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44756/
> -----------------------------------------------------------
> 
> (Updated March 14, 2016, 10:50 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-13125
> 
> 
> Diffs
> -----
> 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLStdHiveAuthorizationValidatorForTest.java fd39c67 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 2dcb6d6 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TableMask.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthorizationValidator.java 59aabe4 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthorizer.java c93e334 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthorizerImpl.java 00fa8cf 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveV1Authorizer.java 8a03989 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/DummyHiveAuthorizationValidator.java 26e3a2c 
>   ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLStdHiveAuthorizationValidator.java 9f586be 
>   ql/src/test/queries/clientpositive/masking_1.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/masking_2.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/masking_3.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/masking_4.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/masking_disablecbo_1.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/masking_disablecbo_2.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/masking_disablecbo_3.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/masking_disablecbo_4.q PRE-CREATION 
>   ql/src/test/results/clientpositive/masking_1.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/masking_2.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/masking_3.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/masking_4.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/masking_disablecbo_1.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/masking_disablecbo_2.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/masking_disablecbo_3.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/masking_disablecbo_4.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44756/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> pengcheng xiong
> 
>


--===============6566618989218508226==--