Fix EXEC / DISCARD without MULTI wrongly reset watch (apache#1562)

The old code called ResetWatchedKeys before the error, and then
ResetWatchedKeys reset the watched_keys_modified flag which causes
the following inconsistencies (with Redis).

kvrocks output:
```
127.0.0.1:6666> watch a
OK
127.0.0.1:6666> watch b     ->>> other client touch a
OK
127.0.0.1:6666> exec
(error) ERR EXEC without MULTI
127.0.0.1:6666> multi
OK
127.0.0.1:6666(TX)> ping
QUEUED
127.0.0.1:6666(TX)> exec
1) PONG
```

redis output:
```
127.0.0.1:6379> watch a
OK
127.0.0.1:6379> watch b     ->>> other client touch a
OK
127.0.0.1:6379> exec
(error) ERR EXEC without MULTI
127.0.0.1:6379> multi
OK
127.0.0.1:6379(TX)> ping
QUEUED
127.0.0.1:6379(TX)> exec
(nil)
````

Introduced in apache#1279

src/commands/cmd_txn.cc

@@ -45,14 +45,14 @@ class CommandMulti : public Commander {
 class CommandDiscard : public Commander {
  public:
   Status Execute(Server *svr, Connection *conn, std::string *output) override {
-    auto reset_watch = MakeScopeExit([svr, conn] { svr->ResetWatchedKeys(conn); });
-
     if (!conn->IsFlagEnabled(Connection::kMultiExec)) {
       *output = redis::Error("ERR DISCARD without MULTI");
       return Status::OK();
     }
 
+    auto reset_watch = MakeScopeExit([svr, conn] { svr->ResetWatchedKeys(conn); });
     conn->ResetMultiExec();
+
     *output = redis::SimpleString("OK");
 
     return Status::OK();
@@ -62,13 +62,12 @@ class CommandDiscard : public Commander {
 class CommandExec : public Commander {
  public:
   Status Execute(Server *svr, Connection *conn, std::string *output) override {
-    auto reset_watch = MakeScopeExit([svr, conn] { svr->ResetWatchedKeys(conn); });
-
     if (!conn->IsFlagEnabled(Connection::kMultiExec)) {
       *output = redis::Error("ERR EXEC without MULTI");
       return Status::OK();
     }
 
+    auto reset_watch = MakeScopeExit([svr, conn] { svr->ResetWatchedKeys(conn); });
     auto reset_multiexec = MakeScopeExit([conn] { conn->ResetMultiExec(); });
 
     if (conn->IsMultiError()) {

tests/gocase/unit/multi/multi_test.go

@@ -177,6 +177,22 @@ func TestMulti(t *testing.T) {
 		require.NoError(t, rdb.Do(ctx, "EXEC").Err())
 	})
 
+	t.Run("EXEC without MULTI is not allowed", func(t *testing.T) {
+		require.EqualError(t, rdb.Do(ctx, "EXEC").Err(), "ERR EXEC without MULTI")
+	})
+
+	t.Run("EXEC without MULTI should not reset watch", func(t *testing.T) {
+		rdb2 := srv.NewClient()
+		defer func() { require.NoError(t, rdb2.Close()) }()
+
+		require.NoError(t, rdb.Do(ctx, "WATCH", "x").Err())
+		require.NoError(t, rdb2.Do(ctx, "SET", "x", "xxx").Err())
+		require.EqualError(t, rdb.Do(ctx, "EXEC").Err(), "ERR EXEC without MULTI")
+		require.NoError(t, rdb.Do(ctx, "MULTI").Err())
+		require.NoError(t, rdb.Do(ctx, "PING").Err())
+		require.Equal(t, rdb.Do(ctx, "EXEC").Val(), nil)
+	})
+
 	t.Run("EXEC works on WATCHed key not modified", func(t *testing.T) {
 		require.NoError(t, rdb.Do(ctx, "WATCH", "x", "y", "z").Err())
 		require.NoError(t, rdb.Do(ctx, "WATCH", "k").Err())
@@ -268,6 +284,22 @@ func TestMulti(t *testing.T) {
 		require.Equal(t, rdb.Do(ctx, "EXEC").Val(), nil)
 	})
 
+	t.Run("DISCARD without MULTI is not allowed", func(t *testing.T) {
+		require.EqualError(t, rdb.Do(ctx, "DISCARD").Err(), "ERR DISCARD without MULTI")
+	})
+
+	t.Run("DISCARD without MULTI should not reset watch", func(t *testing.T) {
+		rdb2 := srv.NewClient()
+		defer func() { require.NoError(t, rdb2.Close()) }()
+
+		require.NoError(t, rdb.Do(ctx, "WATCH", "x").Err())
+		require.NoError(t, rdb2.Do(ctx, "SET", "x", "xxx").Err())
+		require.EqualError(t, rdb.Do(ctx, "DISCARD").Err(), "ERR DISCARD without MULTI")
+		require.NoError(t, rdb.Do(ctx, "MULTI").Err())
+		require.NoError(t, rdb.Do(ctx, "PING").Err())
+		require.Equal(t, rdb.Do(ctx, "EXEC").Val(), nil)
+	})
+
 	t.Run("DISCARD should clear the WATCH dirty flag on the client", func(t *testing.T) {
 		require.NoError(t, rdb.Set(ctx, "x", 30, 0).Err())
 		require.NoError(t, rdb.Do(ctx, "WATCH", "x").Err())