hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Xuefu Zhang" <xzh...@cloudera.com>
Subject Re: Review Request 16403: HIVE-5176: Wincompat : Changes for allowing various path compatibilities with Windows
Date Wed, 02 Apr 2014 19:15:41 GMT


> 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().
> >     
> >     
> >
> 
> Jason Dere wrote:
>     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?

Sorry that I didn't notice that getHdfsUriString() is private. So, if(windows) check isn't
necessary. My previous point was mainly that the class name suggests windows specific, so
it should only handle windows specific things. It's the caller's responsibility to do the
if check. The alternative is to rename the class/method to make platform neutral. Either way
is fine to me.


- Xuefu


-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message