From 533e47709c851df30252ea02f6d683d1764a3db7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=A5=8F=E4=B9=8B=E7=AB=A0?= Date: Thu, 5 Sep 2019 17:50:45 -0700 Subject: [PATCH] Fix WriteBatchWithIndex with MergeOperator bug (#5577) Summary: ``` TEST_F(WriteBatchWithIndexTest, TestGetFromBatchAndDBMerge3) { DB* db; Options options; options.create_if_missing = true; std::string dbname = test::PerThreadDBPath("write_batch_with_index_test"); options.merge_operator = MergeOperators::CreateFromStringId("stringappend"); DestroyDB(dbname, options); Status s = DB::Open(options, dbname, &db); assert(s.ok()); ReadOptions read_options; WriteOptions write_options; FlushOptions flush_options; std::string value; WriteBatchWithIndex batch; ASSERT_OK(db->Put(write_options, "A", "1")); ASSERT_OK(db->Flush(flush_options, db->DefaultColumnFamily())); ASSERT_OK(batch.Merge("A", "2")); ASSERT_OK(batch.GetFromBatchAndDB(db, read_options, "A", &value)); ASSERT_EQ(value, "1,2"); delete db; DestroyDB(dbname, options); } ``` Fix ASSERT in batch.GetFromBatchAndDB() Pull Request resolved: https://github.com/facebook/rocksdb/pull/5577 Differential Revision: D16379847 fbshipit-source-id: b1320e24ec8e71350c525083cc0a16180a63f752 --- .../write_batch_with_index.cc | 5 ++- .../write_batch_with_index_test.cc | 32 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/utilities/write_batch_with_index/write_batch_with_index.cc b/utilities/write_batch_with_index/write_batch_with_index.cc index 272a2ab48..e6675dcc6 100644 --- a/utilities/write_batch_with_index/write_batch_with_index.cc +++ b/utilities/write_batch_with_index/write_batch_with_index.cc @@ -917,9 +917,12 @@ Status WriteBatchWithIndex::GetFromBatchAndDB( } if (merge_operator) { + std::string merge_result; s = MergeHelper::TimedFullMerge( merge_operator, key, merge_data, merge_context.GetOperands(), - pinnable_val->GetSelf(), logger, statistics, env); + &merge_result, logger, statistics, env); + pinnable_val->Reset(); + *pinnable_val->GetSelf() = std::move(merge_result); pinnable_val->PinSelf(); } else { s = Status::InvalidArgument("Options::merge_operator must be set"); diff --git a/utilities/write_batch_with_index/write_batch_with_index_test.cc b/utilities/write_batch_with_index/write_batch_with_index_test.cc index 3e0a33c35..7c7efddd1 100644 --- a/utilities/write_batch_with_index/write_batch_with_index_test.cc +++ b/utilities/write_batch_with_index/write_batch_with_index_test.cc @@ -1308,6 +1308,38 @@ TEST_F(WriteBatchWithIndexTest, TestGetFromBatchAndDBMerge2) { DestroyDB(dbname, options); } + +TEST_F(WriteBatchWithIndexTest, TestGetFromBatchAndDBMerge3) { + DB* db; + Options options; + + options.create_if_missing = true; + std::string dbname = test::PerThreadDBPath("write_batch_with_index_test"); + + options.merge_operator = MergeOperators::CreateFromStringId("stringappend"); + + DestroyDB(dbname, options); + Status s = DB::Open(options, dbname, &db); + assert(s.ok()); + + ReadOptions read_options; + WriteOptions write_options; + FlushOptions flush_options; + std::string value; + + WriteBatchWithIndex batch; + + ASSERT_OK(db->Put(write_options, "A", "1")); + ASSERT_OK(db->Flush(flush_options, db->DefaultColumnFamily())); + ASSERT_OK(batch.Merge("A", "2")); + + ASSERT_OK(batch.GetFromBatchAndDB(db, read_options, "A", &value)); + ASSERT_EQ(value, "1,2"); + + delete db; + DestroyDB(dbname, options); +} + void AssertKey(std::string key, WBWIIterator* iter) { ASSERT_TRUE(iter->Valid()); ASSERT_EQ(key, iter->Entry().key.ToString());