Dan Hecht has posted comments on this change.

Change subject: IMPALA-2988: Refactor HdfsTableSink::Close() so that it cannot 
fail
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4018/2/be/src/exec/hdfs-table-sink.cc
File be/src/exec/hdfs-table-sink.cc:

Line 647:     flush_status.MergeStatus(FinalizePartitionFile(state, 
cur_partition->second.first));
I still don't see why this is MergeStatus() instead of RETURN_IF_ERROR.  We 
have to require that Close() can cleanup whatever state we leave things in if 
FlushFinal() fails, so it doesn't seem like we need to continue here.  After 
all, we may have returned early at e.g. line 638 and not even gotten into this 
loop.


-- 
To view, visit http://gerrit.cloudera.org:8080/4018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2546bc68ba136b2713d744c1b920878606a2217b
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Sailesh Mukil <[email protected]>
Gerrit-HasComments: Yes

Reply via email to