Add secondary index commands to KVExecutor (refs #180)

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;
   }
 

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<std::string> attrs;
+  std::string bad = std::string("ckX") + "idx" + kCompositeKeyDelim + "pk";
+  EXPECT_FALSE(DecodeCompositeKey(bad, &idx, &attrs, &pk));
+}

chain/storage/duckdb.h

@@ -101,20 +101,19 @@ 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<std::string> GetByCompositeKeyPrefix(
       const std::string&) override {
     return {};
   }
 
   int UpdateCompositeKey(const std::string&, const std::string&) override {
-    return 0;
+    return -1;
   }
 
  private:

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));

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());

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"],

executor/kv/kv_executor.cpp

@@ -21,9 +21,23 @@
 
 #include <glog/logging.h>
 
+#include <set>
+
+#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) {
   storage_ = std::move(storage);
@@ -75,6 +89,11 @@ std::unique_ptr<std::string> 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<std::string> resp = contract_manager_->ExecuteData(kv_request.smart_contract_request());
@@ -129,6 +148,11 @@ std::unique_ptr<std::string> 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<std::string> resp = contract_manager_->ExecuteData(kv_request.smart_contract_request());
@@ -145,6 +169,11 @@ std::unique_ptr<std::string> 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<std::string> 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<std::string> 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<std::string>& 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<std::string>& 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<std::string>& old_attributes,
+    const std::vector<std::string>& 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<std::string>& 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<std::string> primary_keys;
+  for (const std::string& composite_key :
+       storage_->GetByCompositeKeyPrefix(prefix)) {
+    std::string decoded_index;
+    std::vector<std::string> 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