> On Feb. 15, 2018, 5:38 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/AbstractFileMergeOperator.java
> > Line 225 (original), 227 (patched)
> > <https://reviews.apache.org/r/65663/diff/1/?file=1961243#file1961243line227>
> >
> >     inherit perms feature till now is for insert statements, not for 
> > concat/merge files. If you want to increase the scope of a feature we shall 
> > do it in a seperate jira and limit this jira for acid tables perm 
> > inheritance.

Merge can happen as part of normal query execution, so it is needed for full 
support.
"Fortunately", it's not supported for ACID in 2.X, so I'll just leave a TODO.


> On Feb. 15, 2018, 5:38 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java
> > Line 231 (original), 235 (patched)
> > <https://reviews.apache.org/r/65663/diff/1/?file=1961245#file1961245line235>
> >
> >     Permission inheritance is taken care in MoveTask. Why is there a need 
> > to do this here?

Nm, this would only be needed for MM tables, not in 2.X


> On Feb. 15, 2018, 5:38 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
> > Lines 2928 (patched)
> > <https://reviews.apache.org/r/65663/diff/1/?file=1961251#file1961251line2928>
> >
> >     Please read comment above, this is not needed.

Not sure what you mean. As far as I can tell mvFile does not inherit 
permissions in the code


> On Feb. 15, 2018, 5:38 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
> > Lines 3181 (patched)
> > <https://reviews.apache.org/r/65663/diff/1/?file=1961251#file1961251line3184>
> >
> >     Because feature is for both perms and grp.

But it takes the dest status, and also preserves /source/ group (from the 
temporary file)


> On Feb. 15, 2018, 5:38 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
> > Line 3182 (original), 3186 (patched)
> > <https://reviews.apache.org/r/65663/diff/1/?file=1961251#file1961251line3189>
> >
> >     These are files. No need for recursive here.

Actually for ACID, some cases of union, etc they may not be files. There was a 
recent bug with DML events about that


> On Feb. 15, 2018, 5:38 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
> > Line 3268 (original), 3278 (patched)
> > <https://reviews.apache.org/r/65663/diff/1/?file=1961251#file1961251line3281>
> >
> >     increasing scope of feature.

This is used when loading partitions and tables after the query and as such can 
easily apply to ACID tables. 
lP/lT is called for DP, etc., and it's easier to add it here then to trace all 
possible paths these calls can take.


- Sergey


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


On Feb. 15, 2018, 1:28 a.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65663/
> -----------------------------------------------------------
> 
> (Updated Feb. 15, 2018, 1:28 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> .
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/common/FileUtils.java 
> 2b7a57bb369f0fc3204157a15dff761c1e73419d 
>   common/src/java/org/apache/hadoop/hive/io/HdfsUtils.java 
> 16fc96eae99705a0625831122afcef3586fe490e 
>   ql/src/java/org/apache/hadoop/hive/ql/Context.java 
> 2ed1c6e00417f713ba8ded16b7591973b7e64271 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/AbstractFileMergeOperator.java 
> 6bba057e764099ff2e6ef974577cb6b42fbecab0 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/CopyTask.java 
> 2683f294f61f5e6e1c553c0095fcc11461229c04 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java 
> a9d03d060adeaa5cad6bef48a63c048f23819d01 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/JoinOperator.java 
> 02827635873ffc319fca40fefbb3cea0fdc4c418 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/MoveTask.java 
> 5cf2c2bc4869fe3f381fd27366a40564eb9c86c2 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ReplCopyTask.java 
> d2f9e79cd8366a4fa7541660c41fe06d72922b13 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/SkewJoinHandler.java 
> cec7c1a8deddd45b44d388e732b45b77fd48a68e 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 
> 614c29bb15557e28523d17d6438830dc9b9d359d 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 
> eefa8f756cdcb7d56ed5ff46f4f0bcbfd170a0cf 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java 
> 5deec4b0f96682dae9b5558ac26060d845044984 
> 
> 
> Diff: https://reviews.apache.org/r/65663/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>

Reply via email to