Repository: incubator-impala Updated Branches: refs/heads/master ed0aa66ee -> 1a611b393
IMPALA-4747: macros should only evaluate their arguments once The way the macros were previously written, they expanded so that the 'status' expression was duplicated. This means if the expression has side-effects, they happen twice. The fix is to introduce a temporary variable. Change-Id: I1229849150d3f747e6780cc072ba063152ade1a9 Reviewed-on: http://gerrit.cloudera.org:8080/5686 Reviewed-by: Matthew Jacobs <[email protected]> Reviewed-by: Henry Robinson <[email protected]> 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/1a611b39 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/1a611b39 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/1a611b39 Branch: refs/heads/master Commit: 1a611b39311837139b21b0acd4e803477510bbfc Parents: ed0aa66 Author: Tim Armstrong <[email protected]> Authored: Wed Jan 11 15:10:39 2017 -0800 Committer: Impala Public Jenkins <[email protected]> Committed: Thu Jan 12 05:42:41 2017 +0000 ---------------------------------------------------------------------- be/src/exec/hash-table-test.cc | 2 +- be/src/runtime/buffered-tuple-stream-test.cc | 2 +- be/src/runtime/tmp-file-mgr-test.cc | 2 +- be/src/testutil/gtest-util.h | 20 +++++++++++++++++--- 4 files changed, 20 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1a611b39/be/src/exec/hash-table-test.cc ---------------------------------------------------------------------- diff --git a/be/src/exec/hash-table-test.cc b/be/src/exec/hash-table-test.cc index 9f428f1..6f0978e 100644 --- a/be/src/exec/hash-table-test.cc +++ b/be/src/exec/hash-table-test.cc @@ -71,7 +71,7 @@ class HashTableTest : public testing::Test { // internals so a simple build/probe expr is fine. Expr* expr = pool_.Add(new SlotRef(TYPE_INT, 1, true /* nullable */)); build_expr_ctxs_.push_back(pool_.Add(new ExprContext(expr))); - ASSERT_OK(Expr::Prepare(build_expr_ctxs_, NULL, desc, &tracker_)) + ASSERT_OK(Expr::Prepare(build_expr_ctxs_, NULL, desc, &tracker_)); ASSERT_OK(Expr::Open(build_expr_ctxs_, NULL)); expr = pool_.Add(new SlotRef(TYPE_INT, 1, true /* nullable */)); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1a611b39/be/src/runtime/buffered-tuple-stream-test.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/buffered-tuple-stream-test.cc b/be/src/runtime/buffered-tuple-stream-test.cc index 1d36064..4552d15 100644 --- a/be/src/runtime/buffered-tuple-stream-test.cc +++ b/be/src/runtime/buffered-tuple-stream-test.cc @@ -305,7 +305,7 @@ class SimpleTupleStreamTest : public testing::Test { batch = CreateBatch(*desc, offset, num_rows, gen_null); for (int j = 0; j < batch->num_rows(); ++j) { bool b = stream.AddRow(batch->GetRow(j), &status); - ASSERT_OK(status) + ASSERT_OK(status); if (!b) { ASSERT_TRUE(stream.using_small_buffers()); bool got_buffer; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1a611b39/be/src/runtime/tmp-file-mgr-test.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/tmp-file-mgr-test.cc b/be/src/runtime/tmp-file-mgr-test.cc index fe384b8..5e629ef 100644 --- a/be/src/runtime/tmp-file-mgr-test.cc +++ b/be/src/runtime/tmp-file-mgr-test.cc @@ -307,7 +307,7 @@ TEST_F(TmpFileMgrTest, TestAllocateNonWritable) { TmpFileMgr::FileGroup file_group(&tmp_file_mgr, io_mgr(), profile_, id); vector<TmpFileMgr::File*> allocated_files; - ASSERT_OK(CreateFiles(&file_group, &allocated_files)) + ASSERT_OK(CreateFiles(&file_group, &allocated_files)); int64_t offset; ASSERT_OK(FileAllocateSpace(allocated_files[0], 1, &offset)); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/1a611b39/be/src/testutil/gtest-util.h ---------------------------------------------------------------------- diff --git a/be/src/testutil/gtest-util.h b/be/src/testutil/gtest-util.h index 20401ff..8e0257a 100644 --- a/be/src/testutil/gtest-util.h +++ b/be/src/testutil/gtest-util.h @@ -25,9 +25,23 @@ namespace impala { // Macros for backend tests to be used when we expect the status to be OK. -#define EXPECT_OK(status) EXPECT_TRUE(status.ok()) << "Error: " << status.GetDetail(); -#define ASSERT_OK(status) ASSERT_TRUE(status.ok()) << "Error: " << status.GetDetail(); -#define EXPECT_ERROR(status, err) EXPECT_EQ(status.code(), err); +#define EXPECT_OK(status) \ + do { \ + const Status& __status = (status); \ + EXPECT_TRUE(__status.ok()) << "Error: " << __status.GetDetail(); \ + } while (0) + +#define ASSERT_OK(status) \ + do { \ + const Status& __status = (status); \ + ASSERT_TRUE(__status.ok()) << "Error: " << __status.GetDetail(); \ + } while (0) + +#define EXPECT_ERROR(status, err) \ + do { \ + const Status& __status = (status); \ + EXPECT_EQ(__status.code(), err) << "Error: " << __status.GetDetail(); \ + } while (0) // Basic main() function to be used in gtest unit tests. Does not start a JVM and does // not initialize the FE.
