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

Switch to Plain View
Hive >> mail # dev >> Review Request 13059: HIVE-4850 Implement vector mode map join


+
Remus Rusanu 2013-07-30, 11:11
+
Remus Rusanu 2013-10-03, 14:17
+
Remus Rusanu 2013-10-03, 14:20
+
Remus Rusanu 2013-10-09, 13:50
+
Remus Rusanu 2013-10-12, 21:51
+
Eric Hanson 2013-11-11, 20:09
Copy link to this message
-
RE: Review Request 13059: HIVE-4850 Implement vector mode map join
Thanks for the review!
I will probably have to add a JIRA to address them, otherwise I don't have a vehicle for submitting the patch :)

-----Original Message-----
From: Eric Hanson [mailto:[EMAIL PROTECTED]] On Behalf Of Eric Hanson
Sent: Monday, November 11, 2013 10:09 PM
To: Jitendra Pandey; Eric Hanson (SQL SERVER)
Cc: Remus Rusanu; hive
Subject: Re: Review Request 13059: HIVE-4850 Implement vector mode map join
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13059/#review28671
-----------------------------------------------------------

ql/src/java/org/apache/hadoop/hive/ql/exec/vector/ColumnVector.java
<https://reviews.apache.org/r/13059/#comment55588>

    Nice to see good comments

ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorColumnAssign.java
<https://reviews.apache.org/r/13059/#comment55589>

    A comment that explains at a high level where and how this interface is used would be helpful.

ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorColumnAssignFactory.java
<https://reviews.apache.org/r/13059/#comment55593>

    should these fields be marked private?

ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorColumnAssignFactory.java
<https://reviews.apache.org/r/13059/#comment55594>

    Please add a comment that explains this method.
    
    Should this be a new method on ByteColumnVector or can you use BytesColumnVector.setVal()?
    
    setVal automatically extends the internal buffer if needed.

ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorColumnAssignFactory.java
<https://reviews.apache.org/r/13059/#comment55612>

    Please add a comment to explain the purpose of this method
    

ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorColumnAssignFactory.java
<https://reviews.apache.org/r/13059/#comment55617>

    conventions are to put blanks before and after operators =, < etc.

ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorColumnAssignFactory.java
<https://reviews.apache.org/r/13059/#comment55616>

    The Sun Java coding style conventions that are used for Hive say to use this style:
    
    } else [if (...)] {
      ...
    }

ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorColumnAssignFactory.java
<https://reviews.apache.org/r/13059/#comment55618>

    Please add a descriptive comment for this method

ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorColumnAssignFactory.java
<https://reviews.apache.org/r/13059/#comment55622>

    and the what?

ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorMapJoinOperator.java
<https://reviews.apache.org/r/13059/#comment55627>

    remove blank comment?

ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorMapJoinOperator.java
<https://reviews.apache.org/r/13059/#comment55629>

    correct spelling of Vectorization

ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorMapJoinOperator.java
<https://reviews.apache.org/r/13059/#comment55632>

    supper -> super?
    Please explain what out-of-band params are.

ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorMapJoinOperator.java
<https://reviews.apache.org/r/13059/#comment55635>

    colon should be surounded by blanks

ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizedRowBatch.java
<https://reviews.apache.org/r/13059/#comment55653>

    Please add a comment that describes what this method does.

ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java
<https://reviews.apache.org/r/13059/#comment55654>

    Please add a comment

ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java
<https://reviews.apache.org/r/13059/#comment55649>

    Please add a comment explaining what's done by this method

ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java
<https://reviews.apache.org/r/13059/#comment55650>

    Please add a descriptive comment for this method
This looks good. I made a bunch of stylistic comments. Could you also add a page or so of design description to the design document for vectorized query execution attached to HIVE-4160? Thanks Remus. -Eric

- Eric Hanson
On Oct. 12, 2013, 9:51 p.m., Remus Rusanu wrote: