fix(transaction): Don't transactionalize empty EVAL inside EXEC (#3231)

dragonflydb · Jun 30, 2024 · a931177 · a931177
1 parent 90b5ec4
commit a931177
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 1 deletion.
diff --git a/src/server/main_service.cc b/src/server/main_service.cc
@@ -705,7 +705,16 @@ Transaction::MultiMode DeduceExecMode(ExecEvalState state,
   bool transactional = contains_global;
   if (!transactional) {
     for (const auto& scmd : exec_info.body) {
-      transactional |= scmd.Cid()->IsTransactional();
+      // We can only tell if eval is transactional based on they keycount
+      if (absl::StartsWith(scmd.Cid()->name(), "EVAL")) {
+        CmdArgVec arg_vec{};
+        StoredCmd cmd = scmd;
+        cmd.Fill(&arg_vec);
+        auto keys = DetermineKeys(scmd.Cid(), absl::MakeSpan(arg_vec));
+        transactional |= (keys && keys.value().num_args() > 0);
+      } else {
+        transactional |= scmd.Cid()->IsTransactional();
+      }
       contains_global |= scmd.Cid()->opt_mask() & CO::GLOBAL_TRANS;
 
       // We can't run no-key-transactional commands in lock-ahead mode currently,

diff --git a/src/server/multi_test.cc b/src/server/multi_test.cc
@@ -1083,12 +1083,22 @@ TEST_F(MultiEvalTest, ScriptSquashingUknownCmd) {
 }
 
 TEST_F(MultiEvalTest, MultiAndEval) {
+  if (auto mode = absl::GetFlag(FLAGS_multi_exec_mode); mode == Transaction::NON_ATOMIC) {
+    GTEST_SKIP() << "Skipped MultiAndEval test because multi_exec_mode is non atomic";
+    return;
+  }
+
   // We had a bug in borrowing interpreters which caused a crash in this scenario
   Run({"multi"});
   Run({"eval", "return redis.call('set', 'x', 'y1')", "1", "x"});
   Run({"exec"});
 
   Run({"eval", "return redis.call('set', 'x', 'y1')", "1", "x"});
+
+  Run({"multi"});
+  Run({"eval", "return 'OK';", "0"});
+  auto resp = Run({"exec"});
+  EXPECT_EQ(resp, "OK");
 }
 
 }  // namespace dfly