Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Plain View
Hive, mail # dev - Review Request: Add Vectorized Substr


+
Timothy Chen 2013-05-13, 21:54
+
Eric Hanson 2013-05-14, 00:21
+
Timothy Chen 2013-05-15, 01:16
+
Eric Hanson 2013-05-15, 18:33
+
Timothy Chen 2013-05-21, 17:56
Copy link to this message
-
Re: Review Request: Add Vectorized Substr
Eric Hanson 2013-05-21, 22:55

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

ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/StringSubstrColStart.java
<https://reviews.apache.org/r/11106/#comment42976>

    setting isNull[0] to any value (true or false) is not necessary here because you set noNulls to true.

ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/StringSubstrColStart.java
<https://reviews.apache.org/r/11106/#comment42974>

    You should only copy n values (the current batch size), not inV.isNull.length. If n<<the default batch size then this code will be a performance problem.
    
    Also, you should only do this if
    !inV.noNulls. Move it inside the if.

ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/StringSubstrColStartLen.java
<https://reviews.apache.org/r/11106/#comment42980>

    this line is not needed because you set outV.noNulls to true

ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/StringSubstrColStartLen.java
<https://reviews.apache.org/r/11106/#comment42981>

    you should only do this if !noNulls. Move inside if statement. Also, only copy n values (batch.size). Not the full array.

ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorStringExpressions.java
<https://reviews.apache.org/r/11106/#comment42982>

    please add some comments to this function to say what the major sections are doing/testing

ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorStringExpressions.java
<https://reviews.apache.org/r/11106/#comment42984>

    explain why this assertion is correct

ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorStringExpressions.java
<https://reviews.apache.org/r/11106/#comment42985>

    explain assertion

ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorStringExpressions.java
<https://reviews.apache.org/r/11106/#comment42983>

    please add some comments to this function

ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorStringExpressions.java
<https://reviews.apache.org/r/11106/#comment42987>

    explain assertion
    

ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorStringExpressions.java
<https://reviews.apache.org/r/11106/#comment42988>

    explain
- Eric Hanson
On May 21, 2013, 5:56 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11106/
> -----------------------------------------------------------
>
> (Updated May 21, 2013, 5:56 p.m.)
>
>
> Review request for hive.
>
>
> Description
> -------
>
> Add Vectorized Substr
>
>
> This addresses bug HIVE-4495.
>     https://issues.apache.org/jira/browse/HIVE-4495
>
>
> Diffs
> -----
>
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/StringSubstrColStart.java PRE-CREATION
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/StringSubstrColStartLen.java PRE-CREATION
>   ql/src/test/org/apache/hadoop/hive/ql/exec/vector/expressions/TestVectorStringExpressions.java 6e26412
>
> Diff: https://reviews.apache.org/r/11106/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Timothy Chen
>
>

+
Timothy Chen 2013-05-22, 01:19
+
Timothy Chen 2013-05-28, 16:28
+
Eric Hanson 2013-05-28, 16:24