[4/4] incubator-impala git commit: IMPALA-5787: Dropped status in KuduTableSink::Send()

[ta...@apache.org]

IMPALA-5787: Dropped status in KuduTableSink::Send()

KuduTableSink drops the Status returned from WriteKuduValue(),
potentially hiding failures.

There are only two possible ways WriteKuduValue() can fail: one of the
KuduPartialRow::Set<Type> functions fails, which can only happen if
the specified type is incorrect, which would be a bug in planning and
is therefore DCHECK-able, or if TimestampValue::UtcToUnixTimeMicros()
fails, which WriteKuduValue() DCHECKs against but also has a path to
return the error in release builds.

So similarly, this patch adds a DCHECK that the status is OK, and a
path to return the error in release builds.

It also adds WARN_UNUSED_RESULT to WriteKuduValue() to prevent similar
bugs in the future.

Testing:
- Ran the Kudu tests.

Change-Id: Ia228d98ca90de8638b0afcc242c788e225fb450f
Reviewed-on: http://gerrit.cloudera.org:8080/7667
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Reviewed-by: Matthew Jacobs <mj...@cloudera.com>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/8149dbfc
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/8149dbfc
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/8149dbfc

Branch: refs/heads/master
Commit: 8149dbfc9cc0898a7b376d754ac36b9c314ba71e
Parents: bc93b16
Author: Thomas Tauber-Marshall <tm...@cloudera.com>
Authored: Fri Aug 11 10:36:45 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Wed Aug 16 02:27:35 2017 +0000

----------------------------------------------------------------------
 be/src/exec/kudu-table-sink.cc | 8 +++++++-
 be/src/exec/kudu-util.h        | 2 +-
 2 files changed, 8 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/8149dbfc/be/src/exec/kudu-table-sink.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/kudu-table-sink.cc b/be/src/exec/kudu-table-sink.cc
index bfc34a6..541d398 100644
--- a/be/src/exec/kudu-table-sink.cc
+++ b/be/src/exec/kudu-table-sink.cc
@@ -237,7 +237,13 @@ Status KuduTableSink::Send(RuntimeState* state, RowBatch* batch) {
       }
 
       PrimitiveType type = output_expr_evals_[j]->root().type().type;
-      WriteKuduValue(col, type, value, true, write->mutable_row());
+      Status s = WriteKuduValue(col, type, value, true, write->mutable_row());
+      // This can only fail if we set a col to an incorrect type, which would be a bug in
+      // planning, so we can DCHECK.
+      DCHECK(s.ok()) << "WriteKuduValue failed for col = "
+                     << table_schema.Column(col).name() << " and type = "
+                     << output_expr_evals_[j]->root().type() << ": " << s.GetDetail();
+      RETURN_IF_ERROR(s);
     }
     if (add_row) write_ops.push_back(move(write));
   }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/8149dbfc/be/src/exec/kudu-util.h
----------------------------------------------------------------------
diff --git a/be/src/exec/kudu-util.h b/be/src/exec/kudu-util.h
index 4503ce1..eb32bcd 100644
--- a/be/src/exec/kudu-util.h
+++ b/be/src/exec/kudu-util.h
@@ -76,7 +76,7 @@ void LogKuduMessage(kudu::client::KuduLogSeverity severity, const char* filename
 /// into memory owned by the row. If false, string data must remain valid while the row
 /// is being used.
 Status WriteKuduValue(int col, PrimitiveType type, const void* value,
-    bool copy_strings, kudu::KuduPartialRow* row);
+    bool copy_strings, kudu::KuduPartialRow* row) WARN_UNUSED_RESULT;
 
 } /// namespace impala
 #endif