----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25341/#review52339 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java <https://reviews.apache.org/r/25341/#comment91103> I don't see needToRename being used elsewhere. So can you replace this chunk with if (fs.exists(outPaths[idx]) && !fs.rename(outPaths[idx], finalPaths[idx]) {..} ? ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java <https://reviews.apache.org/r/25341/#comment91107> Can you add a comment what is happening here? Are you just stripping off the _attemptId from taskId_attemptId? If so can you use Utilities.getTaskIdFromFilename() instead? ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java <https://reviews.apache.org/r/25341/#comment91222> You are correct. My Bad. I should have removed this assertion. This assertion is not valid anymore. - Prasanth_J On Sept. 4, 2014, 3:49 p.m., Alan Gates wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25341/ > ----------------------------------------------------------- > > (Updated Sept. 4, 2014, 3:49 p.m.) > > > Review request for hive and Prasanth_J. > > > Bugs: HIVE-7078 > https://issues.apache.org/jira/browse/HIVE-7078 > > > Repository: hive-git > > > Description > ------- > > Changes FileSinkOperator to use RecordUpdater in cases where an ACID write is > being done. > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java d4e61d8 > ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java f584926 > ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java c3a83d4 > ql/src/java/org/apache/hadoop/hive/ql/plan/FileSinkDesc.java 301dde5 > ql/src/test/org/apache/hadoop/hive/ql/exec/TestFileSinkOperator.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/25341/diff/ > > > Testing > ------- > > Added a new unit test TestFileSinkOperator that tests writing of standard > (non-ACID) data via RecordWriter and acid data via RecordUpdater, in both > partitioned and non-partitioned cases. > > > Thanks, > > Alan Gates > >