From f245fd6c016493681be75a26e6e2f4ff803d09cd Mon Sep 17 00:00:00 2001 From: Andrey Zagrebin Date: Thu, 20 Dec 2018 17:30:50 +0100 Subject: [PATCH 1/3] Extend StringAppendTESTOperator to use string delimiter of variable length --- java/rocksjni/merge_operator.cc | 22 +++++++++++++++ .../org/rocksdb/StringAppendOperator.java | 15 ++++++++++ java/src/test/java/org/rocksdb/MergeTest.java | 25 +++++++++++++++++ utilities/merge_operators.h | 1 + .../string_append/stringappend2.cc | 28 +++++++++---------- .../string_append/stringappend2.h | 24 ++++++++-------- 6 files changed, 90 insertions(+), 25 deletions(-) diff --git a/java/rocksjni/merge_operator.cc b/java/rocksjni/merge_operator.cc index e06a06f7e35..ef91f561d9d 100644 --- a/java/rocksjni/merge_operator.cc +++ b/java/rocksjni/merge_operator.cc @@ -36,6 +36,28 @@ jlong Java_org_rocksdb_StringAppendOperator_newSharedStringAppendOperator( return reinterpret_cast(sptr_string_append_op); } +/* + * Class: org_rocksdb_StringAppendOperator + * Method: newSharedStringAppendTESTOperator + * Signature: ([B)J + */ +jlong Java_org_rocksdb_StringAppendOperator_newSharedStringAppendTESTOperator( + JNIEnv* env, jclass /*jclazz*/, jbyteArray jdelim) { + jboolean has_exception = JNI_FALSE; + std::string delim = rocksdb::JniUtil::byteString( + env, jdelim, + [](const char* str, const size_t len) { return std::string(str, len); }, + &has_exception); + if (has_exception == JNI_TRUE) { + // exception occurred + return 0; + } + + auto* sptr_string_append_test_op = new std::shared_ptr( + rocksdb::MergeOperators::CreateStringAppendTESTOperator(delim)); + return reinterpret_cast(sptr_string_append_test_op); +} + /* * Class: org_rocksdb_StringAppendOperator * Method: disposeInternal diff --git a/java/src/main/java/org/rocksdb/StringAppendOperator.java b/java/src/main/java/org/rocksdb/StringAppendOperator.java index 978cad6ccfe..e25e8fd73b1 100644 --- a/java/src/main/java/org/rocksdb/StringAppendOperator.java +++ b/java/src/main/java/org/rocksdb/StringAppendOperator.java @@ -5,6 +5,8 @@ package org.rocksdb; +import java.nio.charset.Charset; + /** * StringAppendOperator is a merge operator that concatenates * two strings. @@ -18,6 +20,19 @@ public StringAppendOperator(char delim) { super(newSharedStringAppendOperator(delim)); } + public StringAppendOperator(byte[] delim) { + super(newSharedStringAppendTESTOperator(delim)); + } + + public StringAppendOperator(String delim) { + this(delim.getBytes()); + } + + public StringAppendOperator(String delim, Charset charset) { + this(delim.getBytes(charset)); + } + private native static long newSharedStringAppendOperator(final char delim); + private native static long newSharedStringAppendTESTOperator(final byte[] delim); @Override protected final native void disposeInternal(final long handle); } diff --git a/java/src/test/java/org/rocksdb/MergeTest.java b/java/src/test/java/org/rocksdb/MergeTest.java index 55469847617..2daf5f64c50 100644 --- a/java/src/test/java/org/rocksdb/MergeTest.java +++ b/java/src/test/java/org/rocksdb/MergeTest.java @@ -183,6 +183,31 @@ public void operatorOption() } } + @Test + public void stringDelimiter() throws RocksDBException { + stringDelimiter("DELIM"); + stringDelimiter(""); + } + + private void stringDelimiter(String delim) throws RocksDBException { + try (final StringAppendOperator stringAppendOperator = new StringAppendOperator(delim.getBytes()); + final Options opt = new Options() + .setCreateIfMissing(true) + .setMergeOperator(stringAppendOperator); + final RocksDB db = RocksDB.open(opt, dbFolder.getRoot().getAbsolutePath())) { + // Writing aa under key + db.put("key".getBytes(), "aa".getBytes()); + + // Writing bb under key + db.merge("key".getBytes(), "bb".getBytes()); + + final byte[] value = db.get("key".getBytes()); + final String strValue = new String(value); + + assertThat(strValue).isEqualTo("aa" + delim + "bb"); + } + } + @Test public void uint64AddOperatorOption() throws InterruptedException, RocksDBException { diff --git a/utilities/merge_operators.h b/utilities/merge_operators.h index 4c720b822fe..757ae040979 100644 --- a/utilities/merge_operators.h +++ b/utilities/merge_operators.h @@ -21,6 +21,7 @@ class MergeOperators { static std::shared_ptr CreateStringAppendOperator(); static std::shared_ptr CreateStringAppendOperator(char delim_char); static std::shared_ptr CreateStringAppendTESTOperator(); + static std::shared_ptr CreateStringAppendTESTOperator(std::string delim); static std::shared_ptr CreateMaxOperator(); static std::shared_ptr CreateBytesXOROperator(); diff --git a/utilities/merge_operators/string_append/stringappend2.cc b/utilities/merge_operators/string_append/stringappend2.cc index 6e46d80a139..51de2f64d10 100644 --- a/utilities/merge_operators/string_append/stringappend2.cc +++ b/utilities/merge_operators/string_append/stringappend2.cc @@ -15,11 +15,6 @@ namespace rocksdb { -// Constructor: also specify the delimiter character. -StringAppendTESTOperator::StringAppendTESTOperator(char delim_char) - : delim_(delim_char) { -} - // Implementation for the merge operation (concatenates two strings) bool StringAppendTESTOperator::FullMergeV2( const MergeOperationInput& merge_in, @@ -37,7 +32,7 @@ bool StringAppendTESTOperator::FullMergeV2( size_t numBytes = 0; for (auto it = merge_in.operand_list.begin(); it != merge_in.operand_list.end(); ++it) { - numBytes += it->size() + 1; // Plus 1 for the delimiter + numBytes += it->size() + delim_.size(); // Plus one delimiter } // Only print the delimiter after the first entry has been printed @@ -48,20 +43,20 @@ bool StringAppendTESTOperator::FullMergeV2( merge_out->new_value.reserve(numBytes + merge_in.existing_value->size()); merge_out->new_value.append(merge_in.existing_value->data(), merge_in.existing_value->size()); - printDelim = true; + printDelim = !delim_.empty(); } else if (numBytes) { merge_out->new_value.reserve( - numBytes - 1); // Minus 1 since we have one less delimiter + numBytes - delim_.size()); // Minus 1 delimiter since we have one less delimiter } // Concatenate the sequence of strings (and add a delimiter between each) for (auto it = merge_in.operand_list.begin(); it != merge_in.operand_list.end(); ++it) { if (printDelim) { - merge_out->new_value.append(1, delim_); + merge_out->new_value.append(delim_); } merge_out->new_value.append(it->data(), it->size()); - printDelim = true; + printDelim = !delim_.empty(); } return true; @@ -87,17 +82,17 @@ bool StringAppendTESTOperator::_AssocPartialMergeMulti( // Determine and reserve correct size for *new_value. size_t size = 0; for (const auto& operand : operand_list) { - size += operand.size(); + size += operand.size() + delim_.size(); } - size += operand_list.size() - 1; // Delimiters + size -= delim_.size(); // since we have one less delimiter new_value->reserve(size); // Apply concatenation new_value->assign(operand_list.front().data(), operand_list.front().size()); - for (std::deque::const_iterator it = operand_list.begin() + 1; + for (auto it = operand_list.begin() + 1; it != operand_list.end(); ++it) { - new_value->append(1, delim_); + new_value->append(delim_); new_value->append(it->data(), it->size()); } @@ -114,4 +109,9 @@ MergeOperators::CreateStringAppendTESTOperator() { return std::make_shared(','); } +std::shared_ptr +MergeOperators::CreateStringAppendTESTOperator(std::string delim) { + return std::make_shared(delim); +} + } // namespace rocksdb diff --git a/utilities/merge_operators/string_append/stringappend2.h b/utilities/merge_operators/string_append/stringappend2.h index d979f1451b6..95832cff139 100644 --- a/utilities/merge_operators/string_append/stringappend2.h +++ b/utilities/merge_operators/string_append/stringappend2.h @@ -13,7 +13,7 @@ #pragma once #include #include - +#include #include "rocksdb/merge_operator.h" #include "rocksdb/slice.h" @@ -21,18 +21,20 @@ namespace rocksdb { class StringAppendTESTOperator : public MergeOperator { public: - // Constructor with delimiter - explicit StringAppendTESTOperator(char delim_char); + // Constructor with string delimiter + explicit StringAppendTESTOperator(std::string delim_str) : delim_(std::move(delim_str)) {}; + + // Constructor with char delimiter + explicit StringAppendTESTOperator(char delim_char) : delim_(std::string(1, delim_char)) {}; - virtual bool FullMergeV2(const MergeOperationInput& merge_in, - MergeOperationOutput* merge_out) const override; + bool FullMergeV2(const MergeOperationInput& merge_in, + MergeOperationOutput* merge_out) const override; - virtual bool PartialMergeMulti(const Slice& key, - const std::deque& operand_list, - std::string* new_value, Logger* logger) const - override; + bool PartialMergeMulti(const Slice& key, + const std::deque& operand_list, + std::string* new_value, Logger* logger) const override; - virtual const char* Name() const override; + const char* Name() const override; private: // A version of PartialMerge that actually performs "partial merging". @@ -41,7 +43,7 @@ class StringAppendTESTOperator : public MergeOperator { const std::deque& operand_list, std::string* new_value, Logger* logger) const; - char delim_; // The delimiter is inserted between elements + std::string delim_; // The delimiter is inserted between elements }; From 25e43db9b043452efdeff173d97e32d67a06933a Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Sun, 24 Nov 2019 21:51:40 +0100 Subject: [PATCH 2/3] Tidy up Java API, we don't need the convenience methods --- java/rocksjni/merge_operator.cc | 2 +- .../java/org/rocksdb/StringAppendOperator.java | 14 ++------------ 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/java/rocksjni/merge_operator.cc b/java/rocksjni/merge_operator.cc index ef91f561d9d..64490864db5 100644 --- a/java/rocksjni/merge_operator.cc +++ b/java/rocksjni/merge_operator.cc @@ -42,7 +42,7 @@ jlong Java_org_rocksdb_StringAppendOperator_newSharedStringAppendOperator( * Signature: ([B)J */ jlong Java_org_rocksdb_StringAppendOperator_newSharedStringAppendTESTOperator( - JNIEnv* env, jclass /*jclazz*/, jbyteArray jdelim) { + JNIEnv* env, jclass, jbyteArray jdelim) { jboolean has_exception = JNI_FALSE; std::string delim = rocksdb::JniUtil::byteString( env, jdelim, diff --git a/java/src/main/java/org/rocksdb/StringAppendOperator.java b/java/src/main/java/org/rocksdb/StringAppendOperator.java index e25e8fd73b1..e8f48cb996b 100644 --- a/java/src/main/java/org/rocksdb/StringAppendOperator.java +++ b/java/src/main/java/org/rocksdb/StringAppendOperator.java @@ -5,8 +5,6 @@ package org.rocksdb; -import java.nio.charset.Charset; - /** * StringAppendOperator is a merge operator that concatenates * two strings. @@ -16,22 +14,14 @@ public StringAppendOperator() { this(','); } - public StringAppendOperator(char delim) { + public StringAppendOperator(final char delim) { super(newSharedStringAppendOperator(delim)); } - public StringAppendOperator(byte[] delim) { + public StringAppendOperator(final byte[] delim) { super(newSharedStringAppendTESTOperator(delim)); } - public StringAppendOperator(String delim) { - this(delim.getBytes()); - } - - public StringAppendOperator(String delim, Charset charset) { - this(delim.getBytes(charset)); - } - private native static long newSharedStringAppendOperator(final char delim); private native static long newSharedStringAppendTESTOperator(final byte[] delim); @Override protected final native void disposeInternal(final long handle); From 32c4538e046680858325c4347018852e06f163fc Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Sun, 24 Nov 2019 21:57:54 +0100 Subject: [PATCH 3/3] Separate tests --- java/src/test/java/org/rocksdb/MergeTest.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/java/src/test/java/org/rocksdb/MergeTest.java b/java/src/test/java/org/rocksdb/MergeTest.java index 2daf5f64c50..4790bb8020e 100644 --- a/java/src/test/java/org/rocksdb/MergeTest.java +++ b/java/src/test/java/org/rocksdb/MergeTest.java @@ -15,6 +15,7 @@ import org.junit.Test; import org.junit.rules.TemporaryFolder; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.assertThat; public class MergeTest { @@ -183,26 +184,30 @@ public void operatorOption() } } + @Test + public void emptyStringDelimiter() throws RocksDBException { + stringDelimiter(""); + } + @Test public void stringDelimiter() throws RocksDBException { stringDelimiter("DELIM"); - stringDelimiter(""); } private void stringDelimiter(String delim) throws RocksDBException { - try (final StringAppendOperator stringAppendOperator = new StringAppendOperator(delim.getBytes()); + try (final StringAppendOperator stringAppendOperator = new StringAppendOperator(delim.getBytes(UTF_8)); final Options opt = new Options() .setCreateIfMissing(true) .setMergeOperator(stringAppendOperator); final RocksDB db = RocksDB.open(opt, dbFolder.getRoot().getAbsolutePath())) { // Writing aa under key - db.put("key".getBytes(), "aa".getBytes()); + db.put("key".getBytes(UTF_8), "aa".getBytes(UTF_8)); // Writing bb under key - db.merge("key".getBytes(), "bb".getBytes()); + db.merge("key".getBytes(UTF_8), "bb".getBytes(UTF_8)); - final byte[] value = db.get("key".getBytes()); - final String strValue = new String(value); + final byte[] value = db.get("key".getBytes(UTF_8)); + final String strValue = new String(value, UTF_8); assertThat(strValue).isEqualTo("aa" + delim + "bb"); }