----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18230/#review35132 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/exec/MapJoinOperator.java <https://reviews.apache.org/r/18230/#comment65547> this seems ugly to me. can't you delegate to the hashtable loader to decide which class to use? ql/src/java/org/apache/hadoop/hive/ql/exec/MapJoinOperator.java <https://reviews.apache.org/r/18230/#comment65548> the old code complies with the coding guidelines... can you change back? ql/src/java/org/apache/hadoop/hive/ql/exec/persistence/MapJoinKey.java <https://reviews.apache.org/r/18230/#comment65549> what causes the warning? ql/src/java/org/apache/hadoop/hive/ql/exec/persistence/MapJoinKeyBase.java <https://reviews.apache.org/r/18230/#comment65546> Can you name the base class mapjoinkey and the two new implementations something different (mapjoinkeybytes, mapjoinkeyobject or whatever) that way the contract with all the classes using mapjoinkey shouldn't change and there will be fewer code changes. I also think that mapjoinkey should be the base (i.e.: that's what the clients see (impls should have the awkward names) ql/src/java/org/apache/hadoop/hive/ql/exec/persistence/MapJoinKeyBase.java <https://reviews.apache.org/r/18230/#comment65550> looking for subclasses in the base is um not so nice. can't you avoid this static stuff? just make read a member and override the approriate stuff. you're already passing in a ref object, so why not call read on that? ql/src/java/org/apache/hadoop/hive/ql/exec/persistence/MapJoinKeyBytes.java <https://reviews.apache.org/r/18230/#comment65551> this and the next field (vectorized) is going to be neigh impossible to maintain. you really care about fixed length here is that it? we should add this to objectinstpectorutils or something like that. i think there's already some code in hive that returns size of datatype to you. - Gunther Hagleitner On Feb. 20, 2014, 7:46 p.m., Sergey Shelukhin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18230/ > ----------------------------------------------------------- > > (Updated Feb. 20, 2014, 7:46 p.m.) > > > Review request for hive, Gunther Hagleitner and Jitendra Pandey. > > > Repository: hive-git > > > Description > ------- > > See JIRA > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java a182cd7 > ql/src/java/org/apache/hadoop/hive/ql/exec/AbstractMapJoinOperator.java > 3cfaacf > ql/src/java/org/apache/hadoop/hive/ql/exec/HashTableSinkOperator.java > 24f1229 > ql/src/java/org/apache/hadoop/hive/ql/exec/JoinUtil.java 46e37c2 > ql/src/java/org/apache/hadoop/hive/ql/exec/MapJoinOperator.java c0f4cd7 > ql/src/java/org/apache/hadoop/hive/ql/exec/persistence/HashMapWrapper.java > 61545b5 > ql/src/java/org/apache/hadoop/hive/ql/exec/persistence/MapJoinKey.java > 2ac0928 > ql/src/java/org/apache/hadoop/hive/ql/exec/persistence/MapJoinKeyBase.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/exec/persistence/MapJoinKeyBytes.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/exec/persistence/MapJoinTableContainer.java > 9ce0ae6 > > ql/src/java/org/apache/hadoop/hive/ql/exec/persistence/MapJoinTableContainerSerDe.java > 83ba0f0 > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/HashTableLoader.java 47f9d21 > > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorHashKeyWrapperBatch.java > 581046e > > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorMapJoinOperator.java > 2466a3b > > ql/src/test/org/apache/hadoop/hive/ql/exec/persistence/TestMapJoinEqualityTableContainer.java > c541ad2 > ql/src/test/org/apache/hadoop/hive/ql/exec/persistence/TestMapJoinKey.java > a103a51 > ql/src/test/org/apache/hadoop/hive/ql/exec/persistence/Utilities.java > 2cb1ac3 > > Diff: https://reviews.apache.org/r/18230/diff/ > > > Testing > ------- > > > Thanks, > > Sergey Shelukhin > >
