You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Sergey Shelukhin <se...@hortonworks.com> on 2017/11/17 22:42:25 UTC

Review Request 63927: HIVE-17631 upgrade orc to 1.4.0

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

Review request for hive, Jason Dere and Prasanth_J.


Repository: hive-git


Description
-------

see jira


Diffs
-----

  llap-server/src/java/org/apache/hadoop/hive/llap/io/encoded/OrcEncodedDataReader.java c32f79fe95 
  llap-server/src/java/org/apache/hadoop/hive/llap/io/encoded/SerDeEncodedDataReader.java 599b5191c6 
  llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestIncrementalObjectSizeEstimator.java 1ec702011a 
  pom.xml 04fb7c3c55 
  ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java 1e5b841f4b 
  ql/src/java/org/apache/hadoop/hive/ql/io/orc/ReaderImpl.java cbbbb150b6 
  ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/EncodedReaderImpl.java 80b7be8e5a 
  ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/Reader.java d2bb6417a2 
  ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/ReaderImpl.java a916d58f3e 
  ql/src/java/org/apache/hadoop/hive/ql/plan/DynamicValue.java a20328cb69 


Diff: https://reviews.apache.org/r/63927/diff/1/


Testing
-------


Thanks,

Sergey Shelukhin

Re: Review Request 63927: HIVE-17631 upgrade orc to 1.4.0

Posted by j....@gmail.com.

> On Nov. 18, 2017, 12:16 a.m., Prasanth_J wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java
> > Lines 2123 (patched)
> > <https://reviews.apache.org/r/63927/diff/2/?file=1897097#file1897097line2123>
> >
> >     Can you set truthValues[pred] to YES_NO or YES_NO_NULL instead, as some AND filters can still prune stripes.

YES_NO_NULL if nulls present else YES_NO


- Prasanth_J


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


On Nov. 17, 2017, 11:33 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63927/
> -----------------------------------------------------------
> 
> (Updated Nov. 17, 2017, 11:33 p.m.)
> 
> 
> Review request for hive, Jason Dere and Prasanth_J.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   llap-server/src/java/org/apache/hadoop/hive/llap/io/encoded/OrcEncodedDataReader.java c32f79fe95 
>   llap-server/src/java/org/apache/hadoop/hive/llap/io/encoded/SerDeEncodedDataReader.java 599b5191c6 
>   llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestIncrementalObjectSizeEstimator.java 1ec702011a 
>   pom.xml 04fb7c3c55 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java 1e5b841f4b 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/ReaderImpl.java cbbbb150b6 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/EncodedReaderImpl.java 80b7be8e5a 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/Reader.java d2bb6417a2 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/ReaderImpl.java a916d58f3e 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/DynamicValue.java a20328cb69 
> 
> 
> Diff: https://reviews.apache.org/r/63927/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>

Re: Review Request 63927: HIVE-17631 upgrade orc to 1.4.0

Posted by j....@gmail.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63927/#review191419
-----------------------------------------------------------




ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java
Lines 2123 (patched)
<https://reviews.apache.org/r/63927/#comment269235>

    Can you set truthValues[pred] to YES_NO or YES_NO_NULL instead, as some AND filters can still prune stripes.


- Prasanth_J


On Nov. 17, 2017, 11:33 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63927/
> -----------------------------------------------------------
> 
> (Updated Nov. 17, 2017, 11:33 p.m.)
> 
> 
> Review request for hive, Jason Dere and Prasanth_J.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   llap-server/src/java/org/apache/hadoop/hive/llap/io/encoded/OrcEncodedDataReader.java c32f79fe95 
>   llap-server/src/java/org/apache/hadoop/hive/llap/io/encoded/SerDeEncodedDataReader.java 599b5191c6 
>   llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestIncrementalObjectSizeEstimator.java 1ec702011a 
>   pom.xml 04fb7c3c55 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java 1e5b841f4b 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/ReaderImpl.java cbbbb150b6 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/EncodedReaderImpl.java 80b7be8e5a 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/Reader.java d2bb6417a2 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/ReaderImpl.java a916d58f3e 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/DynamicValue.java a20328cb69 
> 
> 
> Diff: https://reviews.apache.org/r/63927/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>

Re: Review Request 63927: HIVE-17631 upgrade orc to 1.4.0

Posted by j....@gmail.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63927/#review191430
-----------------------------------------------------------


Ship it!




Ship It!

- Prasanth_J


On Nov. 18, 2017, 1:24 a.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63927/
> -----------------------------------------------------------
> 
> (Updated Nov. 18, 2017, 1:24 a.m.)
> 
> 
> Review request for hive, Jason Dere and Prasanth_J.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   llap-server/src/java/org/apache/hadoop/hive/llap/io/encoded/OrcEncodedDataReader.java c32f79fe95 
>   llap-server/src/java/org/apache/hadoop/hive/llap/io/encoded/SerDeEncodedDataReader.java 599b5191c6 
>   llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestIncrementalObjectSizeEstimator.java 1ec702011a 
>   pom.xml 04fb7c3c55 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java 1e5b841f4b 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/ReaderImpl.java cbbbb150b6 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/EncodedReaderImpl.java 80b7be8e5a 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/Reader.java d2bb6417a2 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/ReaderImpl.java a916d58f3e 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/DynamicValue.java a20328cb69 
> 
> 
> Diff: https://reviews.apache.org/r/63927/diff/3/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>

Re: Review Request 63927: HIVE-17631 upgrade orc to 1.4.0

Posted by j....@gmail.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63927/#review191431
-----------------------------------------------------------




ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java
Line 2124 (original), 2126 (patched)
<https://reviews.apache.org/r/63927/#comment269248>

    Not all versions of orc has hasNull flag in proto. So hasHasNull has to be verified before .hasNull()? If it doesn't have hasNull flag it will just return false which is wrong as it may or may not have nulls.


- Prasanth_J


On Nov. 18, 2017, 1:24 a.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63927/
> -----------------------------------------------------------
> 
> (Updated Nov. 18, 2017, 1:24 a.m.)
> 
> 
> Review request for hive, Jason Dere and Prasanth_J.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   llap-server/src/java/org/apache/hadoop/hive/llap/io/encoded/OrcEncodedDataReader.java c32f79fe95 
>   llap-server/src/java/org/apache/hadoop/hive/llap/io/encoded/SerDeEncodedDataReader.java 599b5191c6 
>   llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestIncrementalObjectSizeEstimator.java 1ec702011a 
>   pom.xml 04fb7c3c55 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java 1e5b841f4b 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/ReaderImpl.java cbbbb150b6 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/EncodedReaderImpl.java 80b7be8e5a 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/Reader.java d2bb6417a2 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/ReaderImpl.java a916d58f3e 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/DynamicValue.java a20328cb69 
> 
> 
> Diff: https://reviews.apache.org/r/63927/diff/3/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>

Re: Review Request 63927: HIVE-17631 upgrade orc to 1.4.0

Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63927/
-----------------------------------------------------------

(Updated Nov. 18, 2017, 1:24 a.m.)


Review request for hive, Jason Dere and Prasanth_J.


Repository: hive-git


Description
-------

see jira


Diffs (updated)
-----

  llap-server/src/java/org/apache/hadoop/hive/llap/io/encoded/OrcEncodedDataReader.java c32f79fe95 
  llap-server/src/java/org/apache/hadoop/hive/llap/io/encoded/SerDeEncodedDataReader.java 599b5191c6 
  llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestIncrementalObjectSizeEstimator.java 1ec702011a 
  pom.xml 04fb7c3c55 
  ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java 1e5b841f4b 
  ql/src/java/org/apache/hadoop/hive/ql/io/orc/ReaderImpl.java cbbbb150b6 
  ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/EncodedReaderImpl.java 80b7be8e5a 
  ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/Reader.java d2bb6417a2 
  ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/ReaderImpl.java a916d58f3e 
  ql/src/java/org/apache/hadoop/hive/ql/plan/DynamicValue.java a20328cb69 


Diff: https://reviews.apache.org/r/63927/diff/3/

Changes: https://reviews.apache.org/r/63927/diff/2-3/


Testing
-------


Thanks,

Sergey Shelukhin

Re: Review Request 63927: HIVE-17631 upgrade orc to 1.4.0

Posted by Sergey Shelukhin <se...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63927/
-----------------------------------------------------------

(Updated Nov. 17, 2017, 11:33 p.m.)


Review request for hive, Jason Dere and Prasanth_J.


Repository: hive-git


Description
-------

see jira


Diffs (updated)
-----

  llap-server/src/java/org/apache/hadoop/hive/llap/io/encoded/OrcEncodedDataReader.java c32f79fe95 
  llap-server/src/java/org/apache/hadoop/hive/llap/io/encoded/SerDeEncodedDataReader.java 599b5191c6 
  llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestIncrementalObjectSizeEstimator.java 1ec702011a 
  pom.xml 04fb7c3c55 
  ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java 1e5b841f4b 
  ql/src/java/org/apache/hadoop/hive/ql/io/orc/ReaderImpl.java cbbbb150b6 
  ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/EncodedReaderImpl.java 80b7be8e5a 
  ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/Reader.java d2bb6417a2 
  ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/ReaderImpl.java a916d58f3e 
  ql/src/java/org/apache/hadoop/hive/ql/plan/DynamicValue.java a20328cb69 


Diff: https://reviews.apache.org/r/63927/diff/2/

Changes: https://reviews.apache.org/r/63927/diff/1-2/


Testing
-------


Thanks,

Sergey Shelukhin

Re: Review Request 63927: HIVE-17631 upgrade orc to 1.4.0

Posted by Sergey Shelukhin <se...@hortonworks.com>.

> On Nov. 17, 2017, 10:56 p.m., Prasanth_J wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java
> > Lines 2124 (patched)
> > <https://reviews.apache.org/r/63927/diff/1/?file=1897079#file1897079line2124>
> >
> >     This is handled by ORC-148 already. Any exception will not prune RG/Stripe.

Doesn't work, tests fail


- Sergey


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


On Nov. 17, 2017, 10:42 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63927/
> -----------------------------------------------------------
> 
> (Updated Nov. 17, 2017, 10:42 p.m.)
> 
> 
> Review request for hive, Jason Dere and Prasanth_J.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   llap-server/src/java/org/apache/hadoop/hive/llap/io/encoded/OrcEncodedDataReader.java c32f79fe95 
>   llap-server/src/java/org/apache/hadoop/hive/llap/io/encoded/SerDeEncodedDataReader.java 599b5191c6 
>   llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestIncrementalObjectSizeEstimator.java 1ec702011a 
>   pom.xml 04fb7c3c55 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java 1e5b841f4b 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/ReaderImpl.java cbbbb150b6 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/EncodedReaderImpl.java 80b7be8e5a 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/Reader.java d2bb6417a2 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/ReaderImpl.java a916d58f3e 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/DynamicValue.java a20328cb69 
> 
> 
> Diff: https://reviews.apache.org/r/63927/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>

Re: Review Request 63927: HIVE-17631 upgrade orc to 1.4.0

Posted by j....@gmail.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63927/#review191405
-----------------------------------------------------------




pom.xml
Line 185 (original), 185 (patched)
<https://reviews.apache.org/r/63927/#comment269214>

    Use 1.4.1?



ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java
Lines 2124 (patched)
<https://reviews.apache.org/r/63927/#comment269218>

    This is handled by ORC-148 already. Any exception will not prune RG/Stripe.



ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/EncodedReaderImpl.java
Lines 147 (patched)
<https://reviews.apache.org/r/63927/#comment269215>

    looks like its already returned in close()? plz remove the TODO if so


- Prasanth_J


On Nov. 17, 2017, 10:42 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63927/
> -----------------------------------------------------------
> 
> (Updated Nov. 17, 2017, 10:42 p.m.)
> 
> 
> Review request for hive, Jason Dere and Prasanth_J.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   llap-server/src/java/org/apache/hadoop/hive/llap/io/encoded/OrcEncodedDataReader.java c32f79fe95 
>   llap-server/src/java/org/apache/hadoop/hive/llap/io/encoded/SerDeEncodedDataReader.java 599b5191c6 
>   llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestIncrementalObjectSizeEstimator.java 1ec702011a 
>   pom.xml 04fb7c3c55 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcInputFormat.java 1e5b841f4b 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/ReaderImpl.java cbbbb150b6 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/EncodedReaderImpl.java 80b7be8e5a 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/Reader.java d2bb6417a2 
>   ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/ReaderImpl.java a916d58f3e 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/DynamicValue.java a20328cb69 
> 
> 
> Diff: https://reviews.apache.org/r/63927/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>