You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Eugene Koifman <ek...@hortonworks.com> on 2018/02/01 02:24:44 UTC

Re: Review Request 65276: HIVE-18516

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




ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
Lines 3784 (patched)
<https://reviews.apache.org/r/65276/#comment276302>

    how is isFullAcidTable different from isAcid ?
    There should be some java doc if they are indeed different



ql/src/test/org/apache/hadoop/hive/ql/TestTxnLoadData.java
Lines 460 (patched)
<https://reviews.apache.org/r/65276/#comment276303>

    is this left in here intentionally?  checkExpected() logs this to the log file laready



ql/src/test/queries/clientpositive/load_data_acid_rename.q
Lines 1 (patched)
<https://reviews.apache.org/r/65276/#comment276299>

    why is this needed here?  this is deperecated in HiveConf


- Eugene Koifman


On Jan. 31, 2018, 10 p.m., Deepak Jaiswal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65276/
> -----------------------------------------------------------
> 
> (Updated Jan. 31, 2018, 10 p.m.)
> 
> 
> Review request for hive, Eugene Koifman and Jason Dere.
> 
> 
> Bugs: HIVE-18516
>     https://issues.apache.org/jira/browse/HIVE-18516
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> load data should rename files consistent with insert statements for ACID Tables.
> Includes test change for a missed test.
> 
> 
> Diffs
> -----
> 
>   itests/src/test/resources/testconfiguration.properties d86ff58840 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 63bcedc000 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java 5868d4dd56 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnLoadData.java a9cba456ef 
>   ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveCopyFiles.java c6a4a8926b 
>   ql/src/test/queries/clientnegative/load_data_into_acid.q 2ac5b561ae 
>   ql/src/test/queries/clientpositive/load_data_acid_rename.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/smb_mapjoin_7.q 4a6afb0496 
>   ql/src/test/results/clientnegative/load_data_into_acid.q.out 46b5cdd2c8 
>   ql/src/test/results/clientpositive/beeline/smb_mapjoin_7.q.out 7a6f8c53a5 
>   ql/src/test/results/clientpositive/llap/load_data_acid_rename.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/smb_mapjoin_7.q.out b71c5b87c1 
>   ql/src/test/results/clientpositive/spark/smb_mapjoin_7.q.out ac49c02913 
> 
> 
> Diff: https://reviews.apache.org/r/65276/diff/5/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Deepak Jaiswal
> 
>


Re: Review Request 65276: HIVE-18516

Posted by Deepak Jaiswal <dj...@hortonworks.com>.

> On Feb. 1, 2018, 2:24 a.m., Eugene Koifman wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
> > Lines 3784 (patched)
> > <https://reviews.apache.org/r/65276/diff/5/?file=1951001#file1951001line3794>
> >
> >     how is isFullAcidTable different from isAcid ?
> >     There should be some java doc if they are indeed different

isAcid is used only when it is ACID and IUD op only. It is false when it is load data which is the case. I will update the variable name to something better and add comments.


> On Feb. 1, 2018, 2:24 a.m., Eugene Koifman wrote:
> > ql/src/test/org/apache/hadoop/hive/ql/TestTxnLoadData.java
> > Lines 460 (patched)
> > <https://reviews.apache.org/r/65276/diff/5/?file=1951003#file1951003line460>
> >
> >     is this left in here intentionally?  checkExpected() logs this to the log file laready

Thanks for pointing this out. I will remove it.


> On Feb. 1, 2018, 2:24 a.m., Eugene Koifman wrote:
> > ql/src/test/queries/clientpositive/load_data_acid_rename.q
> > Lines 1 (patched)
> > <https://reviews.apache.org/r/65276/diff/5/?file=1951006#file1951006line1>
> >
> >     why is this needed here?  this is deperecated in HiveConf

Thanks. I will remove it.


- Deepak


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


On Jan. 31, 2018, 10 p.m., Deepak Jaiswal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65276/
> -----------------------------------------------------------
> 
> (Updated Jan. 31, 2018, 10 p.m.)
> 
> 
> Review request for hive, Eugene Koifman and Jason Dere.
> 
> 
> Bugs: HIVE-18516
>     https://issues.apache.org/jira/browse/HIVE-18516
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> load data should rename files consistent with insert statements for ACID Tables.
> Includes test change for a missed test.
> 
> 
> Diffs
> -----
> 
>   itests/src/test/resources/testconfiguration.properties d86ff58840 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 63bcedc000 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java 5868d4dd56 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnLoadData.java a9cba456ef 
>   ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveCopyFiles.java c6a4a8926b 
>   ql/src/test/queries/clientnegative/load_data_into_acid.q 2ac5b561ae 
>   ql/src/test/queries/clientpositive/load_data_acid_rename.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/smb_mapjoin_7.q 4a6afb0496 
>   ql/src/test/results/clientnegative/load_data_into_acid.q.out 46b5cdd2c8 
>   ql/src/test/results/clientpositive/beeline/smb_mapjoin_7.q.out 7a6f8c53a5 
>   ql/src/test/results/clientpositive/llap/load_data_acid_rename.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/smb_mapjoin_7.q.out b71c5b87c1 
>   ql/src/test/results/clientpositive/spark/smb_mapjoin_7.q.out ac49c02913 
> 
> 
> Diff: https://reviews.apache.org/r/65276/diff/5/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Deepak Jaiswal
> 
>