From e46f2b5384ea68190695c13eac382c41abc6cc2a Mon Sep 17 00:00:00 2001
From: Roman Gershman <romange@gmail.com>
Date: Tue, 29 Mar 2022 17:27:48 +0300
Subject: [PATCH] Support returning arrays from lua scripts

---
 src/core/interpreter.cc      | 11 +++++++++--
 src/server/dragonfly_test.cc | 13 +++++++++----
 src/server/main_service.cc   |  4 ++--
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/src/core/interpreter.cc b/src/core/interpreter.cc
index 0577b5fac..a53a49eef 100644
--- a/src/core/interpreter.cc
+++ b/src/core/interpreter.cc
@@ -397,6 +397,8 @@ bool Interpreter::Exists(string_view sha) const {
 }
 
 auto Interpreter::RunFunction(string_view sha, std::string* error) -> RunResult {
+  DVLOG(1) << "RunFunction " << sha << " " << lua_gettop(lua_);
+
   DCHECK_EQ(40u, sha.size());
 
   lua_getglobal(lua_, "__redis__err__handler");
@@ -413,6 +415,8 @@ auto Interpreter::RunFunction(string_view sha, std::string* error) -> RunResult
     return NOT_EXISTS;
   }
 
+  // At this point lua stack has 2 globals.
+
   /* We have zero arguments and expect
    * a single return value. */
   int err = lua_pcall(lua_, 0, 1, -2);
@@ -438,7 +442,11 @@ bool Interpreter::IsResultSafe() const {
     return true;
 
   bool res = IsTableSafe();
-  lua_settop(lua_, top);
+
+  // Stack can contain intermediate unwindings that were not clean up.
+  DCHECK_GE(lua_gettop(lua_), top);
+  lua_settop(lua_, top);  // restore to the original setting.
+
   return res;
 }
 
@@ -506,7 +514,6 @@ bool Interpreter::IsTableSafe() const {
 
     lua_pop(lua_, 1);
   };
-  DCHECK_EQ(1, lua_gettop(lua_));
 
   return true;
 }
diff --git a/src/server/dragonfly_test.cc b/src/server/dragonfly_test.cc
index 0b607dbb9..2d9cbff8b 100644
--- a/src/server/dragonfly_test.cc
+++ b/src/server/dragonfly_test.cc
@@ -248,10 +248,7 @@ TEST_F(DflyEngineTest, FlushDb) {
 }
 
 TEST_F(DflyEngineTest, Eval) {
-  auto resp = Run({"eval", "return 43", "0"});
-  EXPECT_THAT(resp[0], IntArg(43));
-
-  resp = Run({"incrby", "foo", "42"});
+  auto resp = Run({"incrby", "foo", "42"});
   EXPECT_THAT(resp[0], IntArg(42));
 
   resp = Run({"eval", "return redis.call('get', 'foo')", "0"});
@@ -272,6 +269,14 @@ TEST_F(DflyEngineTest, Eval) {
   ASSERT_FALSE(service_->IsShardSetLocked());
 }
 
+TEST_F(DflyEngineTest, EvalResp) {
+  auto resp = Run({"eval", "return 43", "0"});
+  EXPECT_THAT(resp[0], IntArg(43));
+
+  resp = Run({"eval", "return {5, 'foo', 17.5}", "0"});
+  EXPECT_THAT(resp, ElementsAre(IntArg(5), "foo", "17.5"));
+}
+
 TEST_F(DflyEngineTest, EvalSha) {
   auto resp = Run({"script", "load", "return 5"});
   EXPECT_THAT(resp, ElementsAre(ArgType(RespExpr::STRING)));
diff --git a/src/server/main_service.cc b/src/server/main_service.cc
index 9ffc3faef..ea5d5d2de 100644
--- a/src/server/main_service.cc
+++ b/src/server/main_service.cc
@@ -120,11 +120,10 @@ class EvalSerializer : public ObjectExplorer {
   }
 
   void OnArrayStart(unsigned len) final {
-    LOG(FATAL) << "TBD";
+    rb_->StartArray(len);
   }
 
   void OnArrayEnd() final {
-    LOG(FATAL) << "TBD";
   }
 
   void OnNil() final {
@@ -802,6 +801,7 @@ void Service::EvalInternal(const EvalArgs& eval_args, Interpreter* interpreter,
     string resp = absl::StrCat("Error running script (call to ", eval_args.sha, "): ", error);
     return (*cntx)->SendError(resp, facade::kScriptErrType);
   }
+
   CHECK(result == Interpreter::RUN_OK);
 
   EvalSerializer ser{static_cast<RedisReplyBuilder*>(cntx->reply_builder())};