From 5ef63f41d28bcc3c8c10ade9be79859289a5e4ef Mon Sep 17 00:00:00 2001
From: Roman Gershman <romange@gmail.com>
Date: Thu, 7 Apr 2022 12:01:18 +0300
Subject: [PATCH] Fix bugs related to error reporting.

1. Fix #9
2. SINTER now reports error if some of its keys are of wrong type.
---
 src/core/compact_object.cc      |  4 ++--
 src/core/compact_object_test.cc |  3 ++-
 src/server/list_family.cc       |  3 +++
 src/server/set_family.cc        | 34 +++++++++++++++++++++++++--------
 src/server/set_family_test.cc   |  5 +++++
 5 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/src/core/compact_object.cc b/src/core/compact_object.cc
index ba20d49d9..4aa94c0d6 100644
--- a/src/core/compact_object.cc
+++ b/src/core/compact_object.cc
@@ -676,8 +676,8 @@ void CompactObj::SetString(std::string_view str) {
 
     if (str.size() <= kInlineLen) {
       SetMeta(str.size(), mask);
-
-      memcpy(u_.inline_str, str.data(), str.size());
+      if (!str.empty())
+        memcpy(u_.inline_str, str.data(), str.size());
       return;
     }
   }
diff --git a/src/core/compact_object_test.cc b/src/core/compact_object_test.cc
index df708ad7a..562a455cd 100644
--- a/src/core/compact_object_test.cc
+++ b/src/core/compact_object_test.cc
@@ -65,7 +65,6 @@ class CompactObjectTest : public ::testing::Test {
 TEST_F(CompactObjectTest, Basic) {
   robj* rv = createRawStringObject("foo", 3);
   cobj_.ImportRObj(rv);
-  return;
 
   CompactObj a;
   a.SetExpire(true);
@@ -83,6 +82,8 @@ TEST_F(CompactObjectTest, Basic) {
   CompactObj c = a.AsRef();
   EXPECT_EQ(a, c);
   EXPECT_TRUE(c.HasExpire());
+
+  cobj_.SetString(string_view{});
 }
 
 TEST_F(CompactObjectTest, NonInline) {
diff --git a/src/server/list_family.cc b/src/server/list_family.cc
index e4aa05064..187e22e00 100644
--- a/src/server/list_family.cc
+++ b/src/server/list_family.cc
@@ -258,9 +258,12 @@ void ListFamily::LIndex(CmdArgList args, ConnectionContext* cntx) {
   auto cb = [&](Transaction* t, EngineShard* shard) {
     return OpIndex(OpArgs{shard, t->db_index()}, key, index);
   };
+
   OpResult<string> result = cntx->transaction->ScheduleSingleHopT(std::move(cb));
   if (result) {
     (*cntx)->SendBulkString(result.value());
+  } else if (result.status() == OpStatus::WRONG_TYPE) {
+    (*cntx)->SendError(result.status());
   } else {
     (*cntx)->SendNull();
   }
diff --git a/src/server/set_family.cc b/src/server/set_family.cc
index 30b5a1b2c..56806f868 100644
--- a/src/server/set_family.cc
+++ b/src/server/set_family.cc
@@ -12,6 +12,7 @@ extern "C" {
 }
 
 #include "base/logging.h"
+#include "base/stl_util.h"
 #include "server/command_registry.h"
 #include "server/conn_context.h"
 #include "server/engine_shard_set.h"
@@ -158,15 +159,22 @@ ResultSetView DiffResultVec(const ResultStringVec& result_vec, ShardId src_shard
 OpResult<SvArray> InterResultVec(const ResultStringVec& result_vec, unsigned required_shard_cnt) {
   absl::flat_hash_map<std::string_view, unsigned> uniques;
 
+  for (const auto& res : result_vec) {
+    if (!res && !base::_in(res.status(), {OpStatus::SKIPPED, OpStatus::KEY_NOTFOUND}))
+      return res.status();
+  }
+
+  for (const auto& res : result_vec) {
+    if (res.status() == OpStatus::KEY_NOTFOUND)
+      return OpStatus::OK;  // empty set.
+  }
+
   bool first = true;
   for (const auto& res : result_vec) {
     if (res.status() == OpStatus::SKIPPED)
-      continue;
-    if (res.status() == OpStatus::KEY_NOTFOUND)
-      return SvArray{};
-    if (!res) {
-      return res.status();
-    }
+        continue;
+
+    DCHECK(res);  // we handled it above.
 
     // I use this awkward 'first' condition instead of table[s]++ deliberately.
     // I do not want to add keys that I know will not stay in the set.
@@ -1042,15 +1050,25 @@ OpResult<StringVec> SetFamily::OpInter(const Transaction* t, EngineShard* es, bo
   // we must copy by value because AsRObj is temporary.
   vector<SetType> sets(keys.size());
 
+  OpStatus status = OpStatus::OK;
+
   for (size_t i = 0; i < keys.size(); ++i) {
     OpResult<PrimeIterator> find_res = es->db_slice().Find(t->db_index(), keys[i], OBJ_SET);
-    if (!find_res)
-      return find_res.status();
+    if (!find_res) {
+      if (status == OpStatus::OK || status == OpStatus::KEY_NOTFOUND ||
+        find_res.status() != OpStatus::KEY_NOTFOUND) {
+        status = find_res.status();
+      }
+      continue;
+    }
     const PrimeValue& pv = find_res.value()->second;
     void* ptr = pv.RObjPtr();
     sets[i] = make_pair(ptr, pv.Encoding());
   }
 
+  if (status != OpStatus::OK)
+    return status;
+
   auto comp = [](const SetType& left, const SetType& right) {
     return SetTypeLen(left) < SetTypeLen(right);
   };
diff --git a/src/server/set_family_test.cc b/src/server/set_family_test.cc
index 953b42e57..60fe631dd 100644
--- a/src/server/set_family_test.cc
+++ b/src/server/set_family_test.cc
@@ -80,6 +80,11 @@ TEST_F(SetFamilyTest, SInter) {
   EXPECT_THAT(resp[0], IntArg(2));
   resp = Run({"smembers", "d"});
   EXPECT_THAT(resp, UnorderedElementsAre("3", "2"));
+
+  Run({"set", "y", ""});
+  resp = Run({"sinter", "x", "y"});
+  ASSERT_EQ(1, GetDebugInfo("IO0").shards_count);
+  EXPECT_THAT(resp, ElementsAre(ErrArg("WRONGTYPE Operation against a key")));
 }
 
 TEST_F(SetFamilyTest, SMove) {