> On Jan. 15, 2014, 5:06 p.m., Xuefu Zhang wrote: > > Look good to me. Two comments: > > > > 1. Could we change the variable name and method name from ...URI to ...Path > > or ...Dir to reflect the meaning? > > 2. I saw a few places that use Path.toUri().toString() instead of > > Path.toString(). Any catch here?
1. Yup will rename methods and variables. 2. Not really. AFAIK, there is no catch. Regardless, at all of those places also we should be using path (no string anywhere). So, those will be gone in subsequent patches as well. - Ashutosh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16899/#review31879 ----------------------------------------------------------- On Jan. 15, 2014, 5:45 a.m., Ashutosh Chauhan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16899/ > ----------------------------------------------------------- > > (Updated Jan. 15, 2014, 5:45 a.m.) > > > Review request for hive. > > > Bugs: HIVE-6197 > https://issues.apache.org/jira/browse/HIVE-6197 > > > Repository: hive > > > Description > ------- > > Refactor patch. > > > Diffs > ----- > > trunk/ql/src/java/org/apache/hadoop/hive/ql/Context.java 1558249 > trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/HashTableSinkOperator.java > 1558249 > trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 1558249 > trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecDriver.java 1558249 > trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/mr/HashTableLoader.java > 1558249 > trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/mr/MapredLocalTask.java > 1558249 > trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/tez/DagUtils.java 1558249 > > trunk/ql/src/java/org/apache/hadoop/hive/ql/index/bitmap/BitmapIndexHandler.java > 1558249 > > trunk/ql/src/java/org/apache/hadoop/hive/ql/index/compact/CompactIndexHandler.java > 1558249 > trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMRUnion1.java > 1558249 > trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMapRedUtils.java > 1558249 > > trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/GenMRSkewJoinProcessor.java > 1558249 > > trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/MapJoinResolver.java > 1558249 > trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java > 1558249 > trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/MapWork.java 1558249 > trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/MapredLocalWork.java > 1558249 > trunk/ql/src/test/org/apache/hadoop/hive/ql/exec/TestPlan.java 1558249 > > trunk/ql/src/test/org/apache/hadoop/hive/ql/io/TestHiveBinarySearchRecordReader.java > 1558249 > > trunk/ql/src/test/org/apache/hadoop/hive/ql/io/TestSymlinkTextInputFormat.java > 1558249 > > Diff: https://reviews.apache.org/r/16899/diff/ > > > Testing > ------- > > No new functionality. Regression suite suffices. > > > Thanks, > > Ashutosh Chauhan > >
