From e697da0b18897ab0e68f1fced8ea029cf080b5af Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Fri, 20 Dec 2019 14:25:52 -0800 Subject: [PATCH] RocksDB#keyMayExist should not assume database values are unicode strings (#6186) Summary: Closes https://github.com/facebook/rocksdb/issues/6183 Pull Request resolved: https://github.com/facebook/rocksdb/pull/6186 Differential Revision: D19201281 Pulled By: pdillinger fbshipit-source-id: 1c96b4ea09e826f91e44b0009eba3de0991d9053 --- HISTORY.md | 1 + java/CMakeLists.txt | 1 + java/rocksjni/rocksjni.cc | 209 ++++++++------ java/src/main/java/org/rocksdb/Holder.java | 46 +++ java/src/main/java/org/rocksdb/RocksDB.java | 271 +++++++++++------- .../rocksdb/CompactionFilterFactoryTest.java | 3 +- .../java/org/rocksdb/KeyMayExistTest.java | 145 +++++++--- 7 files changed, 446 insertions(+), 230 deletions(-) create mode 100644 java/src/main/java/org/rocksdb/Holder.java diff --git a/HISTORY.md b/HISTORY.md index 24f4aea04..f5d77802e 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -3,6 +3,7 @@ ### Public API Change * Added a rocksdb::FileSystem class in include/rocksdb/file_system.h to encapsulate file creation/read/write operations, and an option DBOptions::file_system to allow a user to pass in an instance of rocksdb::FileSystem. If its a non-null value, this will take precendence over DBOptions::env for file operations. A new API rocksdb::FileSystem::Default() returns a platform default object. The DBOptions::env option and Env::Default() API will continue to be used for threading and other OS related functions, and where DBOptions::file_system is not specified, for file operations. For storage developers who are accustomed to rocksdb::Env, the interface in rocksdb::FileSystem is new and will probably undergo some changes as more storage systems are ported to it from rocksdb::Env. As of now, no env other than Posix has been ported to the new interface. * A new rocksdb::NewSstFileManager() API that allows the caller to pass in separate Env and FileSystem objects. +* Changed Java API for RocksDB.keyMayExist functions to use Holder instead of StringBuilder, so that retrieved values need not decode to Strings. ### Bug Fixes * Fix a bug that can cause unnecessary bg thread to be scheduled(#6104). diff --git a/java/CMakeLists.txt b/java/CMakeLists.txt index 1951bfa48..0f6c231b2 100644 --- a/java/CMakeLists.txt +++ b/java/CMakeLists.txt @@ -144,6 +144,7 @@ set(JAVA_MAIN_CLASSES src/main/java/org/rocksdb/HdfsEnv.java src/main/java/org/rocksdb/HistogramData.java src/main/java/org/rocksdb/HistogramType.java + src/main/java/org/rocksdb/Holder.java src/main/java/org/rocksdb/IndexType.java src/main/java/org/rocksdb/InfoLogLevel.java src/main/java/org/rocksdb/IngestExternalFileOptions.java diff --git a/java/rocksjni/rocksjni.cc b/java/rocksjni/rocksjni.cc index 58c06fae8..7f6d5ebfe 100644 --- a/java/rocksjni/rocksjni.cc +++ b/java/rocksjni/rocksjni.cc @@ -1588,127 +1588,156 @@ jobjectArray Java_org_rocksdb_RocksDB_multiGet__JJ_3_3B_3I_3I_3J( ////////////////////////////////////////////////////////////////////////////// // rocksdb::DB::KeyMayExist -jboolean key_may_exist_helper(JNIEnv* env, rocksdb::DB* db, - const rocksdb::ReadOptions& read_opt, - rocksdb::ColumnFamilyHandle* cf_handle, - jbyteArray jkey, jint jkey_off, jint jkey_len, - jobject jstring_builder, bool* has_exception) { +bool key_may_exist_helper(JNIEnv* env, jlong jdb_handle, jlong jcf_handle, + jlong jread_opts_handle, + jbyteArray jkey, jint jkey_offset, jint jkey_len, + bool* has_exception, std::string* value, bool* value_found) { + auto* db = reinterpret_cast(jdb_handle); + rocksdb::ColumnFamilyHandle* cf_handle; + if (jcf_handle == 0) { + cf_handle = db->DefaultColumnFamily(); + } else { + cf_handle = reinterpret_cast(jcf_handle); + } + rocksdb::ReadOptions read_opts = + jread_opts_handle == 0 ? rocksdb::ReadOptions() + : *(reinterpret_cast(jread_opts_handle)); + jbyte* key = new jbyte[jkey_len]; - env->GetByteArrayRegion(jkey, jkey_off, jkey_len, key); + env->GetByteArrayRegion(jkey, jkey_offset, jkey_len, key); if (env->ExceptionCheck()) { // exception thrown: ArrayIndexOutOfBoundsException delete[] key; *has_exception = true; return false; } - rocksdb::Slice key_slice(reinterpret_cast(key), jkey_len); - std::string value; - bool value_found = false; - bool keyMayExist; - if (cf_handle != nullptr) { - keyMayExist = - db->KeyMayExist(read_opt, cf_handle, key_slice, &value, &value_found); - } else { - keyMayExist = db->KeyMayExist(read_opt, key_slice, &value, &value_found); - } + const bool exists = db->KeyMayExist( + read_opts, cf_handle, key_slice, value, value_found); // cleanup delete[] key; - // extract the value - if (value_found && !value.empty()) { - jobject jresult_string_builder = - rocksdb::StringBuilderJni::append(env, jstring_builder, value.c_str()); - if (jresult_string_builder == nullptr) { - *has_exception = true; - return false; - } - } - - *has_exception = false; - return static_cast(keyMayExist); + return exists; } + /* * Class: org_rocksdb_RocksDB * Method: keyMayExist - * Signature: (J[BIILjava/lang/StringBuilder;)Z + * Signature: (JJJ[BII)Z */ -jboolean Java_org_rocksdb_RocksDB_keyMayExist__J_3BIILjava_lang_StringBuilder_2( - JNIEnv* env, jobject, jlong jdb_handle, - jbyteArray jkey, jint jkey_off, jint jkey_len, jobject jstring_builder) { - auto* db = reinterpret_cast(jdb_handle); +jboolean Java_org_rocksdb_RocksDB_keyMayExist( + JNIEnv* env, jobject, jlong jdb_handle, jlong jcf_handle, + jlong jread_opts_handle, + jbyteArray jkey, jint jkey_offset, jint jkey_len) { + bool has_exception = false; - return key_may_exist_helper(env, db, rocksdb::ReadOptions(), nullptr, jkey, - jkey_off, jkey_len, jstring_builder, &has_exception); -} + std::string value; + bool value_found = false; -/* - * Class: org_rocksdb_RocksDB - * Method: keyMayExist - * Signature: (J[BIIJLjava/lang/StringBuilder;)Z - */ -jboolean -Java_org_rocksdb_RocksDB_keyMayExist__J_3BIIJLjava_lang_StringBuilder_2( - JNIEnv* env, jobject, jlong jdb_handle, - jbyteArray jkey, jint jkey_off, jint jkey_len, - jlong jcf_handle, jobject jstring_builder) { - auto* db = reinterpret_cast(jdb_handle); - auto* cf_handle = reinterpret_cast(jcf_handle); - if (cf_handle != nullptr) { - bool has_exception = false; - return key_may_exist_helper(env, db, rocksdb::ReadOptions(), cf_handle, - jkey, jkey_off, jkey_len, jstring_builder, - &has_exception); - } else { - rocksdb::RocksDBExceptionJni::ThrowNew( - env, rocksdb::Status::InvalidArgument("Invalid ColumnFamilyHandle.")); - return true; + const bool exists = key_may_exist_helper( + env, jdb_handle, jcf_handle, jread_opts_handle, + jkey, jkey_offset, jkey_len, + &has_exception, &value, &value_found); + + if (has_exception) { + // java exception already raised + return false; } + + return static_cast(exists); } /* * Class: org_rocksdb_RocksDB - * Method: keyMayExist - * Signature: (JJ[BIILjava/lang/StringBuilder;)Z + * Method: keyMayExistFoundValue + * Signature: (JJJ[BII)[[B */ -jboolean -Java_org_rocksdb_RocksDB_keyMayExist__JJ_3BIILjava_lang_StringBuilder_2( - JNIEnv* env, jobject, jlong jdb_handle, jlong jread_options_handle, - jbyteArray jkey, jint jkey_off, jint jkey_len, jobject jstring_builder) { - auto* db = reinterpret_cast(jdb_handle); - auto& read_options = - *reinterpret_cast(jread_options_handle); +jobjectArray Java_org_rocksdb_RocksDB_keyMayExistFoundValue( + JNIEnv* env, jobject, jlong jdb_handle, jlong jcf_handle, + jlong jread_opts_handle, + jbyteArray jkey, jint jkey_offset, jint jkey_len) { + bool has_exception = false; - return key_may_exist_helper(env, db, read_options, nullptr, jkey, jkey_off, - jkey_len, jstring_builder, &has_exception); -} + std::string value; + bool value_found = false; -/* - * Class: org_rocksdb_RocksDB - * Method: keyMayExist - * Signature: (JJ[BIIJLjava/lang/StringBuilder;)Z - */ -jboolean -Java_org_rocksdb_RocksDB_keyMayExist__JJ_3BIIJLjava_lang_StringBuilder_2( - JNIEnv* env, jobject, jlong jdb_handle, jlong jread_options_handle, - jbyteArray jkey, jint jkey_off, jint jkey_len, jlong jcf_handle, - jobject jstring_builder) { - auto* db = reinterpret_cast(jdb_handle); - auto& read_options = - *reinterpret_cast(jread_options_handle); - auto* cf_handle = reinterpret_cast(jcf_handle); - if (cf_handle != nullptr) { - bool has_exception = false; - return key_may_exist_helper(env, db, read_options, cf_handle, jkey, - jkey_off, jkey_len, jstring_builder, &has_exception); + const bool exists = key_may_exist_helper( + env, jdb_handle, jcf_handle, jread_opts_handle, + jkey, jkey_offset, jkey_len, + &has_exception, &value, &value_found); + + if (has_exception) { + // java exception already raised + return nullptr; + } + + jbyte result_flags[1]; + if (!exists) { + result_flags[0] = 0; + } else if (!value_found) { + result_flags[0] = 1; } else { - rocksdb::RocksDBExceptionJni::ThrowNew( - env, rocksdb::Status::InvalidArgument("Invalid ColumnFamilyHandle.")); - return true; + // found + result_flags[0] = 2; + } + + jobjectArray jresults = rocksdb::ByteJni::new2dByteArray(env, 2); + if (jresults == nullptr) { + // exception occurred + return nullptr; } + + // prepare the result flag + jbyteArray jresult_flags = env->NewByteArray(1); + if (jresult_flags == nullptr) { + // exception thrown: OutOfMemoryError + return nullptr; + } + env->SetByteArrayRegion(jresult_flags, 0, 1, result_flags); + if (env->ExceptionCheck()) { + // exception thrown: ArrayIndexOutOfBoundsException + env->DeleteLocalRef(jresult_flags); + return nullptr; + } + + env->SetObjectArrayElement(jresults, 0, jresult_flags); + if (env->ExceptionCheck()) { + // exception thrown: ArrayIndexOutOfBoundsException + env->DeleteLocalRef(jresult_flags); + return nullptr; + } + + env->DeleteLocalRef(jresult_flags); + + if (result_flags[0] == 2) { + // set the value + const jsize jvalue_len = static_cast(value.size()); + jbyteArray jresult_value = env->NewByteArray(jvalue_len); + if (jresult_value == nullptr) { + // exception thrown: OutOfMemoryError + return nullptr; + } + env->SetByteArrayRegion(jresult_value, 0, jvalue_len, + const_cast(reinterpret_cast(value.data()))); + if (env->ExceptionCheck()) { + // exception thrown: ArrayIndexOutOfBoundsException + env->DeleteLocalRef(jresult_value); + return nullptr; + } + env->SetObjectArrayElement(jresults, 1, jresult_value); + if (env->ExceptionCheck()) { + // exception thrown: ArrayIndexOutOfBoundsException + env->DeleteLocalRef(jresult_value); + return nullptr; + } + + env->DeleteLocalRef(jresult_value); + } + + return jresults; } /* diff --git a/java/src/main/java/org/rocksdb/Holder.java b/java/src/main/java/org/rocksdb/Holder.java new file mode 100644 index 000000000..716a0bda0 --- /dev/null +++ b/java/src/main/java/org/rocksdb/Holder.java @@ -0,0 +1,46 @@ +// Copyright (c) 2016, Facebook, Inc. All rights reserved. +// This source code is licensed under both the GPLv2 (found in the +// COPYING file in the root directory) and Apache 2.0 License +// (found in the LICENSE.Apache file in the root directory). + +package org.rocksdb; + +/** + * Simple instance reference wrapper. + */ +public class Holder { + private /* @Nullable */ T value; + + /** + * Constructs a new Holder with null instance. + */ + public Holder() { + } + + /** + * Constructs a new Holder. + * + * @param value the instance or null + */ + public Holder(/* @Nullable */ final T value) { + this.value = value; + } + + /** + * Get the instance reference. + * + * @return value the instance reference or null + */ + public /* @Nullable */ T getValue() { + return value; + } + + /** + * Set the instance reference. + * + * @param value the instance reference or null + */ + public void setValue(/* @Nullable */ final T value) { + this.value = value; + } +} diff --git a/java/src/main/java/org/rocksdb/RocksDB.java b/java/src/main/java/org/rocksdb/RocksDB.java index 25412640b..ba16c207d 100644 --- a/java/src/main/java/org/rocksdb/RocksDB.java +++ b/java/src/main/java/org/rocksdb/RocksDB.java @@ -2189,68 +2189,94 @@ public class RocksDB extends RocksObject { /** * If the key definitely does not exist in the database, then this method - * returns false, else true. + * returns null, else it returns an instance of KeyMayExistResult * - * This check is potentially lighter-weight than invoking DB::Get(). One way - * to make this lighter weight is to avoid doing any IOs. + * If the caller wants to obtain value when the key + * is found in memory, then {@code valueHolder} must be set. + * + * This check is potentially lighter-weight than invoking + * {@link #get(byte[])}. One way to make this lighter weight is to avoid + * doing any IOs. * * @param key byte array of a key to search for - * @param value StringBuilder instance which is a out parameter if a value is - * found in block-cache. - * @return boolean value indicating if key does not exist or might exist. + * @param valueHolder non-null to retrieve the value if it is found, or null + * if the value is not needed. If non-null, upon return of the function, + * the {@code value} will be set if it could be retrieved. + * + * @return false if the key definitely does not exist in the database, + * otherwise true. */ - public boolean keyMayExist(final byte[] key, final StringBuilder value) { - return keyMayExist(nativeHandle_, key, 0, key.length, value); + public boolean keyMayExist(final byte[] key, + /* @Nullable */ final Holder valueHolder) { + return keyMayExist(key, 0, key.length, valueHolder); } /** * If the key definitely does not exist in the database, then this method - * returns false, else true. + * returns null, else it returns an instance of KeyMayExistResult * - * This check is potentially lighter-weight than invoking DB::Get(). One way - * to make this lighter weight is to avoid doing any IOs. + * If the caller wants to obtain value when the key + * is found in memory, then {@code valueHolder} must be set. + * + * This check is potentially lighter-weight than invoking + * {@link #get(byte[], int, int)}. One way to make this lighter weight is to + * avoid doing any IOs. * * @param key byte array of a key to search for * @param offset the offset of the "key" array to be used, must be * non-negative and no larger than "key".length * @param len the length of the "key" array to be used, must be non-negative * and no larger than "key".length - * @param value StringBuilder instance which is a out parameter if a value is - * found in block-cache. + * @param valueHolder non-null to retrieve the value if it is found, or null + * if the value is not needed. If non-null, upon return of the function, + * the {@code value} will be set if it could be retrieved. * - * @return boolean value indicating if key does not exist or might exist. + * @return false if the key definitely does not exist in the database, + * otherwise true. */ - public boolean keyMayExist(final byte[] key, final int offset, final int len, - final StringBuilder value) { - checkBounds(offset, len, key.length); - return keyMayExist(nativeHandle_, key, offset, len, value); + public boolean keyMayExist(final byte[] key, + final int offset, final int len, + /* @Nullable */ final Holder valueHolder) { + return keyMayExist((ColumnFamilyHandle)null, key, offset, len, valueHolder); } /** * If the key definitely does not exist in the database, then this method - * returns false, else true. + * returns null, else it returns an instance of KeyMayExistResult * - * This check is potentially lighter-weight than invoking DB::Get(). One way - * to make this lighter weight is to avoid doing any IOs. + * If the caller wants to obtain value when the key + * is found in memory, then {@code valueHolder} must be set. + * + * This check is potentially lighter-weight than invoking + * {@link #get(ColumnFamilyHandle,byte[])}. One way to make this lighter + * weight is to avoid doing any IOs. * * @param columnFamilyHandle {@link ColumnFamilyHandle} instance * @param key byte array of a key to search for - * @param value StringBuilder instance which is a out parameter if a value is - * found in block-cache. - * @return boolean value indicating if key does not exist or might exist. + * @param valueHolder non-null to retrieve the value if it is found, or null + * if the value is not needed. If non-null, upon return of the function, + * the {@code value} will be set if it could be retrieved. + * + * @return false if the key definitely does not exist in the database, + * otherwise true. */ - public boolean keyMayExist(final ColumnFamilyHandle columnFamilyHandle, - final byte[] key, final StringBuilder value) { - return keyMayExist(nativeHandle_, key, 0, key.length, - columnFamilyHandle.nativeHandle_, value); + public boolean keyMayExist( + final ColumnFamilyHandle columnFamilyHandle, final byte[] key, + /* @Nullable */ final Holder valueHolder) { + return keyMayExist(columnFamilyHandle, key, 0, key.length, + valueHolder); } /** * If the key definitely does not exist in the database, then this method - * returns false, else true. + * returns null, else it returns an instance of KeyMayExistResult + * + * If the caller wants to obtain value when the key + * is found in memory, then {@code valueHolder} must be set. * - * This check is potentially lighter-weight than invoking DB::Get(). One way - * to make this lighter weight is to avoid doing any IOs. + * This check is potentially lighter-weight than invoking + * {@link #get(ColumnFamilyHandle, byte[], int, int)}. One way to make this + * lighter weight is to avoid doing any IOs. * * @param columnFamilyHandle {@link ColumnFamilyHandle} instance * @param key byte array of a key to search for @@ -2258,42 +2284,58 @@ public class RocksDB extends RocksObject { * non-negative and no larger than "key".length * @param len the length of the "key" array to be used, must be non-negative * and no larger than "key".length - * @param value StringBuilder instance which is a out parameter if a value is - * found in block-cache. - * @return boolean value indicating if key does not exist or might exist. + * @param valueHolder non-null to retrieve the value if it is found, or null + * if the value is not needed. If non-null, upon return of the function, + * the {@code value} will be set if it could be retrieved. + * + * @return false if the key definitely does not exist in the database, + * otherwise true. */ - public boolean keyMayExist(final ColumnFamilyHandle columnFamilyHandle, - final byte[] key, int offset, int len, final StringBuilder value) { - checkBounds(offset, len, key.length); - return keyMayExist(nativeHandle_, key, offset, len, - columnFamilyHandle.nativeHandle_, value); + public boolean keyMayExist( + final ColumnFamilyHandle columnFamilyHandle, + final byte[] key, int offset, int len, + /* @Nullable */ final Holder valueHolder) { + return keyMayExist(columnFamilyHandle, null, key, offset, len, + valueHolder); } /** * If the key definitely does not exist in the database, then this method - * returns false, else true. + * returns null, else it returns an instance of KeyMayExistResult * - * This check is potentially lighter-weight than invoking DB::Get(). One way - * to make this lighter weight is to avoid doing any IOs. + * If the caller wants to obtain value when the key + * is found in memory, then {@code valueHolder} must be set. + * + * This check is potentially lighter-weight than invoking + * {@link #get(ReadOptions, byte[])}. One way to make this + * lighter weight is to avoid doing any IOs. * * @param readOptions {@link ReadOptions} instance * @param key byte array of a key to search for - * @param value StringBuilder instance which is a out parameter if a value is - * found in block-cache. - * @return boolean value indicating if key does not exist or might exist. + * @param valueHolder non-null to retrieve the value if it is found, or null + * if the value is not needed. If non-null, upon return of the function, + * the {@code value} will be set if it could be retrieved. + * + * @return false if the key definitely does not exist in the database, + * otherwise true. */ - public boolean keyMayExist(final ReadOptions readOptions, - final byte[] key, final StringBuilder value) { - return keyMayExist(nativeHandle_, readOptions.nativeHandle_, - key, 0, key.length, value); + public boolean keyMayExist( + final ReadOptions readOptions, final byte[] key, + /* @Nullable */ final Holder valueHolder) { + return keyMayExist(readOptions, key, 0, key.length, + valueHolder); } /** * If the key definitely does not exist in the database, then this method - * returns false, else true. + * returns null, else it returns an instance of KeyMayExistResult * - * This check is potentially lighter-weight than invoking DB::Get(). One way - * to make this lighter weight is to avoid doing any IOs. + * If the caller wants to obtain value when the key + * is found in memory, then {@code valueHolder} must be set. + * + * This check is potentially lighter-weight than invoking + * {@link #get(ReadOptions, byte[], int, int)}. One way to make this + * lighter weight is to avoid doing any IOs. * * @param readOptions {@link ReadOptions} instance * @param key byte array of a key to search for @@ -2301,65 +2343,103 @@ public class RocksDB extends RocksObject { * non-negative and no larger than "key".length * @param len the length of the "key" array to be used, must be non-negative * and no larger than "key".length - * @param value StringBuilder instance which is a out parameter if a value is - * found in block-cache. - * @return boolean value indicating if key does not exist or might exist. + * @param valueHolder non-null to retrieve the value if it is found, or null + * if the value is not needed. If non-null, upon return of the function, + * the {@code value} will be set if it could be retrieved. + * + * @return false if the key definitely does not exist in the database, + * otherwise true. */ - public boolean keyMayExist(final ReadOptions readOptions, + public boolean keyMayExist( + final ReadOptions readOptions, final byte[] key, final int offset, final int len, - final StringBuilder value) { - checkBounds(offset, len, key.length); - return keyMayExist(nativeHandle_, readOptions.nativeHandle_, - key, offset, len, value); + /* @Nullable */ final Holder valueHolder) { + return keyMayExist(null, readOptions, + key, offset, len, valueHolder); } /** * If the key definitely does not exist in the database, then this method - * returns false, else true. + * returns null, else it returns an instance of KeyMayExistResult * - * This check is potentially lighter-weight than invoking DB::Get(). One way - * to make this lighter weight is to avoid doing any IOs. + * If the caller wants to obtain value when the key + * is found in memory, then {@code valueHolder} must be set. + * + * This check is potentially lighter-weight than invoking + * {@link #get(ColumnFamilyHandle, ReadOptions, byte[])}. One way to make this + * lighter weight is to avoid doing any IOs. * - * @param readOptions {@link ReadOptions} instance * @param columnFamilyHandle {@link ColumnFamilyHandle} instance + * @param readOptions {@link ReadOptions} instance * @param key byte array of a key to search for - * @param value StringBuilder instance which is a out parameter if a value is - * found in block-cache. - * @return boolean value indicating if key does not exist or might exist. + * @param valueHolder non-null to retrieve the value if it is found, or null + * if the value is not needed. If non-null, upon return of the function, + * the {@code value} will be set if it could be retrieved. + * + * @return false if the key definitely does not exist in the database, + * otherwise true. */ - public boolean keyMayExist(final ReadOptions readOptions, - final ColumnFamilyHandle columnFamilyHandle, final byte[] key, - final StringBuilder value) { - return keyMayExist(nativeHandle_, readOptions.nativeHandle_, - key, 0, key.length, columnFamilyHandle.nativeHandle_, - value); + public boolean keyMayExist( + final ColumnFamilyHandle columnFamilyHandle, + final ReadOptions readOptions, final byte[] key, + /* @Nullable */ final Holder valueHolder) { + return keyMayExist(columnFamilyHandle, readOptions, + key, 0, key.length, valueHolder); } /** * If the key definitely does not exist in the database, then this method - * returns false, else true. + * returns null, else it returns an instance of KeyMayExistResult * - * This check is potentially lighter-weight than invoking DB::Get(). One way - * to make this lighter weight is to avoid doing any IOs. + * If the caller wants to obtain value when the key + * is found in memory, then {@code valueHolder} must be set. + * + * This check is potentially lighter-weight than invoking + * {@link #get(ColumnFamilyHandle, ReadOptions, byte[], int, int)}. + * One way to make this lighter weight is to avoid doing any IOs. * - * @param readOptions {@link ReadOptions} instance * @param columnFamilyHandle {@link ColumnFamilyHandle} instance + * @param readOptions {@link ReadOptions} instance * @param key byte array of a key to search for * @param offset the offset of the "key" array to be used, must be * non-negative and no larger than "key".length * @param len the length of the "key" array to be used, must be non-negative * and no larger than "key".length - * @param value StringBuilder instance which is a out parameter if a value is - * found in block-cache. - * @return boolean value indicating if key does not exist or might exist. + * @param valueHolder non-null to retrieve the value if it is found, or null + * if the value is not needed. If non-null, upon return of the function, + * the {@code value} will be set if it could be retrieved. + * + * @return false if the key definitely does not exist in the database, + * otherwise true. */ - public boolean keyMayExist(final ReadOptions readOptions, - final ColumnFamilyHandle columnFamilyHandle, final byte[] key, - final int offset, final int len, final StringBuilder value) { + public boolean keyMayExist( + final ColumnFamilyHandle columnFamilyHandle, + final ReadOptions readOptions, + final byte[] key, final int offset, final int len, + /* @Nullable */ final Holder valueHolder) { checkBounds(offset, len, key.length); - return keyMayExist(nativeHandle_, readOptions.nativeHandle_, - key, offset, len, columnFamilyHandle.nativeHandle_, - value); + if (valueHolder == null) { + return keyMayExist(nativeHandle_, + columnFamilyHandle == null ? 0 : columnFamilyHandle.nativeHandle_, + readOptions == null ? 0 : readOptions.nativeHandle_, + key, offset, len); + } else { + final byte[][] result = keyMayExistFoundValue( + nativeHandle_, + columnFamilyHandle == null ? 0 : columnFamilyHandle.nativeHandle_, + readOptions == null ? 0 : readOptions.nativeHandle_, + key, offset, len); + if (result[0][0] == 0x0) { + valueHolder.setValue(null); + return false; + } else if (result[0][0] == 0x1) { + valueHolder.setValue(null); + return true; + } else { + valueHolder.setValue(result[1]); + return true; + } + } } /** @@ -4158,19 +4238,12 @@ public class RocksDB extends RocksObject { private native byte[][] multiGet(final long dbHandle, final long rOptHandle, final byte[][] keys, final int[] keyOffsets, final int[] keyLengths, final long[] columnFamilyHandles); - private native boolean keyMayExist(final long handle, final byte[] key, - final int keyOffset, final int keyLength, - final StringBuilder stringBuilder); - private native boolean keyMayExist(final long handle, final byte[] key, - final int keyOffset, final int keyLength, final long cfHandle, - final StringBuilder stringBuilder); - private native boolean keyMayExist(final long handle, - final long optionsHandle, final byte[] key, final int keyOffset, - final int keyLength, final StringBuilder stringBuilder); - private native boolean keyMayExist(final long handle, - final long optionsHandle, final byte[] key, final int keyOffset, - final int keyLength, final long cfHandle, - final StringBuilder stringBuilder); + private native boolean keyMayExist( + final long handle, final long cfHandle, final long readOptHandle, + final byte[] key, final int keyOffset, final int keyLength); + private native byte[][] keyMayExistFoundValue( + final long handle, final long cfHandle, final long readOptHandle, + final byte[] key, final int keyOffset, final int keyLength); private native long iterator(final long handle); private native long iterator(final long handle, final long readOptHandle); private native long iteratorCF(final long handle, final long cfHandle); diff --git a/java/src/test/java/org/rocksdb/CompactionFilterFactoryTest.java b/java/src/test/java/org/rocksdb/CompactionFilterFactoryTest.java index efa29b1d9..e05f1eef3 100644 --- a/java/src/test/java/org/rocksdb/CompactionFilterFactoryTest.java +++ b/java/src/test/java/org/rocksdb/CompactionFilterFactoryTest.java @@ -55,7 +55,8 @@ public class CompactionFilterFactoryTest { rocksDb.compactRange(cfHandles.get(1)); assertThat(rocksDb.get(cfHandles.get(1), key1)).isEqualTo(value1); - assertThat(rocksDb.keyMayExist(cfHandles.get(1), key2, new StringBuilder())).isFalse(); + final boolean exists = rocksDb.keyMayExist(cfHandles.get(1), key2, null); + assertThat(exists).isFalse(); } finally { for (final ColumnFamilyHandle cfHandle : cfHandles) { cfHandle.close(); diff --git a/java/src/test/java/org/rocksdb/KeyMayExistTest.java b/java/src/test/java/org/rocksdb/KeyMayExistTest.java index a1335ca62..45b06be35 100644 --- a/java/src/test/java/org/rocksdb/KeyMayExistTest.java +++ b/java/src/test/java/org/rocksdb/KeyMayExistTest.java @@ -13,6 +13,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.assertThat; public class KeyMayExistTest { @@ -41,81 +42,118 @@ public class KeyMayExistTest { try { assertThat(columnFamilyHandleList.size()). isEqualTo(2); - db.put("key".getBytes(), "value".getBytes()); + db.put("key".getBytes(UTF_8), "value".getBytes(UTF_8)); // Test without column family - StringBuilder retValue = new StringBuilder(); - boolean exists = db.keyMayExist("key".getBytes(), retValue); + final Holder holder = new Holder<>(); + boolean exists = db.keyMayExist("key".getBytes(UTF_8), holder); + assertThat(exists).isTrue(); + assertThat(holder.getValue()).isNotNull(); + assertThat(new String(holder.getValue(), UTF_8)).isEqualTo("value"); + + exists = db.keyMayExist("key".getBytes(UTF_8), null); assertThat(exists).isTrue(); - assertThat(retValue.toString()).isEqualTo("value"); // Slice key - StringBuilder builder = new StringBuilder("prefix"); - int offset = builder.toString().length(); + final StringBuilder builder = new StringBuilder("prefix"); + final int offset = builder.toString().length(); builder.append("slice key 0"); - int len = builder.toString().length() - offset; + final int len = builder.toString().length() - offset; builder.append("suffix"); - byte[] sliceKey = builder.toString().getBytes(); - byte[] sliceValue = "slice value 0".getBytes(); + final byte[] sliceKey = builder.toString().getBytes(UTF_8); + final byte[] sliceValue = "slice value 0".getBytes(UTF_8); db.put(sliceKey, offset, len, sliceValue, 0, sliceValue.length); - retValue = new StringBuilder(); - exists = db.keyMayExist(sliceKey, offset, len, retValue); + exists = db.keyMayExist(sliceKey, offset, len, holder); + assertThat(exists).isTrue(); + assertThat(holder.getValue()).isNotNull(); + assertThat(holder.getValue()).isEqualTo(sliceValue); + + exists = db.keyMayExist(sliceKey, offset, len, null); assertThat(exists).isTrue(); - assertThat(retValue.toString().getBytes()).isEqualTo(sliceValue); // Test without column family but with readOptions try (final ReadOptions readOptions = new ReadOptions()) { - retValue = new StringBuilder(); - exists = db.keyMayExist(readOptions, "key".getBytes(), retValue); + exists = db.keyMayExist(readOptions, "key".getBytes(UTF_8), holder); + assertThat(exists).isTrue(); + assertThat(holder.getValue()).isNotNull(); + assertThat(new String(holder.getValue(), UTF_8)).isEqualTo("value"); + + exists = db.keyMayExist(readOptions, "key".getBytes(UTF_8), null); assertThat(exists).isTrue(); - assertThat(retValue.toString()).isEqualTo("value"); - retValue = new StringBuilder(); - exists = db.keyMayExist(readOptions, sliceKey, offset, len, retValue); + exists = db.keyMayExist(readOptions, sliceKey, offset, len, holder); + assertThat(exists).isTrue(); + assertThat(holder.getValue()).isNotNull(); + assertThat(holder.getValue()).isEqualTo(sliceValue); + + exists = db.keyMayExist(readOptions, sliceKey, offset, len, null); assertThat(exists).isTrue(); - assertThat(retValue.toString().getBytes()).isEqualTo(sliceValue); } // Test with column family - retValue = new StringBuilder(); - exists = db.keyMayExist(columnFamilyHandleList.get(0), "key".getBytes(), - retValue); + exists = db.keyMayExist(columnFamilyHandleList.get(0), "key".getBytes(UTF_8), + holder); + assertThat(exists).isTrue(); + assertThat(holder.getValue()).isNotNull(); + assertThat(new String(holder.getValue(), UTF_8)).isEqualTo("value"); + + exists = db.keyMayExist(columnFamilyHandleList.get(0), "key".getBytes(UTF_8), + null); assertThat(exists).isTrue(); - assertThat(retValue.toString()).isEqualTo("value"); // Test slice sky with column family - retValue = new StringBuilder(); exists = db.keyMayExist(columnFamilyHandleList.get(0), sliceKey, offset, len, - retValue); + holder); + assertThat(exists).isTrue(); + assertThat(holder.getValue()).isNotNull(); + assertThat(holder.getValue()).isEqualTo(sliceValue); + + exists = db.keyMayExist(columnFamilyHandleList.get(0), sliceKey, offset, len, + null); assertThat(exists).isTrue(); - assertThat(retValue.toString().getBytes()).isEqualTo(sliceValue); // Test with column family and readOptions try (final ReadOptions readOptions = new ReadOptions()) { - retValue = new StringBuilder(); - exists = db.keyMayExist(readOptions, - columnFamilyHandleList.get(0), "key".getBytes(), - retValue); + exists = db.keyMayExist(columnFamilyHandleList.get(0), readOptions, + "key".getBytes(UTF_8), holder); + assertThat(exists).isTrue(); + assertThat(holder.getValue()).isNotNull(); + assertThat(new String(holder.getValue(), UTF_8)).isEqualTo("value"); + + exists = db.keyMayExist(columnFamilyHandleList.get(0), readOptions, + "key".getBytes(UTF_8), null); assertThat(exists).isTrue(); - assertThat(retValue.toString()).isEqualTo("value"); // Test slice key with column family and read options - retValue = new StringBuilder(); - exists = db.keyMayExist(readOptions, - columnFamilyHandleList.get(0), sliceKey, offset, len, - retValue); + exists = db.keyMayExist(columnFamilyHandleList.get(0), readOptions, + sliceKey, offset, len, holder); + assertThat(exists).isTrue(); + assertThat(holder.getValue()).isNotNull(); + assertThat(holder.getValue()).isEqualTo(sliceValue); + + exists = db.keyMayExist(columnFamilyHandleList.get(0), readOptions, + sliceKey, offset, len, null); assertThat(exists).isTrue(); - assertThat(retValue.toString().getBytes()).isEqualTo(sliceValue); } - // KeyMayExist in CF1 must return false - assertThat(db.keyMayExist(columnFamilyHandleList.get(1), - "key".getBytes(), retValue)).isFalse(); + // KeyMayExist in CF1 must return null value + exists = db.keyMayExist(columnFamilyHandleList.get(1), + "key".getBytes(UTF_8), holder); + assertThat(exists).isFalse(); + assertThat(holder.getValue()).isNull(); + exists = db.keyMayExist(columnFamilyHandleList.get(1), + "key".getBytes(UTF_8), null); + assertThat(exists).isFalse(); // slice key - assertThat(db.keyMayExist(columnFamilyHandleList.get(1), - sliceKey, 1, 3, retValue)).isFalse(); + exists = db.keyMayExist(columnFamilyHandleList.get(1), + sliceKey, 1, 3, holder); + assertThat(exists).isFalse(); + assertThat(holder.getValue()).isNull(); + exists = db.keyMayExist(columnFamilyHandleList.get(1), + sliceKey, 1, 3, null); + assertThat(exists).isFalse(); } finally { for (final ColumnFamilyHandle columnFamilyHandle : columnFamilyHandleList) { @@ -124,4 +162,31 @@ public class KeyMayExistTest { } } } + + @Test + public void keyMayExistNonUnicodeString() throws RocksDBException { + try (final Options options = new Options() + .setCreateIfMissing(true) + .setCreateMissingColumnFamilies(true); + final RocksDB db = RocksDB.open(options, + dbFolder.getRoot().getAbsolutePath())) { + final byte key[] = "key".getBytes(UTF_8); + final byte value[] = { (byte)0x80 }; // invalid unicode code-point + db.put(key, value); + + final byte buf[] = new byte[10]; + final int read = db.get(key, buf); + assertThat(read).isEqualTo(1); + assertThat(buf).startsWith(value); + + final Holder holder = new Holder<>(); + boolean exists = db.keyMayExist("key".getBytes(UTF_8), holder); + assertThat(exists).isTrue(); + assertThat(holder.getValue()).isNotNull(); + assertThat(holder.getValue()).isEqualTo(value); + + exists = db.keyMayExist("key".getBytes(UTF_8), null); + assertThat(exists).isTrue(); + } + } }