From 94f52f9f609cee7e72030b33097f9d0867282e87 Mon Sep 17 00:00:00 2001 From: Andrey Zagrebin Date: Thu, 20 Dec 2018 17:30:50 +0100 Subject: [PATCH 1/2] 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 | 28 +++++++++++++++++++ utilities/merge_operators.h | 1 + .../string_append/stringappend2.cc | 28 +++++++++---------- .../string_append/stringappend2.h | 24 ++++++++-------- 6 files changed, 93 insertions(+), 25 deletions(-) diff --git a/java/rocksjni/merge_operator.cc b/java/rocksjni/merge_operator.cc index 782153f57..90f4c6aba 100644 --- a/java/rocksjni/merge_operator.cc +++ b/java/rocksjni/merge_operator.cc @@ -35,6 +35,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 978cad6cc..e25e8fd73 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 73b90869c..3bb962625 100644 --- a/java/src/test/java/org/rocksdb/MergeTest.java +++ b/java/src/test/java/org/rocksdb/MergeTest.java @@ -108,6 +108,34 @@ 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()); + + // Writing empty under key + db.merge("key".getBytes(), "".getBytes()); + + final byte[] value = db.get("key".getBytes()); + final String strValue = new String(value); + + assertThat(strValue).isEqualTo("aa" + delim + "bb" + delim); + } + } + @Test public void cFOperatorOption() throws InterruptedException, RocksDBException { diff --git a/utilities/merge_operators.h b/utilities/merge_operators.h index 4c720b822..757ae0409 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 6e46d80a1..51de2f64d 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 d979f1451..95832cff1 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 ab354516ec2cafbd85a6a6369033c4ce62a7133a Mon Sep 17 00:00:00 2001 From: Andrey Zagrebin Date: Thu, 31 Jan 2019 18:24:44 +0100 Subject: [PATCH 2/2] address comments --- java/CMakeLists.txt | 2 + java/Makefile | 1 + java/rocksjni/merge_operator.cc | 30 ++++++++---- .../org/rocksdb/StringAppendOperator.java | 15 ------ ...ngAppendOperatorWithVariableDelimitor.java | 47 +++++++++++++++++++ java/src/test/java/org/rocksdb/MergeTest.java | 2 +- 6 files changed, 73 insertions(+), 24 deletions(-) create mode 100644 java/src/main/java/org/rocksdb/StringAppendOperatorWithVariableDelimitor.java diff --git a/java/CMakeLists.txt b/java/CMakeLists.txt index 96c08b231..a9747c67f 100644 --- a/java/CMakeLists.txt +++ b/java/CMakeLists.txt @@ -123,6 +123,7 @@ set(NATIVE_JAVA_CLASSES org.rocksdb.SstFileWriter org.rocksdb.Statistics org.rocksdb.StringAppendOperator + org.rocksdb.StringAppendOperatorWithVariableDelimitor org.rocksdb.TableFormatConfig org.rocksdb.Transaction org.rocksdb.TransactionDB @@ -260,6 +261,7 @@ add_jar( src/main/java/org/rocksdb/StatsLevel.java src/main/java/org/rocksdb/Status.java src/main/java/org/rocksdb/StringAppendOperator.java + src/main/java/org/rocksdb/StringAppendOperatorWithVariableDelimitor.java src/main/java/org/rocksdb/TableFormatConfig.java src/main/java/org/rocksdb/TickerType.java src/main/java/org/rocksdb/TransactionalDB.java diff --git a/java/Makefile b/java/Makefile index f58fff06e..77a2c701b 100644 --- a/java/Makefile +++ b/java/Makefile @@ -60,6 +60,7 @@ NATIVE_JAVA_CLASSES = org.rocksdb.AbstractCompactionFilter\ org.rocksdb.VectorMemTableConfig\ org.rocksdb.Snapshot\ org.rocksdb.StringAppendOperator\ + org.rocksdb.StringAppendOperatorWithVariableDelimitor\ org.rocksdb.WriteBatch\ org.rocksdb.WriteBatch.Handler\ org.rocksdb.WriteOptions\ diff --git a/java/rocksjni/merge_operator.cc b/java/rocksjni/merge_operator.cc index 90f4c6aba..ec90de1e0 100644 --- a/java/rocksjni/merge_operator.cc +++ b/java/rocksjni/merge_operator.cc @@ -13,6 +13,7 @@ #include #include "include/org_rocksdb_StringAppendOperator.h" +#include "include/org_rocksdb_StringAppendOperatorWithVariableDelimitor.h" #include "rocksdb/db.h" #include "rocksdb/memtablerep.h" #include "rocksdb/merge_operator.h" @@ -37,10 +38,23 @@ jlong Java_org_rocksdb_StringAppendOperator_newSharedStringAppendOperator( /* * Class: org_rocksdb_StringAppendOperator + * Method: disposeInternal + * Signature: (J)V + */ +void Java_org_rocksdb_StringAppendOperator_disposeInternal(JNIEnv* /*env*/, + jobject /*jobj*/, + jlong jhandle) { + auto* sptr_string_append_op = + reinterpret_cast*>(jhandle); + delete sptr_string_append_op; // delete std::shared_ptr +} + +/* + * Class: org_rocksdb_StringAppendOperatorWithVariableDelimitor * Method: newSharedStringAppendTESTOperator * Signature: ([B)J */ -jlong Java_org_rocksdb_StringAppendOperator_newSharedStringAppendTESTOperator( +jlong Java_org_rocksdb_StringAppendOperatorWithVariableDelimitor_newSharedStringAppendTESTOperator( JNIEnv* env, jclass /*jclazz*/, jbyteArray jdelim) { jboolean has_exception = JNI_FALSE; std::string delim = rocksdb::JniUtil::byteString( @@ -58,14 +72,14 @@ jlong Java_org_rocksdb_StringAppendOperator_newSharedStringAppendTESTOperator( } /* - * Class: org_rocksdb_StringAppendOperator + * Class: org_rocksdb_StringAppendOperatorWithVariableDelimitor * Method: disposeInternal * Signature: (J)V */ -void Java_org_rocksdb_StringAppendOperator_disposeInternal(JNIEnv* /*env*/, - jobject /*jobj*/, - jlong jhandle) { - auto* sptr_string_append_op = - reinterpret_cast*>(jhandle); - delete sptr_string_append_op; // delete std::shared_ptr +void Java_org_rocksdb_StringAppendOperatorWithVariableDelimitor_disposeInternal(JNIEnv* /*env*/, + jobject /*jobj*/, + jlong jhandle) { + auto* sptr_string_append_test_op = + reinterpret_cast*>(jhandle); + delete sptr_string_append_test_op; // delete std::shared_ptr } diff --git a/java/src/main/java/org/rocksdb/StringAppendOperator.java b/java/src/main/java/org/rocksdb/StringAppendOperator.java index e25e8fd73..978cad6cc 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. @@ -20,19 +18,6 @@ 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/main/java/org/rocksdb/StringAppendOperatorWithVariableDelimitor.java b/java/src/main/java/org/rocksdb/StringAppendOperatorWithVariableDelimitor.java new file mode 100644 index 000000000..0c35e103a --- /dev/null +++ b/java/src/main/java/org/rocksdb/StringAppendOperatorWithVariableDelimitor.java @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.rocksdb; + +import java.nio.charset.Charset; + +/** Merge operator that concatenates two strings with a delimiter of variable length, string or byte array. */ +public class StringAppendOperatorWithVariableDelimitor extends MergeOperator { + public StringAppendOperatorWithVariableDelimitor() { + this(','); + } + + public StringAppendOperatorWithVariableDelimitor(char delim) { + this(Character.toString(delim)); + } + + public StringAppendOperatorWithVariableDelimitor(byte[] delim) { + super(newSharedStringAppendTESTOperator(delim)); + } + + public StringAppendOperatorWithVariableDelimitor(String delim) { + this(delim.getBytes()); + } + + public StringAppendOperatorWithVariableDelimitor(String delim, Charset charset) { + this(delim.getBytes(charset)); + } + + 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 3bb962625..261785f87 100644 --- a/java/src/test/java/org/rocksdb/MergeTest.java +++ b/java/src/test/java/org/rocksdb/MergeTest.java @@ -115,7 +115,7 @@ public void stringDelimiter() throws RocksDBException { } private void stringDelimiter(String delim) throws RocksDBException { - try (final StringAppendOperator stringAppendOperator = new StringAppendOperator(delim.getBytes()); + try (final MergeOperator stringAppendOperator = new StringAppendOperatorWithVariableDelimitor(delim.getBytes()); final Options opt = new Options() .setCreateIfMissing(true) .setMergeOperator(stringAppendOperator);