diff --git a/chain/storage/composite_key_codec.cpp b/chain/storage/composite_key_codec.cpp index 781fa99543..3df108941c 100644 --- a/chain/storage/composite_key_codec.cpp +++ b/chain/storage/composite_key_codec.cpp @@ -75,8 +75,11 @@ std::string EncodeCompositeKeyPrefix( result.append(attribute_prefix[i]); } - // Trailing delimiter makes this a strict byte prefix of the full key. - result.push_back(kCompositeKeyDelim); + // End on a delimiter so "Davis" can't match "Davisville". With no + // attributes, the delimiter after index_name already does this. + if (!attribute_prefix.empty()) { + result.push_back(kCompositeKeyDelim); + } return result; } @@ -90,7 +93,8 @@ bool DecodeCompositeKey(const std::string& encoded, size_t ns_len = std::strlen(kCompositeKeyNamespace); if (encoded.size() < ns_len + 1 || - encoded.compare(0, ns_len, kCompositeKeyNamespace) != 0) { + encoded.compare(0, ns_len, kCompositeKeyNamespace) != 0 || + encoded[ns_len] != kCompositeKeyDelim) { return false; } diff --git a/chain/storage/composite_key_codec_test.cpp b/chain/storage/composite_key_codec_test.cpp index 30509f5f98..306dc21446 100644 --- a/chain/storage/composite_key_codec_test.cpp +++ b/chain/storage/composite_key_codec_test.cpp @@ -82,3 +82,19 @@ TEST(CompositeKeyCodecTest, EmptyAttributes) { EXPECT_EQ(attrs.size(), 0u); EXPECT_EQ(pk, "user:1"); } + +TEST(CompositeKeyCodecTest, PrefixWithNoAttributesMatchesWholeIndex) { + std::string prefix = EncodeCompositeKeyPrefix("byCity", {}); + EXPECT_EQ(EncodeCompositeKey("byCity", {"Davis"}, "user:1").find(prefix), 0u); + EXPECT_EQ(EncodeCompositeKey("byCity", {}, "user:2").find(prefix), 0u); + // Must not reach into another index whose name starts the same way. + EXPECT_NE(EncodeCompositeKey("byCity2", {"Davis"}, "user:3").find(prefix), + 0u); +} + +TEST(CompositeKeyCodecTest, DecodeRejectsMissingDelimiterAfterNamespace) { + std::string idx, pk; + std::vector attrs; + std::string bad = std::string("ckX") + "idx" + kCompositeKeyDelim + "pk"; + EXPECT_FALSE(DecodeCompositeKey(bad, &idx, &attrs, &pk)); +} diff --git a/chain/storage/duckdb.h b/chain/storage/duckdb.h index 153c19c61b..8573669c29 100644 --- a/chain/storage/duckdb.h +++ b/chain/storage/duckdb.h @@ -101,12 +101,11 @@ class DuckDB : public Storage { bool Flush(bool should_sync = false) override { return true; } - // No-op overrides so DuckDB stays concrete: Storage declares these - // composite-key methods pure virtual, and without them DuckDB would be - // abstract and fail to compile. The SQL backend doesn't use them. - int CreateCompositeKey(const std::string&) override { return 0; } + // Composite keys aren't supported on DuckDB. Return -1 (failure) so callers + // don't think an index entry was written. + int CreateCompositeKey(const std::string&) override { return -1; } - int DeleteCompositeKey(const std::string&) override { return 0; } + int DeleteCompositeKey(const std::string&) override { return -1; } std::vector GetByCompositeKeyPrefix( const std::string&) override { @@ -114,7 +113,7 @@ class DuckDB : public Storage { } int UpdateCompositeKey(const std::string&, const std::string&) override { - return 0; + return -1; } private: diff --git a/chain/storage/kv_storage_test.cpp b/chain/storage/kv_storage_test.cpp index b6719072c1..f3ebbac872 100644 --- a/chain/storage/kv_storage_test.cpp +++ b/chain/storage/kv_storage_test.cpp @@ -418,6 +418,20 @@ TEST_P(KVStorageTest, GetAllItemsExcludesCompositeKeys) { EXPECT_EQ(davis.size(), 1u); } +// GetRange must not return index entries either. "ck\0..." sorts between +// "apple" and "dog", so an unfiltered scan would include it. +TEST_P(KVStorageTest, GetRangeExcludesCompositeKeys) { + EXPECT_EQ(storage->SetValue("apple", "1"), 0); + EXPECT_EQ(storage->SetValue("dog", "2"), 0); + EXPECT_EQ(storage->CreateCompositeKey( + EncodeCompositeKey("byCity", {"Davis"}, "apple")), + 0); + + // MemoryDB's GetRange is unordered, so accept either order. + std::string range = storage->GetRange("a", "z"); + EXPECT_TRUE(range == "[1,2]" || range == "[2,1]") << range; +} + INSTANTIATE_TEST_CASE_P(KVStorageTest, KVStorageTest, ::testing::Values(MEM, LEVELDB, LEVELDB_WITH_BLOCK_CACHE)); diff --git a/chain/storage/leveldb.cpp b/chain/storage/leveldb.cpp index 15b1a85115..71c253dfa9 100644 --- a/chain/storage/leveldb.cpp +++ b/chain/storage/leveldb.cpp @@ -203,10 +203,18 @@ std::string ResLevelDB::GetValue(const std::string& key) { std::string ResLevelDB::GetRange(const std::string& min_key, const std::string& max_key) { std::string values = "["; + // Hide composite-key index entries from user-facing scans. + const std::string ck_prefix = + std::string(kCompositeKeyNamespace) + kCompositeKeyDelim; + leveldb::Iterator* it = db_->NewIterator(leveldb::ReadOptions()); bool first_iteration = true; for (it->Seek(min_key); it->Valid() && it->key().ToString() <= max_key; it->Next()) { + if (it->key().size() >= ck_prefix.size() && + memcmp(it->key().data(), ck_prefix.data(), ck_prefix.size()) == 0) { + continue; + } if (!first_iteration) values.append(","); first_iteration = false; values.append(it->value().ToString()); diff --git a/executor/kv/BUILD b/executor/kv/BUILD index 89bb67c7fd..5790e016fd 100644 --- a/executor/kv/BUILD +++ b/executor/kv/BUILD @@ -26,6 +26,7 @@ cc_library( hdrs = ["kv_executor.h"], deps = [ "//chain/storage", + "//chain/storage:composite_key_codec", "//common:comm", "//executor/common:transaction_manager", "//executor/contract/executor:contract_executor", @@ -34,6 +35,17 @@ cc_library( ], ) +cc_test( + name = "kv_executor_index_test", + srcs = ["kv_executor_index_test.cpp"], + deps = [ + ":kv_executor", + "//chain/storage:leveldb", + "//chain/storage:memory_db", + "//common/test:test_main", + ], +) + cc_test( name = "kv_executor_test", srcs = ["kv_executor_test.cpp"], diff --git a/executor/kv/kv_executor.cpp b/executor/kv/kv_executor.cpp index 9751e64533..19f53600e8 100644 --- a/executor/kv/kv_executor.cpp +++ b/executor/kv/kv_executor.cpp @@ -21,9 +21,23 @@ #include +#include + +#include "chain/storage/composite_key_codec.h" #include "executor/contract/executor/contract_executor.h" namespace resdb { +namespace { + +// Index entry keys start with "ck\0". SET must not write them, or a client +// could plant fake index entries. +bool IsCompositeKeyRange(const std::string& key) { + static const std::string kPrefix = std::string( + storage::kCompositeKeyNamespace) + storage::kCompositeKeyDelim; + return key.compare(0, kPrefix.size(), kPrefix) == 0; +} + +} // namespace KVExecutor::KVExecutor(std::unique_ptr storage) { storage_ = std::move(storage); @@ -75,6 +89,11 @@ std::unique_ptr KVExecutor::ExecuteRequest( std::string result = ExecuteSQL(kv_request.sql_query()); kv_response.set_sql_response(result); kv_response.set_value(result); // keep legacy field populated + } else if (kv_request.cmd() == KVRequest::CREATE_INDEX_ENTRY || + kv_request.cmd() == KVRequest::DELETE_INDEX_ENTRY || + kv_request.cmd() == KVRequest::UPDATE_INDEX_ENTRY || + kv_request.cmd() == KVRequest::QUERY_BY_INDEX) { + ExecuteIndexCommand(kv_request, &kv_response); } else if(!kv_request.smart_contract_request().empty()){ std::unique_ptr resp = contract_manager_->ExecuteData(kv_request.smart_contract_request()); @@ -129,6 +148,11 @@ std::unique_ptr KVExecutor::ExecuteData( std::string result = ExecuteSQL(kv_request.sql_query()); kv_response.set_sql_response(result); kv_response.set_value(result); // keep legacy field populated + } else if (kv_request.cmd() == KVRequest::CREATE_INDEX_ENTRY || + kv_request.cmd() == KVRequest::DELETE_INDEX_ENTRY || + kv_request.cmd() == KVRequest::UPDATE_INDEX_ENTRY || + kv_request.cmd() == KVRequest::QUERY_BY_INDEX) { + ExecuteIndexCommand(kv_request, &kv_response); } else if(!kv_request.smart_contract_request().empty()){ std::unique_ptr resp = contract_manager_->ExecuteData(kv_request.smart_contract_request()); @@ -145,6 +169,11 @@ std::unique_ptr KVExecutor::ExecuteData( } void KVExecutor::Set(const std::string& key, const std::string& value) { + if (IsCompositeKeyRange(key)) { + LOG(ERROR) << "SET rejected: keys starting with \"ck\\0\" are reserved " + "for secondary indexes"; + return; + } storage_->SetValueWithSeq(key, value, seq_); } @@ -162,6 +191,11 @@ std::string KVExecutor::GetRange(const std::string& min_key, void KVExecutor::SetWithVersion(const std::string& key, const std::string& value, int version) { + if (IsCompositeKeyRange(key)) { + LOG(ERROR) << "SET_WITH_VERSION rejected: keys starting with \"ck\\0\" " + "are reserved for secondary indexes"; + return; + } storage_->SetValueWithVersion(key, value, version); } @@ -220,5 +254,151 @@ std::string KVExecutor::ExecuteSQL(const std::string& sql_query) { return storage_->ExecuteSQL(sql_query); } +void KVExecutor::ExecuteIndexCommand(const KVRequest& request, + KVResponse* response) { + std::vector attributes(request.attributes().begin(), + request.attributes().end()); + switch (request.cmd()) { + case KVRequest::CREATE_INDEX_ENTRY: + response->set_success(CreateIndexEntry(request.index_name(), attributes, + request.primary_key())); + break; + case KVRequest::DELETE_INDEX_ENTRY: + response->set_success(DeleteIndexEntry(request.index_name(), attributes, + request.primary_key())); + break; + case KVRequest::UPDATE_INDEX_ENTRY: { + std::vector new_attributes(request.new_attributes().begin(), + request.new_attributes().end()); + response->set_success(UpdateIndexEntry(request.index_name(), attributes, + new_attributes, + request.primary_key())); + break; + } + case KVRequest::QUERY_BY_INDEX: + QueryByIndex(request.index_name(), attributes, request.with_values(), + response->mutable_items()); + break; + default: + break; + } +} + +// MemoryDB stores SET and SET_WITH_VERSION records separately, so check both. +bool KVExecutor::PrimaryKeyExists(const std::string& key) { + return !Get(key).empty() || + !storage_->GetValueWithVersion(key, 0).first.empty(); +} + +// Exact match: a prefix search for "user:1" also returns "user:10". +bool KVExecutor::IndexEntryExists(const std::string& composite_key) { + for (const std::string& key : + storage_->GetByCompositeKeyPrefix(composite_key)) { + if (key == composite_key) { + return true; + } + } + return false; +} + +bool KVExecutor::CreateIndexEntry(const std::string& index_name, + const std::vector& attributes, + const std::string& primary_key) { + if (index_name.empty() || primary_key.empty()) { + LOG(ERROR) << "CreateIndexEntry: index_name and primary_key are required"; + return false; + } + if (!PrimaryKeyExists(primary_key)) { + LOG(ERROR) << "CreateIndexEntry: primary key not found: " << primary_key; + return false; + } + std::string composite_key = + storage::EncodeCompositeKey(index_name, attributes, primary_key); + if (composite_key.empty()) { + LOG(ERROR) << "CreateIndexEntry: input contains a \\0 byte"; + return false; + } + return storage_->CreateCompositeKey(composite_key) == 0; +} + +bool KVExecutor::DeleteIndexEntry(const std::string& index_name, + const std::vector& attributes, + const std::string& primary_key) { + if (index_name.empty() || primary_key.empty()) { + LOG(ERROR) << "DeleteIndexEntry: index_name and primary_key are required"; + return false; + } + std::string composite_key = + storage::EncodeCompositeKey(index_name, attributes, primary_key); + if (composite_key.empty() || !IndexEntryExists(composite_key)) { + LOG(ERROR) << "DeleteIndexEntry: no such index entry"; + return false; + } + return storage_->DeleteCompositeKey(composite_key) == 0; +} + +bool KVExecutor::UpdateIndexEntry( + const std::string& index_name, + const std::vector& old_attributes, + const std::vector& new_attributes, + const std::string& primary_key) { + if (index_name.empty() || primary_key.empty()) { + LOG(ERROR) << "UpdateIndexEntry: index_name and primary_key are required"; + return false; + } + std::string old_key = + storage::EncodeCompositeKey(index_name, old_attributes, primary_key); + std::string new_key = + storage::EncodeCompositeKey(index_name, new_attributes, primary_key); + if (old_key.empty() || new_key.empty()) { + LOG(ERROR) << "UpdateIndexEntry: input contains a \\0 byte"; + return false; + } + if (!IndexEntryExists(old_key)) { + LOG(ERROR) << "UpdateIndexEntry: no existing entry to update"; + return false; + } + return storage_->UpdateCompositeKey(old_key, new_key) == 0; +} + +void KVExecutor::QueryByIndex(const std::string& index_name, + const std::vector& attribute_prefix, + bool with_values, Items* items) { + if (index_name.empty()) { + LOG(ERROR) << "QueryByIndex: index_name is required"; + return; + } + std::string prefix = + storage::EncodeCompositeKeyPrefix(index_name, attribute_prefix); + if (prefix.empty()) { + LOG(ERROR) << "QueryByIndex: input contains a \\0 byte"; + return; + } + // A record can have several matching entries (e.g. {"Davis", "active"} and + // {"Davis", "inactive"}), so collect into a set: each key once, sorted. + std::set primary_keys; + for (const std::string& composite_key : + storage_->GetByCompositeKeyPrefix(prefix)) { + std::string decoded_index; + std::vector decoded_attributes; + std::string primary_key; + if (storage::DecodeCompositeKey(composite_key, &decoded_index, + &decoded_attributes, &primary_key)) { + primary_keys.insert(primary_key); + } + } + for (const std::string& primary_key : primary_keys) { + Item* item = items->add_item(); + item->set_key(primary_key); + if (with_values) { + // Read here, so the caller doesn't send one request per match. + std::string value = Get(primary_key); + if (value.empty()) { + value = storage_->GetValueWithVersion(primary_key, 0).first; + } + item->mutable_value_info()->set_value(value); + } + } +} } // namespace resdb diff --git a/executor/kv/kv_executor.h b/executor/kv/kv_executor.h index f0199b21f5..93e70ed69f 100644 --- a/executor/kv/kv_executor.h +++ b/executor/kv/kv_executor.h @@ -21,7 +21,9 @@ #include #include +#include #include +#include #include "chain/storage/storage.h" #include "executor/common/transaction_manager.h" @@ -58,7 +60,35 @@ class KVExecutor : public TransactionManager { void GetTopHistory(const std::string& key, int top_number, Items* items); std::string ExecuteSQL(const std::string& sql_query); + // Secondary indexes (issue #180). An index entry says which attributes a + // record has, e.g. ("by_city", {"Davis"}) -> "user:1", and is stored as the + // key "ck\0by_city\0Davis\0user:1". Only these functions build such keys. + + // Fails if the record (primary_key) doesn't exist. + bool CreateIndexEntry(const std::string& index_name, + const std::vector& attributes, + const std::string& primary_key); + // Fails if the entry doesn't exist. + bool DeleteIndexEntry(const std::string& index_name, + const std::vector& attributes, + const std::string& primary_key); + // Moves an entry to new attributes in one write. Fails if it doesn't exist. + bool UpdateIndexEntry(const std::string& index_name, + const std::vector& old_attributes, + const std::vector& new_attributes, + const std::string& primary_key); + // Adds the primary keys of matching entries to `items`, each once, sorted. + // With `with_values`, each record's value is read and returned too. + void QueryByIndex(const std::string& index_name, + const std::vector& attribute_prefix, + bool with_values, Items* items); + private: + // Shared by ExecuteRequest and ExecuteData. + void ExecuteIndexCommand(const KVRequest& request, KVResponse* response); + bool PrimaryKeyExists(const std::string& key); + bool IndexEntryExists(const std::string& composite_key); + std::unique_ptr contract_manager_; }; diff --git a/executor/kv/kv_executor_index_test.cpp b/executor/kv/kv_executor_index_test.cpp new file mode 100644 index 0000000000..88fd7cff13 --- /dev/null +++ b/executor/kv/kv_executor_index_test.cpp @@ -0,0 +1,421 @@ +/* + * 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. + */ + +#include +#include + +#include +#include +#include +#include +#include + +#include "chain/storage/leveldb.h" +#include "chain/storage/memory_db.h" +#include "executor/kv/kv_executor.h" +#include "proto/kv/kv.pb.h" + +namespace resdb { +namespace { + +using ::testing::Combine; +using ::testing::ElementsAre; +using ::testing::IsEmpty; +using ::testing::Values; + +enum Backend { MEMORY_DB, LEVEL_DB }; +// ExecuteData and ExecuteRequest each dispatch commands; test both. +enum EntryPoint { EXECUTE_DATA, EXECUTE_REQUEST }; + +class KVExecutorIndexTest + : public ::testing::TestWithParam> { + protected: + KVExecutorIndexTest() { + std::unique_ptr storage; + if (std::get<0>(GetParam()) == LEVEL_DB) { + db_path_ = ::testing::TempDir() + "kv_executor_index_test_db"; + std::filesystem::remove_all(db_path_); + storage = storage::NewResLevelDB(db_path_); + } else { + storage = std::make_unique(); + } + executor_ = std::make_unique(std::move(storage)); + } + + ~KVExecutorIndexTest() override { + executor_.reset(); // close LevelDB before deleting its folder + if (!db_path_.empty()) { + std::filesystem::remove_all(db_path_); + } + } + + KVResponse Execute(const KVRequest& request) { + std::unique_ptr raw; + if (std::get<1>(GetParam()) == EXECUTE_DATA) { + std::string data; + request.SerializeToString(&data); + raw = executor_->ExecuteData(data); + } else { + raw = executor_->ExecuteRequest(request); + } + KVResponse response; + if (raw != nullptr) { + response.ParseFromString(*raw); + } + return response; + } + + void Set(const std::string& key, const std::string& value) { + KVRequest request; + request.set_cmd(KVRequest::SET); + request.set_key(key); + request.set_value(value); + Execute(request); + } + + void SetWithVersion(const std::string& key, const std::string& value) { + KVRequest request; + request.set_cmd(KVRequest::SET_WITH_VERSION); + request.set_key(key); + request.set_value(value); + request.set_version(0); + Execute(request); + } + + std::string Get(const std::string& key) { + KVRequest request; + request.set_cmd(KVRequest::GET); + request.set_key(key); + return Execute(request).value(); + } + + bool IndexCommand(KVRequest::CMD cmd, const std::string& index, + const std::vector& attributes, + const std::string& primary_key, + const std::vector& new_attributes = {}) { + KVRequest request; + request.set_cmd(cmd); + request.set_index_name(index); + for (const auto& a : attributes) request.add_attributes(a); + for (const auto& a : new_attributes) request.add_new_attributes(a); + request.set_primary_key(primary_key); + return Execute(request).success(); + } + + bool Create(const std::string& index, const std::vector& attrs, + const std::string& pk) { + return IndexCommand(KVRequest::CREATE_INDEX_ENTRY, index, attrs, pk); + } + + bool Delete(const std::string& index, const std::vector& attrs, + const std::string& pk) { + return IndexCommand(KVRequest::DELETE_INDEX_ENTRY, index, attrs, pk); + } + + bool Update(const std::string& index, const std::vector& from, + const std::vector& to, const std::string& pk) { + return IndexCommand(KVRequest::UPDATE_INDEX_ENTRY, index, from, pk, to); + } + + std::vector> QueryWithValues( + const std::string& index, const std::vector& attrs) { + KVRequest request; + request.set_cmd(KVRequest::QUERY_BY_INDEX); + request.set_index_name(index); + for (const auto& a : attrs) request.add_attributes(a); + request.set_with_values(true); + KVResponse response = Execute(request); + std::vector> records; + for (const Item& item : response.items().item()) { + records.push_back({item.key(), item.value_info().value()}); + } + return records; + } + + std::vector Query(const std::string& index, + const std::vector& attrs) { + KVRequest request; + request.set_cmd(KVRequest::QUERY_BY_INDEX); + request.set_index_name(index); + for (const auto& a : attrs) request.add_attributes(a); + // Keep the response in a variable: looping over Execute(...).items() + // directly would read from a temporary that's already been destroyed. + KVResponse response = Execute(request); + std::vector keys; + for (const Item& item : response.items().item()) { + keys.push_back(item.key()); + } + return keys; + } + + std::string db_path_; + std::unique_ptr executor_; +}; + +// A string holding a real zero byte (a plain "a\0b" literal stops at the \0). +std::string WithNul(const std::string& a, const std::string& b) { + return a + std::string(1, '\0') + b; +} + +TEST_P(KVExecutorIndexTest, CreateThenQuery) { + Set("user:1", "Ana"); + Set("user:2", "Bo"); + Set("user:3", "Cy"); + EXPECT_TRUE(Create("by_city", {"Davis"}, "user:1")); + EXPECT_TRUE(Create("by_city", {"Davis"}, "user:2")); + EXPECT_TRUE(Create("by_city", {"Sacramento"}, "user:3")); + + EXPECT_THAT(Query("by_city", {"Davis"}), ElementsAre("user:1", "user:2")); + EXPECT_THAT(Query("by_city", {"Sacramento"}), ElementsAre("user:3")); + EXPECT_THAT(Query("by_city", {"Chico"}), IsEmpty()); +} + +TEST_P(KVExecutorIndexTest, CreateFailsWhenRecordDoesNotExist) { + EXPECT_FALSE(Create("by_city", {"Davis"}, "user:99")); + EXPECT_THAT(Query("by_city", {"Davis"}), IsEmpty()); +} + +// MemoryDB keeps SET_WITH_VERSION records in a separate map from SET records. +TEST_P(KVExecutorIndexTest, CreateWorksForRecordWrittenWithVersion) { + SetWithVersion("user:5", "Eve"); + EXPECT_TRUE(Create("by_city", {"Davis"}, "user:5")); + EXPECT_THAT(Query("by_city", {"Davis"}), ElementsAre("user:5")); +} + +TEST_P(KVExecutorIndexTest, CreateRejectsBadInput) { + Set("user:1", "Ana"); + EXPECT_FALSE(Create("", {"Davis"}, "user:1")); + EXPECT_FALSE(Create("by_city", {"Davis"}, "")); + EXPECT_FALSE(Create("by_city", {WithNul("Da", "vis")}, "user:1")); + EXPECT_FALSE(Create(WithNul("by", "city"), {"Davis"}, "user:1")); + EXPECT_THAT(Query("by_city", {"Davis"}), IsEmpty()); +} + +TEST_P(KVExecutorIndexTest, CreatingTwiceKeepsOneEntry) { + Set("user:1", "Ana"); + EXPECT_TRUE(Create("by_city", {"Davis"}, "user:1")); + EXPECT_TRUE(Create("by_city", {"Davis"}, "user:1")); + EXPECT_THAT(Query("by_city", {"Davis"}), ElementsAre("user:1")); +} + +// "Davis" must not match "Davisville". +TEST_P(KVExecutorIndexTest, QueryMatchesWholeAttribute) { + Set("user:1", "Ana"); + Set("user:2", "Bo"); + EXPECT_TRUE(Create("by_city", {"Davis"}, "user:1")); + EXPECT_TRUE(Create("by_city", {"Davisville"}, "user:2")); + EXPECT_THAT(Query("by_city", {"Davis"}), ElementsAre("user:1")); +} + +// Same attribute, different index: queries must not mix them. +TEST_P(KVExecutorIndexTest, QueryOnlyReadsItsOwnIndex) { + Set("user:1", "Ana"); + EXPECT_TRUE(Create("by_city", {"Davis"}, "user:1")); + EXPECT_THAT(Query("by_birthplace", {"Davis"}), IsEmpty()); + EXPECT_THAT(Query("by_cit", {"Davis"}), IsEmpty()); +} + +TEST_P(KVExecutorIndexTest, QueryWithLeadingAttributes) { + Set("user:1", "Ana"); + Set("user:2", "Bo"); + EXPECT_TRUE(Create("by_city_status", {"Davis", "active"}, "user:1")); + EXPECT_TRUE(Create("by_city_status", {"Davis", "inactive"}, "user:2")); + EXPECT_THAT(Query("by_city_status", {"Davis"}), + ElementsAre("user:1", "user:2")); + EXPECT_THAT(Query("by_city_status", {"Davis", "active"}), + ElementsAre("user:1")); +} + +// No attributes = every entry in the index. +TEST_P(KVExecutorIndexTest, QueryWithNoAttributesReturnsWholeIndex) { + Set("user:1", "Ana"); + Set("user:2", "Bo"); + EXPECT_TRUE(Create("by_city", {"Davis"}, "user:1")); + EXPECT_TRUE(Create("by_city", {"Sacramento"}, "user:2")); + EXPECT_THAT(Query("by_city", {}), ElementsAre("user:1", "user:2")); +} + +TEST_P(KVExecutorIndexTest, QueryRejectsBadInput) { + Set("user:1", "Ana"); + EXPECT_TRUE(Create("by_city", {"Davis"}, "user:1")); + EXPECT_THAT(Query("", {"Davis"}), IsEmpty()); + EXPECT_THAT(Query("by_city", {WithNul("Da", "vis")}), IsEmpty()); +} + +TEST_P(KVExecutorIndexTest, DeleteRemovesOnlyThatEntry) { + Set("user:1", "Ana"); + Set("user:10", "Jo"); + EXPECT_TRUE(Create("by_city", {"Davis"}, "user:1")); + EXPECT_TRUE(Create("by_city", {"Davis"}, "user:10")); + + EXPECT_TRUE(Delete("by_city", {"Davis"}, "user:1")); + EXPECT_THAT(Query("by_city", {"Davis"}), ElementsAre("user:10")); + // user:1's entry is gone; "user:10" must not count as a match for it. + EXPECT_FALSE(Delete("by_city", {"Davis"}, "user:1")); +} + +TEST_P(KVExecutorIndexTest, DeleteFailsWhenEntryDoesNotExist) { + Set("user:1", "Ana"); + EXPECT_TRUE(Create("by_city", {"Davis"}, "user:1")); + EXPECT_FALSE(Delete("by_city", {"Sacramento"}, "user:1")); + EXPECT_THAT(Query("by_city", {"Davis"}), ElementsAre("user:1")); +} + +TEST_P(KVExecutorIndexTest, UpdateMovesEntry) { + Set("user:1", "Ana"); + Set("user:2", "Bo"); + EXPECT_TRUE(Create("by_city", {"Davis"}, "user:1")); + EXPECT_TRUE(Create("by_city", {"Davis"}, "user:2")); + + EXPECT_TRUE(Update("by_city", {"Davis"}, {"Sacramento"}, "user:2")); + EXPECT_THAT(Query("by_city", {"Davis"}), ElementsAre("user:1")); + EXPECT_THAT(Query("by_city", {"Sacramento"}), ElementsAre("user:2")); +} + +// A wrong "from" must change nothing, and must not create the new entry. +TEST_P(KVExecutorIndexTest, UpdateFailsWhenEntryDoesNotExist) { + Set("user:1", "Ana"); + EXPECT_TRUE(Create("by_city", {"Davis"}, "user:1")); + EXPECT_FALSE(Update("by_city", {"Chico"}, {"Sacramento"}, "user:1")); + EXPECT_THAT(Query("by_city", {"Davis"}), ElementsAre("user:1")); + EXPECT_THAT(Query("by_city", {"Sacramento"}), IsEmpty()); +} + +TEST_P(KVExecutorIndexTest, UpdateRejectsBadInput) { + Set("user:1", "Ana"); + EXPECT_TRUE(Create("by_city", {"Davis"}, "user:1")); + EXPECT_FALSE(Update("by_city", {"Davis"}, {WithNul("Sac", "ramento")}, + "user:1")); + EXPECT_THAT(Query("by_city", {"Davis"}), ElementsAre("user:1")); +} + +// Values are opaque bytes, so a record holding an image indexes like any other. +TEST_P(KVExecutorIndexTest, IndexRecordWithBinaryValue) { + std::string image("\x89PNG\r\n\x1a\n", 8); // PNG signature + image += std::string(3, '\0'); // zero bytes + image += std::string("\xff\xfe\xfd", 3); // not valid text + + Set("img:1", image); + EXPECT_EQ(Get("img:1"), image); + EXPECT_TRUE(Create("by_type", {"png"}, "img:1")); + EXPECT_THAT(Query("by_type", {"png"}), ElementsAre("img:1")); +} + +// A record with an empty value can't be told apart from a missing one. +TEST_P(KVExecutorIndexTest, CannotIndexRecordWithEmptyValue) { + Set("user:1", ""); + EXPECT_FALSE(Create("by_city", {"Davis"}, "user:1")); +} + +// SET and SET_WITH_VERSION must not be able to plant a fake index entry. +TEST_P(KVExecutorIndexTest, SetCannotWriteIndexEntries) { + const std::string forged = std::string("ck") + '\0' + "by_city" + '\0' + + "Davis" + '\0' + "user:666"; + Set(forged, "x"); + SetWithVersion(forged, "x"); + EXPECT_THAT(Query("by_city", {"Davis"}), IsEmpty()); + EXPECT_EQ(Get(forged), ""); +} + +// Keys that merely start with "ck" (no \0) are ordinary user keys. +TEST_P(KVExecutorIndexTest, SetStillAllowsKeysStartingWithCk) { + Set("ck_balance", "100"); + EXPECT_EQ(Get("ck_balance"), "100"); +} + +// user:1 has two matching entries; it must come back once. +TEST_P(KVExecutorIndexTest, QueryReturnsEachRecordOnce) { + Set("user:1", "Ana"); + EXPECT_TRUE(Create("by_city_status", {"Davis", "active"}, "user:1")); + EXPECT_TRUE(Create("by_city_status", {"Davis", "inactive"}, "user:1")); + EXPECT_THAT(Query("by_city_status", {"Davis"}), ElementsAre("user:1")); +} + +// Stored order is by attributes ("active" < "inactive"), so user:2 comes +// first internally; results must still be sorted by primary key. +TEST_P(KVExecutorIndexTest, QueryResultsAreSortedByPrimaryKey) { + Set("user:1", "Ana"); + Set("user:2", "Bo"); + EXPECT_TRUE(Create("by_city_status", {"Davis", "inactive"}, "user:1")); + EXPECT_TRUE(Create("by_city_status", {"Davis", "active"}, "user:2")); + EXPECT_THAT(Query("by_city_status", {"Davis"}), + ElementsAre("user:1", "user:2")); +} + +TEST_P(KVExecutorIndexTest, UpdateToSameAttributesKeepsEntry) { + Set("user:1", "Ana"); + EXPECT_TRUE(Create("by_city", {"Davis"}, "user:1")); + EXPECT_TRUE(Update("by_city", {"Davis"}, {"Davis"}, "user:1")); + EXPECT_THAT(Query("by_city", {"Davis"}), ElementsAre("user:1")); +} + +TEST_P(KVExecutorIndexTest, UpdateOntoExistingEntry) { + Set("user:1", "Ana"); + EXPECT_TRUE(Create("by_city", {"Davis"}, "user:1")); + EXPECT_TRUE(Create("by_city", {"Sacramento"}, "user:1")); + EXPECT_TRUE(Update("by_city", {"Davis"}, {"Sacramento"}, "user:1")); + EXPECT_THAT(Query("by_city", {"Davis"}), IsEmpty()); + EXPECT_THAT(Query("by_city", {"Sacramento"}), ElementsAre("user:1")); +} + +TEST_P(KVExecutorIndexTest, QueryWithValuesReturnsRecords) { + Set("user:1", "Ana"); + Set("user:2", "Bo"); + EXPECT_TRUE(Create("by_city", {"Davis"}, "user:1")); + EXPECT_TRUE(Create("by_city", {"Davis"}, "user:2")); + + EXPECT_THAT(QueryWithValues("by_city", {"Davis"}), + ElementsAre(std::make_pair("user:1", "Ana"), + std::make_pair("user:2", "Bo"))); +} + +// Without the flag the values must stay out of the reply. +TEST_P(KVExecutorIndexTest, QueryWithoutValuesReturnsKeysOnly) { + Set("user:1", "Ana"); + EXPECT_TRUE(Create("by_city", {"Davis"}, "user:1")); + + KVRequest request; + request.set_cmd(KVRequest::QUERY_BY_INDEX); + request.set_index_name("by_city"); + request.add_attributes("Davis"); + KVResponse response = Execute(request); + ASSERT_EQ(response.items().item_size(), 1); + EXPECT_EQ(response.items().item(0).key(), "user:1"); + EXPECT_EQ(response.items().item(0).value_info().value(), ""); +} + +TEST_P(KVExecutorIndexTest, QueryWithValuesHandlesBinaryAndVersionedRecords) { + std::string image("\x89PNG\r\n\x1a\n", 8); + image += std::string(3, '\0'); + Set("img:1", image); + SetWithVersion("user:5", "Eve"); + EXPECT_TRUE(Create("by_type", {"png"}, "img:1")); + EXPECT_TRUE(Create("by_type", {"png"}, "user:5")); + + EXPECT_THAT(QueryWithValues("by_type", {"png"}), + ElementsAre(std::make_pair("img:1", image), + std::make_pair("user:5", "Eve"))); +} + +INSTANTIATE_TEST_SUITE_P(AllBackendsAndEntryPoints, KVExecutorIndexTest, + Combine(Values(MEMORY_DB, LEVEL_DB), + Values(EXECUTE_DATA, EXECUTE_REQUEST))); + +} // namespace +} // namespace resdb diff --git a/proto/kv/kv.proto b/proto/kv/kv.proto index 6134030480..e1a4f2d810 100644 --- a/proto/kv/kv.proto +++ b/proto/kv/kv.proto @@ -36,6 +36,11 @@ message KVRequest { GET_TOP = 10; // NEW: SQL command type SQL = 11; + // Secondary index (composite key) commands, see issue #180. + CREATE_INDEX_ENTRY = 12; + DELETE_INDEX_ENTRY = 13; + UPDATE_INDEX_ENTRY = 14; + QUERY_BY_INDEX = 15; } CMD cmd = 1; string key = 2; @@ -52,6 +57,15 @@ message KVRequest { bytes smart_contract_request = 10; // NEW: raw SQL text for SQL commands (cmd == SQL) string sql_query = 11; + // NEW: For index commands. The server builds the stored key from these. + string index_name = 12; + repeated string attributes = 13; + string primary_key = 14; + // For UPDATE_INDEX_ENTRY: the attributes to move the entry to. + repeated string new_attributes = 15; + // For QUERY_BY_INDEX: also return each record's value, saving the caller + // one request per match. + bool with_values = 16; } message ValueInfo { @@ -76,4 +90,6 @@ message KVResponse { bytes smart_contract_response = 10; // NEW: response payload for SQL commands string sql_response = 11; + // Whether CREATE/DELETE/UPDATE_INDEX_ENTRY made the change. + bool success = 12; }