> On April 2, 2014, 4:07 p.m., Xuefu Zhang wrote: > > ql/src/test/org/apache/hadoop/hive/ql/WindowsPathUtil.java, line 27 > > <https://reviews.apache.org/r/16403/diff/2/?file=544943#file544943line27> > > > > For what it's worth, it seems the if (windows) should stay with the > > caller of the method. That is, caller should do: > > > > if( windows) { > > WindowsPathUtil.convertPaths(); > > } > > > > Same for getHdfsUriString(). > > > > > >
For the first point, sure we can move the if (windows) check to TestExecDriver/TestHiveMetaStoreChecker/any other callers. For getHdfsUriString() though, this is a private method and it seems like doing this will just bloat convertPathsFromWindowsToHdfs(). Would you prefer if we just eliminated that if(windows) check in getHdfsUriString(), given that calling a method in WindowsPathUtil likely means that the caller is trying to do Windows path-specific stuff? - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16403/#review39269 ----------------------------------------------------------- On April 2, 2014, 2:11 a.m., Jason Dere wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16403/ > ----------------------------------------------------------- > > (Updated April 2, 2014, 2:11 a.m.) > > > Review request for hive and Thejas Nair. > > > Bugs: HIVE-5176 > https://issues.apache.org/jira/browse/HIVE-5176 > > > Repository: hive-git > > > Description > ------- > > We need to make certain changes across the board to allow us to read/parse > windows paths. Some are escaping changes, some are being strict about how we > read paths (through URL.encode/decode, etc) > > > Diffs > ----- > > common/src/test/org/apache/hadoop/hive/conf/TestHiveConf.java a31238b > ql/src/test/org/apache/hadoop/hive/ql/WindowsPathUtil.java PRE-CREATION > ql/src/test/org/apache/hadoop/hive/ql/exec/TestExecDriver.java 5991aae > > ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveMetaStoreChecker.java > e453f56 > > Diff: https://reviews.apache.org/r/16403/diff/ > > > Testing > ------- > > > Thanks, > > Jason Dere > >
