> On Feb. 21, 2014, 4:44 a.m., Gunther Hagleitner wrote: > > ql/src/java/org/apache/hadoop/hive/ql/exec/MapJoinOperator.java, line 177 > > <https://reviews.apache.org/r/18230/diff/3/?file=499105#file499105line177> > > > > this seems ugly to me. can't you delegate to the hashtable loader to > > decide which class to use?
this is in the operator, hashtable is already loaded so there's no loader, and key is already decided. We want to ensure that 1) We use the same key as that table. 2) We don't make decision for each key separately rechecking again and again... but to decide based on previous key we need previous key. > On Feb. 21, 2014, 4:44 a.m., Gunther Hagleitner wrote: > > ql/src/java/org/apache/hadoop/hive/ql/exec/MapJoinOperator.java, line 206 > > <https://reviews.apache.org/r/18230/diff/3/?file=499105#file499105line206> > > > > the old code complies with the coding guidelines... can you change back? there are coding guidelines? :) > On Feb. 21, 2014, 4:44 a.m., Gunther Hagleitner wrote: > > ql/src/java/org/apache/hadoop/hive/ql/exec/persistence/MapJoinKey.java, > > line 104 > > <https://reviews.apache.org/r/18230/diff/3/?file=499107#file499107line104> > > > > what causes the warning? cast to List > On Feb. 21, 2014, 4:44 a.m., Gunther Hagleitner wrote: > > ql/src/java/org/apache/hadoop/hive/ql/exec/persistence/MapJoinKeyBase.java, > > line 98 > > <https://reviews.apache.org/r/18230/diff/3/?file=499108#file499108line98> > > > > 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? these methods actually create a class. Ref can be null (for the first key when we determine what to use), and it can be non-reusable (e.g. when loading hashtable because it's the key from table). Should static stuff be moved to a separate factory class? > On Feb. 21, 2014, 4:44 a.m., Gunther Hagleitner wrote: > > ql/src/java/org/apache/hadoop/hive/ql/exec/persistence/MapJoinKeyBytes.java, > > line 64 > > <https://reviews.apache.org/r/18230/diff/3/?file=499109#file499109line64> > > > > 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. These sizes are actually the ones DataOutput implementation below writes. The thing I care about is that serialized keys are comparable. Why would they be impossible to maintain? - Sergey ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18230/#review35132 ----------------------------------------------------------- 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 > >
