Re: Review Request 27933: HIVE-8810 Make HashTableSinkOperator works for Spark Branch [Spark Branch]

2014-11-13 Thread Jimmy Xiang

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

(Updated Nov. 13, 2014, 7:17 p.m.)


Review request for hive, Chao Sun, Szehon Ho, and Xuefu Zhang.


Changes
---

Reverted some changes to HashTableSinkOperator in HIVE-8621. Changed patch v2 
so that we can reuse some code. Now SparkHashtableSinkOperator uses 
HashTableSinkOperator and SparkHashTableSinkDesc extends HashTableSinkDesc.


Bugs: HIVE-8810
https://issues.apache.org/jira/browse/HIVE-8810


Repository: hive-git


Description
---

Fixed the Spark HashTableSinkOperator


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/HashTableSinkOperator.java 78d9012 
  ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorFactory.java f1c3564 
  ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/exec/spark/HashTableLoader.java 95dad90 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/spark/SparkReduceSinkMapJoinProc.java
 a58a6c5 
  ql/src/java/org/apache/hadoop/hive/ql/plan/MapredLocalWork.java d566087 
  ql/src/java/org/apache/hadoop/hive/ql/plan/SparkHashTableSinkDesc.java 
PRE-CREATION 

Diff: https://reviews.apache.org/r/27933/diff/


Testing
---


Thanks,

Jimmy Xiang



Re: Review Request 27933: HIVE-8810 Make HashTableSinkOperator works for Spark Branch [Spark Branch]

2014-11-13 Thread Xuefu Zhang

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



ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java
https://reviews.apache.org/r/27933/#comment102853

An assert might help.


- Xuefu Zhang


On Nov. 13, 2014, 7:17 p.m., Jimmy Xiang wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27933/
 ---
 
 (Updated Nov. 13, 2014, 7:17 p.m.)
 
 
 Review request for hive, Chao Sun, Szehon Ho, and Xuefu Zhang.
 
 
 Bugs: HIVE-8810
 https://issues.apache.org/jira/browse/HIVE-8810
 
 
 Repository: hive-git
 
 
 Description
 ---
 
 Fixed the Spark HashTableSinkOperator
 
 
 Diffs
 -
 
   ql/src/java/org/apache/hadoop/hive/ql/exec/HashTableSinkOperator.java 
 78d9012 
   ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorFactory.java f1c3564 
   ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java 
 PRE-CREATION 
   ql/src/java/org/apache/hadoop/hive/ql/exec/spark/HashTableLoader.java 
 95dad90 
   
 ql/src/java/org/apache/hadoop/hive/ql/optimizer/spark/SparkReduceSinkMapJoinProc.java
  a58a6c5 
   ql/src/java/org/apache/hadoop/hive/ql/plan/MapredLocalWork.java d566087 
   ql/src/java/org/apache/hadoop/hive/ql/plan/SparkHashTableSinkDesc.java 
 PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27933/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jimmy Xiang
 




Re: Review Request 27933: HIVE-8810 Make HashTableSinkOperator works for Spark Branch [Spark Branch]

2014-11-12 Thread Jimmy Xiang

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

(Updated Nov. 12, 2014, 10:35 p.m.)


Review request for hive, Chao Sun, Szehon Ho, and Xuefu Zhang.


Bugs: HIVE-8810
https://issues.apache.org/jira/browse/HIVE-8810


Repository: hive-git


Description
---

Fixed the Spark HashTableSinkOperator


Diffs
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/HashTableSinkOperator.java 78d9012 
  ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorFactory.java f1c3564 
  ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java 
PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/spark/SparkReduceSinkMapJoinProc.java
 a58a6c5 
  ql/src/java/org/apache/hadoop/hive/ql/plan/SparkHashTableSinkDesc.java 
PRE-CREATION 

Diff: https://reviews.apache.org/r/27933/diff/


Testing
---


Thanks,

Jimmy Xiang



Re: Review Request 27933: HIVE-8810 Make HashTableSinkOperator works for Spark Branch [Spark Branch]

2014-11-12 Thread Xuefu Zhang

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



ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java
https://reviews.apache.org/r/27933/#comment102607

Could we extend from HashTableSinkOperator instead?



ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java
https://reviews.apache.org/r/27933/#comment102577

Nit: Please remove trailing spaces/tabs.



ql/src/java/org/apache/hadoop/hive/ql/plan/SparkHashTableSinkDesc.java
https://reviews.apache.org/r/27933/#comment102599

This class seems to be identiccal to HashTableSinkDesc. Could we extend 
from it instead?


- Xuefu Zhang


On Nov. 12, 2014, 10:35 p.m., Jimmy Xiang wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27933/
 ---
 
 (Updated Nov. 12, 2014, 10:35 p.m.)
 
 
 Review request for hive, Chao Sun, Szehon Ho, and Xuefu Zhang.
 
 
 Bugs: HIVE-8810
 https://issues.apache.org/jira/browse/HIVE-8810
 
 
 Repository: hive-git
 
 
 Description
 ---
 
 Fixed the Spark HashTableSinkOperator
 
 
 Diffs
 -
 
   ql/src/java/org/apache/hadoop/hive/ql/exec/HashTableSinkOperator.java 
 78d9012 
   ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorFactory.java f1c3564 
   ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java 
 PRE-CREATION 
   
 ql/src/java/org/apache/hadoop/hive/ql/optimizer/spark/SparkReduceSinkMapJoinProc.java
  a58a6c5 
   ql/src/java/org/apache/hadoop/hive/ql/plan/SparkHashTableSinkDesc.java 
 PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27933/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jimmy Xiang
 




Re: Review Request 27933: HIVE-8810 Make HashTableSinkOperator works for Spark Branch [Spark Branch]

2014-11-12 Thread Jimmy Xiang

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

(Updated Nov. 12, 2014, 11:58 p.m.)


Review request for hive, Chao Sun, Szehon Ho, and Xuefu Zhang.


Changes
---

Addressed the issues pointed out by Xuefu.


Bugs: HIVE-8810
https://issues.apache.org/jira/browse/HIVE-8810


Repository: hive-git


Description
---

Fixed the Spark HashTableSinkOperator


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/HashTableSinkOperator.java 78d9012 
  ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorFactory.java f1c3564 
  ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java 
PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/spark/SparkReduceSinkMapJoinProc.java
 a58a6c5 
  ql/src/java/org/apache/hadoop/hive/ql/plan/SparkHashTableSinkDesc.java 
PRE-CREATION 

Diff: https://reviews.apache.org/r/27933/diff/


Testing
---


Thanks,

Jimmy Xiang



Re: Review Request 27933: HIVE-8810 Make HashTableSinkOperator works for Spark Branch [Spark Branch]

2014-11-12 Thread Jimmy Xiang


 On Nov. 12, 2014, 11:47 p.m., Xuefu Zhang wrote:
  ql/src/java/org/apache/hadoop/hive/ql/plan/SparkHashTableSinkDesc.java, 
  line 34
  https://reviews.apache.org/r/27933/diff/1/?file=760617#file760617line34
 
  This class seems to be identiccal to HashTableSinkDesc. Could we extend 
  from it instead?

Yes, will fix this.


 On Nov. 12, 2014, 11:47 p.m., Xuefu Zhang wrote:
  ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java, 
  line 57
  https://reviews.apache.org/r/27933/diff/1/?file=760615#file760615line57
 
  Could we extend from HashTableSinkOperator instead?

Probably we couldn't do that. We have some type issue. HashTableSinkOperator 
extends TerminalOperatorHashTableSinkDesc instead.


- Jimmy


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


On Nov. 12, 2014, 11:58 p.m., Jimmy Xiang wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27933/
 ---
 
 (Updated Nov. 12, 2014, 11:58 p.m.)
 
 
 Review request for hive, Chao Sun, Szehon Ho, and Xuefu Zhang.
 
 
 Bugs: HIVE-8810
 https://issues.apache.org/jira/browse/HIVE-8810
 
 
 Repository: hive-git
 
 
 Description
 ---
 
 Fixed the Spark HashTableSinkOperator
 
 
 Diffs
 -
 
   ql/src/java/org/apache/hadoop/hive/ql/exec/HashTableSinkOperator.java 
 78d9012 
   ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorFactory.java f1c3564 
   ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java 
 PRE-CREATION 
   
 ql/src/java/org/apache/hadoop/hive/ql/optimizer/spark/SparkReduceSinkMapJoinProc.java
  a58a6c5 
   ql/src/java/org/apache/hadoop/hive/ql/plan/SparkHashTableSinkDesc.java 
 PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27933/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jimmy Xiang
 




Re: Review Request 27933: HIVE-8810 Make HashTableSinkOperator works for Spark Branch [Spark Branch]

2014-11-12 Thread Chao Sun

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



ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java
https://reviews.apache.org/r/27933/#comment102640

Don't need this check anymore.



ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java
https://reviews.apache.org/r/27933/#comment102639

Can we use SPARKHASHTABLESINK, or something similar?


- Chao Sun


On Nov. 12, 2014, 11:58 p.m., Jimmy Xiang wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27933/
 ---
 
 (Updated Nov. 12, 2014, 11:58 p.m.)
 
 
 Review request for hive, Chao Sun, Szehon Ho, and Xuefu Zhang.
 
 
 Bugs: HIVE-8810
 https://issues.apache.org/jira/browse/HIVE-8810
 
 
 Repository: hive-git
 
 
 Description
 ---
 
 Fixed the Spark HashTableSinkOperator
 
 
 Diffs
 -
 
   ql/src/java/org/apache/hadoop/hive/ql/exec/HashTableSinkOperator.java 
 78d9012 
   ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorFactory.java f1c3564 
   ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java 
 PRE-CREATION 
   
 ql/src/java/org/apache/hadoop/hive/ql/optimizer/spark/SparkReduceSinkMapJoinProc.java
  a58a6c5 
   ql/src/java/org/apache/hadoop/hive/ql/plan/SparkHashTableSinkDesc.java 
 PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27933/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jimmy Xiang
 




Re: Review Request 27933: HIVE-8810 Make HashTableSinkOperator works for Spark Branch [Spark Branch]

2014-11-12 Thread Jimmy Xiang


 On Nov. 13, 2014, 12:34 a.m., Chao Sun wrote:
  ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java, 
  line 274
  https://reviews.apache.org/r/27933/diff/2/?file=760734#file760734line274
 
  Don't need this check anymore.

That's right. No need for this check anymore. Will fix.


 On Nov. 13, 2014, 12:34 a.m., Chao Sun wrote:
  ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java, 
  line 326
  https://reviews.apache.org/r/27933/diff/2/?file=760734#file760734line326
 
  Can we use SPARKHASHTABLESINK, or something similar?

Does this need match the Operator type?


- Jimmy


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


On Nov. 12, 2014, 11:58 p.m., Jimmy Xiang wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27933/
 ---
 
 (Updated Nov. 12, 2014, 11:58 p.m.)
 
 
 Review request for hive, Chao Sun, Szehon Ho, and Xuefu Zhang.
 
 
 Bugs: HIVE-8810
 https://issues.apache.org/jira/browse/HIVE-8810
 
 
 Repository: hive-git
 
 
 Description
 ---
 
 Fixed the Spark HashTableSinkOperator
 
 
 Diffs
 -
 
   ql/src/java/org/apache/hadoop/hive/ql/exec/HashTableSinkOperator.java 
 78d9012 
   ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorFactory.java f1c3564 
   ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java 
 PRE-CREATION 
   
 ql/src/java/org/apache/hadoop/hive/ql/optimizer/spark/SparkReduceSinkMapJoinProc.java
  a58a6c5 
   ql/src/java/org/apache/hadoop/hive/ql/plan/SparkHashTableSinkDesc.java 
 PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27933/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jimmy Xiang
 




Re: Review Request 27933: HIVE-8810 Make HashTableSinkOperator works for Spark Branch [Spark Branch]

2014-11-12 Thread Chao Sun


 On Nov. 13, 2014, 12:34 a.m., Chao Sun wrote:
  ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java, 
  line 326
  https://reviews.apache.org/r/27933/diff/2/?file=760734#file760734line326
 
  Can we use SPARKHASHTABLESINK, or something similar?
 
 Jimmy Xiang wrote:
 Does this need match the Operator type?

I think these two are not related. Somebody can correct me if I'm wrong.
One potential issue with using the same name is that RuleRegExp may become 
harder to define.


- Chao


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


On Nov. 12, 2014, 11:58 p.m., Jimmy Xiang wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27933/
 ---
 
 (Updated Nov. 12, 2014, 11:58 p.m.)
 
 
 Review request for hive, Chao Sun, Szehon Ho, and Xuefu Zhang.
 
 
 Bugs: HIVE-8810
 https://issues.apache.org/jira/browse/HIVE-8810
 
 
 Repository: hive-git
 
 
 Description
 ---
 
 Fixed the Spark HashTableSinkOperator
 
 
 Diffs
 -
 
   ql/src/java/org/apache/hadoop/hive/ql/exec/HashTableSinkOperator.java 
 78d9012 
   ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorFactory.java f1c3564 
   ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java 
 PRE-CREATION 
   
 ql/src/java/org/apache/hadoop/hive/ql/optimizer/spark/SparkReduceSinkMapJoinProc.java
  a58a6c5 
   ql/src/java/org/apache/hadoop/hive/ql/plan/SparkHashTableSinkDesc.java 
 PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27933/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jimmy Xiang
 




Re: Review Request 27933: HIVE-8810 Make HashTableSinkOperator works for Spark Branch [Spark Branch]

2014-11-12 Thread Suhas Satish


 On Nov. 13, 2014, 12:34 a.m., Chao Sun wrote:
  ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java, 
  line 326
  https://reviews.apache.org/r/27933/diff/2/?file=760734#file760734line326
 
  Can we use SPARKHASHTABLESINK, or something similar?
 
 Jimmy Xiang wrote:
 Does this need match the Operator type?
 
 Chao Sun wrote:
 I think these two are not related. Somebody can correct me if I'm wrong.
 One potential issue with using the same name is that RuleRegExp may 
 become harder to define.
 
 Jimmy Xiang wrote:
 Will change SparkHashtableSinkOperator to extend HashtableSinkOperator.  
 I prefer not to change the name. In Spark, is it possible a hashtable sink 
 not a SparkHashtableSinkOperator?

As Chao correctly pointed out, changing the Operator Name may affect any 
RuleRegExp that relies on that name. But I havent come across any specific 
rules for HashTableSinks yet. Even I prefer to keep the same name. 

Jimmy, is your question to have a hash table sink without having an operator ?


- Suhas


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


On Nov. 12, 2014, 11:58 p.m., Jimmy Xiang wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27933/
 ---
 
 (Updated Nov. 12, 2014, 11:58 p.m.)
 
 
 Review request for hive, Chao Sun, Szehon Ho, and Xuefu Zhang.
 
 
 Bugs: HIVE-8810
 https://issues.apache.org/jira/browse/HIVE-8810
 
 
 Repository: hive-git
 
 
 Description
 ---
 
 Fixed the Spark HashTableSinkOperator
 
 
 Diffs
 -
 
   ql/src/java/org/apache/hadoop/hive/ql/exec/HashTableSinkOperator.java 
 78d9012 
   ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorFactory.java f1c3564 
   ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java 
 PRE-CREATION 
   
 ql/src/java/org/apache/hadoop/hive/ql/optimizer/spark/SparkReduceSinkMapJoinProc.java
  a58a6c5 
   ql/src/java/org/apache/hadoop/hive/ql/plan/SparkHashTableSinkDesc.java 
 PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27933/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jimmy Xiang
 




Re: Review Request 27933: HIVE-8810 Make HashTableSinkOperator works for Spark Branch [Spark Branch]

2014-11-12 Thread Jimmy Xiang


 On Nov. 12, 2014, 11:47 p.m., Xuefu Zhang wrote:
  ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java, 
  line 57
  https://reviews.apache.org/r/27933/diff/1/?file=760615#file760615line57
 
  Could we extend from HashTableSinkOperator instead?
 
 Jimmy Xiang wrote:
 Probably we couldn't do that. We have some type issue. 
 HashTableSinkOperator extends TerminalOperatorHashTableSinkDesc instead.
 
 Xuefu Zhang wrote:
 You can change definition for HashTableSinkOperator as:
 
 public class HashTableSinkOperatorT extends HashTableSinkDesc extends 
 TerminalOperatorHashTableSinkDesc implements Serializable;
 
 Then, you can extend SparkHashTableSinkOperator from it:
 
 public class SparkHashTableSinkOperator extends 
 HashTableSinkOperatorSparkHashTableSinkDesc;
 
 Given the amount of code duplication, I think this is worth it.

Ok, will do that. Also need to make some private members in 
HashTableSinkOperator protected.


- Jimmy


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


On Nov. 12, 2014, 11:58 p.m., Jimmy Xiang wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27933/
 ---
 
 (Updated Nov. 12, 2014, 11:58 p.m.)
 
 
 Review request for hive, Chao Sun, Szehon Ho, and Xuefu Zhang.
 
 
 Bugs: HIVE-8810
 https://issues.apache.org/jira/browse/HIVE-8810
 
 
 Repository: hive-git
 
 
 Description
 ---
 
 Fixed the Spark HashTableSinkOperator
 
 
 Diffs
 -
 
   ql/src/java/org/apache/hadoop/hive/ql/exec/HashTableSinkOperator.java 
 78d9012 
   ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorFactory.java f1c3564 
   ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java 
 PRE-CREATION 
   
 ql/src/java/org/apache/hadoop/hive/ql/optimizer/spark/SparkReduceSinkMapJoinProc.java
  a58a6c5 
   ql/src/java/org/apache/hadoop/hive/ql/plan/SparkHashTableSinkDesc.java 
 PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27933/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jimmy Xiang
 




Re: Review Request 27933: HIVE-8810 Make HashTableSinkOperator works for Spark Branch [Spark Branch]

2014-11-12 Thread Jimmy Xiang


 On Nov. 13, 2014, 12:34 a.m., Chao Sun wrote:
  ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java, 
  line 326
  https://reviews.apache.org/r/27933/diff/2/?file=760734#file760734line326
 
  Can we use SPARKHASHTABLESINK, or something similar?
 
 Jimmy Xiang wrote:
 Does this need match the Operator type?
 
 Chao Sun wrote:
 I think these two are not related. Somebody can correct me if I'm wrong.
 One potential issue with using the same name is that RuleRegExp may 
 become harder to define.

Will change SparkHashtableSinkOperator to extend HashtableSinkOperator.  I 
prefer not to change the name. In Spark, is it possible a hashtable sink not a 
SparkHashtableSinkOperator?


- Jimmy


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


On Nov. 12, 2014, 11:58 p.m., Jimmy Xiang wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27933/
 ---
 
 (Updated Nov. 12, 2014, 11:58 p.m.)
 
 
 Review request for hive, Chao Sun, Szehon Ho, and Xuefu Zhang.
 
 
 Bugs: HIVE-8810
 https://issues.apache.org/jira/browse/HIVE-8810
 
 
 Repository: hive-git
 
 
 Description
 ---
 
 Fixed the Spark HashTableSinkOperator
 
 
 Diffs
 -
 
   ql/src/java/org/apache/hadoop/hive/ql/exec/HashTableSinkOperator.java 
 78d9012 
   ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorFactory.java f1c3564 
   ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java 
 PRE-CREATION 
   
 ql/src/java/org/apache/hadoop/hive/ql/optimizer/spark/SparkReduceSinkMapJoinProc.java
  a58a6c5 
   ql/src/java/org/apache/hadoop/hive/ql/plan/SparkHashTableSinkDesc.java 
 PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27933/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jimmy Xiang
 




Re: Review Request 27933: HIVE-8810 Make HashTableSinkOperator works for Spark Branch [Spark Branch]

2014-11-12 Thread Xuefu Zhang


 On Nov. 12, 2014, 11:47 p.m., Xuefu Zhang wrote:
  ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java, 
  line 57
  https://reviews.apache.org/r/27933/diff/1/?file=760615#file760615line57
 
  Could we extend from HashTableSinkOperator instead?
 
 Jimmy Xiang wrote:
 Probably we couldn't do that. We have some type issue. 
 HashTableSinkOperator extends TerminalOperatorHashTableSinkDesc instead.

You can change definition for HashTableSinkOperator as:

public class HashTableSinkOperatorT extends HashTableSinkDesc extends 
TerminalOperatorHashTableSinkDesc implements Serializable;

Then, you can extend SparkHashTableSinkOperator from it:

public class SparkHashTableSinkOperator extends 
HashTableSinkOperatorSparkHashTableSinkDesc;

Given the amount of code duplication, I think this is worth it.


- Xuefu


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


On Nov. 12, 2014, 11:58 p.m., Jimmy Xiang wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27933/
 ---
 
 (Updated Nov. 12, 2014, 11:58 p.m.)
 
 
 Review request for hive, Chao Sun, Szehon Ho, and Xuefu Zhang.
 
 
 Bugs: HIVE-8810
 https://issues.apache.org/jira/browse/HIVE-8810
 
 
 Repository: hive-git
 
 
 Description
 ---
 
 Fixed the Spark HashTableSinkOperator
 
 
 Diffs
 -
 
   ql/src/java/org/apache/hadoop/hive/ql/exec/HashTableSinkOperator.java 
 78d9012 
   ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorFactory.java f1c3564 
   ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java 
 PRE-CREATION 
   
 ql/src/java/org/apache/hadoop/hive/ql/optimizer/spark/SparkReduceSinkMapJoinProc.java
  a58a6c5 
   ql/src/java/org/apache/hadoop/hive/ql/plan/SparkHashTableSinkDesc.java 
 PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27933/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jimmy Xiang
 




Re: Review Request 27933: HIVE-8810 Make HashTableSinkOperator works for Spark Branch [Spark Branch]

2014-11-12 Thread Jimmy Xiang


 On Nov. 13, 2014, 12:34 a.m., Chao Sun wrote:
  ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java, 
  line 326
  https://reviews.apache.org/r/27933/diff/2/?file=760734#file760734line326
 
  Can we use SPARKHASHTABLESINK, or something similar?
 
 Jimmy Xiang wrote:
 Does this need match the Operator type?
 
 Chao Sun wrote:
 I think these two are not related. Somebody can correct me if I'm wrong.
 One potential issue with using the same name is that RuleRegExp may 
 become harder to define.
 
 Jimmy Xiang wrote:
 Will change SparkHashtableSinkOperator to extend HashtableSinkOperator.  
 I prefer not to change the name. In Spark, is it possible a hashtable sink 
 not a SparkHashtableSinkOperator?
 
 Suhas Satish wrote:
 As Chao correctly pointed out, changing the Operator Name may affect any 
 RuleRegExp that relies on that name. But I havent come across any specific 
 rules for HashTableSinks yet. Even I prefer to keep the same name. 
 
 Jimmy, is your question to have a hash table sink without having an 
 operator ?

I meant that all hastable sink operator should be SparkHashtableSinkOperator 
with Spark. I checked the suggestion Xuefu mentioned for 
SparkHashtableSinkOperator to extend HashtableSinkOperator. It has some problem 
with the OperatorFactory. I am thinking to use HashtableSinkOperator instead, 
so no new class SparkHashtableSinkOperator.


- Jimmy


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


On Nov. 12, 2014, 11:58 p.m., Jimmy Xiang wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/27933/
 ---
 
 (Updated Nov. 12, 2014, 11:58 p.m.)
 
 
 Review request for hive, Chao Sun, Szehon Ho, and Xuefu Zhang.
 
 
 Bugs: HIVE-8810
 https://issues.apache.org/jira/browse/HIVE-8810
 
 
 Repository: hive-git
 
 
 Description
 ---
 
 Fixed the Spark HashTableSinkOperator
 
 
 Diffs
 -
 
   ql/src/java/org/apache/hadoop/hive/ql/exec/HashTableSinkOperator.java 
 78d9012 
   ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorFactory.java f1c3564 
   ql/src/java/org/apache/hadoop/hive/ql/exec/SparkHashTableSinkOperator.java 
 PRE-CREATION 
   
 ql/src/java/org/apache/hadoop/hive/ql/optimizer/spark/SparkReduceSinkMapJoinProc.java
  a58a6c5 
   ql/src/java/org/apache/hadoop/hive/ql/plan/SparkHashTableSinkDesc.java 
 PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/27933/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jimmy Xiang