Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/666#discussion_r90690234
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/WriterRecordBatch.java
---
@@ -174,10 +174,15 @@ protected void setupNewSchema() throws IOException {
schema = container.getSchema();
}
+ /** Partially written data will be removed, otherwise clean up will be
performed. */
private void closeWriter() {
if (recordWriter != null) {
try {
- recordWriter.cleanup();
+ if (processed) {
+ recordWriter.cleanup();
+ } else {
+ recordWriter.abort();
+ }
--- End diff --
Can these fail? If so, should we set the recordWriter to null in the
finally section so that the writer is marked as closed even in the failed-close
case?
Also, we mark the context as failed below. Must the caller check this
status and propagate the error up the call stack? Does this happen? Should this
method throw an exception instead to force the attention of the caller?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
