> On July 5, 2012, 8:12 p.m., Carl Steinbach wrote: > >
Thanks Carl. Please find the comments below. I tested these changes on Unix as well as on Windows and exim unit tests are passing. > On July 5, 2012, 8:12 p.m., Carl Steinbach wrote: > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/EximUtil.java, > > line 80 > > <https://reviews.apache.org/r/5777/diff/1/?file=119652#file119652line80> > > > > This check is looking for non-absolute paths, but on Windows it will > > always evaluate to true. I think this check should be replaced with a call > > to Path.isAbsolute(). I verified it on windows. The absolute path on Windows is something like "D:\OSS\Hive\etc". It detects this path and converts it into "/D:/OSS/Hive/etc" and allows us to create a valid URI. In this case - File.IsAbsolute() returns true always. > On July 5, 2012, 8:12 p.m., Carl Steinbach wrote: > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/EximUtil.java, > > line 146 > > <https://reviews.apache.org/r/5777/diff/1/?file=119652#file119652line146> > > > > This check is looking for non-absolute paths, but on Windows it will > > always evaluate to true. I think this check should be replaced with a call > > to Path.isAbsolute(). I verified it on windows. The absolute path on Windows is something like "D:\OSS\Hive\etc". It detects this path and converts it into "/D:/OSS/Hive/etc" and allows us to create a valid URI. In this case - File.IsAbsolute() returns true always. > On July 5, 2012, 8:12 p.m., Carl Steinbach wrote: > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/EximUtil.java, > > line 147 > > <https://reviews.apache.org/r/5777/diff/1/?file=119652#file119652line147> > > > > This logic is no-op unless we're running in testmode, which makes me > > worry that this may still be broken on windows if you run it by hand. Can > > you please check on this? Thanks. I verified it on windows too. It is working. - Kanna ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5777/#review8890 ----------------------------------------------------------- On July 5, 2012, 7:50 p.m., Kanna Karanam wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5777/ > ----------------------------------------------------------- > > (Updated July 5, 2012, 7:50 p.m.) > > > Review request for hive, Carl Steinbach, Edward Capriolo, and Ashutosh > Chauhan. > > > Description > ------- > > 1) Not closing the file handle EximUtil after reading the metadata from the > file. > 2) Nit: Get the path from URI to handle the Windows paths. > > > This addresses bug HIVE-3232. > https://issues.apache.org/jira/browse/HIVE-3232 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/EximUtil.java > 1357818 > > Diff: https://reviews.apache.org/r/5777/diff/ > > > Testing > ------- > > Yes > > > Thanks, > > Kanna Karanam > >
