This is an automated email from the ASF dual-hosted git repository.
twice pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new f3d7ef46 Add support of command FCALL_RO (#1791)
f3d7ef46 is described below
commit f3d7ef465f53e8b44ce1ac2d9c1415b935eabc97
Author: Twice <[email protected]>
AuthorDate: Sun Oct 1 20:47:40 2023 +0900
Add support of command FCALL_RO (#1791)
Co-authored-by: hulk <[email protected]>
---
src/commands/cmd_function.cc | 13 ++++++++-----
src/storage/scripting.cc | 28 +++++++++++++++------------
src/storage/scripting.h | 10 ++++++----
tests/gocase/unit/scripting/function_test.go | 16 +++++++++++++++
tests/gocase/unit/scripting/mylib3.lua | 29 ++++++++++++++++++++++++++++
5 files changed, 75 insertions(+), 21 deletions(-)
diff --git a/src/commands/cmd_function.cc b/src/commands/cmd_function.cc
index 0b673269..8c38618e 100644
--- a/src/commands/cmd_function.cc
+++ b/src/commands/cmd_function.cc
@@ -37,7 +37,7 @@ struct CommandFunction : Commander {
}
std::string libname;
- auto s = lua::FunctionLoad(srv, GET_OR_RET(parser.TakeStr()), true,
replace, &libname);
+ auto s = lua::FunctionLoad(conn, GET_OR_RET(parser.TakeStr()), true,
replace, &libname);
if (!s) return s;
*output = SimpleString(libname);
@@ -63,7 +63,7 @@ struct CommandFunction : Commander {
return lua::FunctionListFunc(srv, funcname, output);
} else if (parser.EatEqICase("delete")) {
auto libname = GET_OR_RET(parser.TakeStr());
- if (!lua::FunctionIsLibExist(srv, libname)) {
+ if (!lua::FunctionIsLibExist(conn, libname)) {
return {Status::NotOK, "no such library"};
}
@@ -78,6 +78,7 @@ struct CommandFunction : Commander {
}
};
+template <bool read_only = false>
struct CommandFCall : Commander {
Status Execute(Server *srv, Connection *conn, std::string *output) override {
int64_t numkeys = GET_OR_RET(ParseInt<int64_t>(args_[2], 10));
@@ -87,14 +88,16 @@ struct CommandFCall : Commander {
return {Status::NotOK, "Number of keys can't be negative"};
}
- return lua::FunctionCall(srv, args_[1],
std::vector<std::string>(args_.begin() + 3, args_.begin() + 3 + numkeys),
- std::vector<std::string>(args_.begin() + 3 +
numkeys, args_.end()), output);
+ return lua::FunctionCall(conn, args_[1],
std::vector<std::string>(args_.begin() + 3, args_.begin() + 3 + numkeys),
+ std::vector<std::string>(args_.begin() + 3 +
numkeys, args_.end()), output, read_only);
}
};
CommandKeyRange GetScriptEvalKeyRange(const std::vector<std::string> &args);
REDIS_REGISTER_COMMANDS(MakeCmdAttr<CommandFunction>("function", -2,
"exclusive no-script", 0, 0, 0),
- MakeCmdAttr<CommandFCall>("fcall", -3, "exclusive
write no-script", GetScriptEvalKeyRange));
+ MakeCmdAttr<CommandFCall<>>("fcall", -3, "exclusive
write no-script", GetScriptEvalKeyRange),
+ MakeCmdAttr<CommandFCall<true>>("fcall_ro", -3,
"read-only ro-script no-script",
+
GetScriptEvalKeyRange));
} // namespace redis
diff --git a/src/storage/scripting.cc b/src/storage/scripting.cc
index 08792ac5..5e0fa00e 100644
--- a/src/storage/scripting.cc
+++ b/src/storage/scripting.cc
@@ -272,7 +272,8 @@ int RedisRegisterFunction(lua_State *lua) {
return 0;
}
-Status FunctionLoad(Server *srv, const std::string &script, bool
need_to_store, bool replace, std::string *lib_name) {
+Status FunctionLoad(redis::Connection *conn, const std::string &script, bool
need_to_store, bool replace,
+ std::string *lib_name, bool read_only) {
std::string first_line, lua_code;
if (auto pos = script.find('\n'); pos != std::string::npos) {
first_line = script.substr(0, pos);
@@ -306,10 +307,10 @@ Status FunctionLoad(Server *srv, const std::string
&script, bool need_to_store,
std::any_of(libname.begin(), libname.end(), [](char v) { return
!std::isalnum(v) && v != '_'; })) {
return {Status::NotOK, "Expect a valid library name in the Shebang
statement"};
}
+ auto srv = conn->GetServer();
+ auto lua = read_only ? conn->Owner()->Lua() : srv->Lua();
- auto lua = srv->Lua();
-
- if (FunctionIsLibExist(srv, libname, need_to_store)) {
+ if (FunctionIsLibExist(conn, libname, need_to_store, read_only)) {
if (!replace) {
return {Status::NotOK, "library already exists, please specify REPLACE
to force load"};
}
@@ -344,15 +345,17 @@ Status FunctionLoad(Server *srv, const std::string
&script, bool need_to_store,
return {Status::NotOK, "Error while running new function lib: " + err_msg};
}
- if (!FunctionIsLibExist(srv, libname, false)) {
+ if (!FunctionIsLibExist(conn, libname, false, read_only)) {
return {Status::NotOK, "Please register some function in FUNCTION LOAD"};
}
return need_to_store ? srv->FunctionSetCode(libname, script) : Status::OK();
}
-bool FunctionIsLibExist(Server *srv, const std::string &libname, bool
need_check_storage) {
- auto lua = srv->Lua();
+bool FunctionIsLibExist(redis::Connection *conn, const std::string &libname,
bool need_check_storage, bool read_only) {
+ auto srv = conn->GetServer();
+ auto lua = read_only ? conn->Owner()->Lua() : srv->Lua();
+
lua_getglobal(lua, REDIS_FUNCTION_LIBRARIES);
if (lua_isnil(lua, -1)) {
@@ -376,13 +379,14 @@ bool FunctionIsLibExist(Server *srv, const std::string
&libname, bool need_check
if (!s) return false;
std::string lib_name;
- s = FunctionLoad(srv, code, false, false, &lib_name);
+ s = FunctionLoad(conn, code, false, false, &lib_name, read_only);
return static_cast<bool>(s);
}
-Status FunctionCall(Server *srv, const std::string &name, const
std::vector<std::string> &keys,
- const std::vector<std::string> &argv, std::string *output)
{
- auto lua = srv->Lua();
+Status FunctionCall(redis::Connection *conn, const std::string &name, const
std::vector<std::string> &keys,
+ const std::vector<std::string> &argv, std::string *output,
bool read_only) {
+ auto srv = conn->GetServer();
+ auto lua = read_only ? conn->Owner()->Lua() : srv->Lua();
lua_getglobal(lua, "__redis__err__handler");
@@ -398,7 +402,7 @@ Status FunctionCall(Server *srv, const std::string &name,
const std::vector<std:
s = srv->FunctionGetCode(libname, &libcode);
if (!s) return s;
- s = FunctionLoad(srv, libcode, false, false, &libname);
+ s = FunctionLoad(conn, libcode, false, false, &libname, read_only);
if (!s) return s;
lua_getglobal(lua, (REDIS_LUA_REGISTER_FUNC_PREFIX + name).c_str());
diff --git a/src/storage/scripting.h b/src/storage/scripting.h
index 24447a83..77df640e 100644
--- a/src/storage/scripting.h
+++ b/src/storage/scripting.h
@@ -62,13 +62,15 @@ Status EvalGenericCommand(redis::Connection *conn, const
std::string &body_or_sh
bool ScriptExists(lua_State *lua, const std::string &sha);
-Status FunctionLoad(Server *srv, const std::string &script, bool
need_to_store, bool replace, std::string *lib_name);
-Status FunctionCall(Server *srv, const std::string &name, const
std::vector<std::string> &keys,
- const std::vector<std::string> &argv, std::string *output);
+Status FunctionLoad(redis::Connection *conn, const std::string &script, bool
need_to_store, bool replace,
+ std::string *lib_name, bool read_only = false);
+Status FunctionCall(redis::Connection *conn, const std::string &name, const
std::vector<std::string> &keys,
+ const std::vector<std::string> &argv, std::string *output,
bool read_only = false);
Status FunctionList(Server *srv, const std::string &libname, bool with_code,
std::string *output);
Status FunctionListFunc(Server *srv, const std::string &funcname, std::string
*output);
Status FunctionDelete(Server *srv, const std::string &name);
-bool FunctionIsLibExist(Server *srv, const std::string &libname, bool
need_check_storage = true);
+bool FunctionIsLibExist(redis::Connection *conn, const std::string &libname,
bool need_check_storage = true,
+ bool read_only = false);
const char *RedisProtocolToLuaType(lua_State *lua, const char *reply);
const char *RedisProtocolToLuaTypeInt(lua_State *lua, const char *reply);
diff --git a/tests/gocase/unit/scripting/function_test.go
b/tests/gocase/unit/scripting/function_test.go
index 28c98ccc..98c90ef2 100644
--- a/tests/gocase/unit/scripting/function_test.go
+++ b/tests/gocase/unit/scripting/function_test.go
@@ -35,6 +35,9 @@ var luaMylib1 string
//go:embed mylib2.lua
var luaMylib2 string
+//go:embed mylib3.lua
+var luaMylib3 string
+
func TestFunction(t *testing.T) {
srv := util.StartServer(t, map[string]string{})
defer srv.Close()
@@ -144,4 +147,17 @@ func TestFunction(t *testing.T) {
require.Equal(t, list[7].(string), "mylib1")
require.Equal(t, len(list), 8)
})
+
+ t.Run("FCALL_RO", func(t *testing.T) {
+ require.NoError(t, rdb.Do(ctx, "FUNCTION", "LOAD",
luaMylib3).Err())
+
+ require.NoError(t, rdb.Set(ctx, "x", 1, 0).Err())
+ require.Equal(t, rdb.Do(ctx, "FCALL", "myget", 1, "x").Val(),
"1")
+ require.Equal(t, rdb.Do(ctx, "FCALL_RO", "myget", 1,
"x").Val(), "1")
+
+ require.Equal(t, rdb.Do(ctx, "FCALL", "myset", 1, "x",
2).Val(), "OK")
+ require.Equal(t, rdb.Get(ctx, "x").Val(), "2")
+
+ util.ErrorRegexp(t, rdb.Do(ctx, "FCALL_RO", "myset", 1, "x",
3).Err(), ".*Write commands are not allowed.*")
+ })
}
diff --git a/tests/gocase/unit/scripting/mylib3.lua
b/tests/gocase/unit/scripting/mylib3.lua
new file mode 100644
index 00000000..31f8fc6c
--- /dev/null
+++ b/tests/gocase/unit/scripting/mylib3.lua
@@ -0,0 +1,29 @@
+#!lua name=mylib3
+
+-- Licensed to the Apache Software Foundation (ASF) under one
+-- or more contributor license agreements. See the NOTICE file
+-- distributed with this work for additional information
+-- regarding copyright ownership. The ASF licenses this file
+-- to you under the Apache License, Version 2.0 (the
+-- "License"); you may not use this file except in compliance
+-- with the License. You may obtain a copy of the License at
+--
+-- http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing,
+-- software distributed under the License is distributed on an
+-- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+-- KIND, either express or implied. See the License for the
+-- specific language governing permissions and limitations
+-- under the License.
+
+local function get(keys, args)
+ return redis.call("get", keys[1])
+end
+
+local function set(keys, args)
+ return redis.call("set", keys[1], args[1])
+end
+
+redis.register_function("myget", get)
+redis.register_function("myset", set)
