Hi, Yuchen, On Sep 01, Yuchen Pei wrote:
revision-id: 04b99a30a3f (mariadb-11.0.1-103-g04b99a30a3f) parent(s): 4f39ae75606 author: Yuchen Pei committer: Yuchen Pei timestamp: 2023-08-16 11:31:16 +1000 message:
MDEV-15935 Adding global/session var redirect_url
Adding a global/session var `redirect_url' of string type. The initial value is empty. Can be supplied in mysqld with --redirect-url. A valid redirect_url should be of the format
{mysql,mariadb}://host[:port]
where <host> is an arbitrary string not containing colons, and <port> is a number between 0 and 65535 inclusive.
diff --git a/mysql-test/main/redirect.opt b/mysql-test/main/redirect.opt
move this to the sys_vars suite, please
--- /dev/null +++ b/mysql-test/main/redirect.opt @@ -0,0 +1 @@ +--redirect_url=mysql://foobar
ok, as long as this isn't a complete feature, but just one piece of it. but when client support will be implemented (CONC-599) and the server will actually send this to the connecting clients (MDEV-31609), this test will need to be fixed, as foobar is not something a client can be redirected to. Also, it'd be more interesting to try something like --init-connect='set @@redirect_url="mysql://foobar"' instead of the simple --redirect_url=mysql://foobar because init-connect is much more flexible, it can generate the url on the fly, it can send different users to different locations. So it'd make sense to test init-connect approach.
diff --git a/mysql-test/main/redirect.test b/mysql-test/main/redirect.test new file mode 100644 --- /dev/null +++ b/mysql-test/main/redirect.test @@ -0,0 +1,56 @@ +--echo # +--echo # MDEV-15935 Connection Redirection Mechanism in MariaDB Client/Server Protocol +--echo # + +set @old_redirect_url=@@redirect_url; +select @@global.redirect_url; +set global redirect_url=default; +select @@global.redirect_url; +--error ER_WRONG_VALUE_FOR_VAR +set global redirect_url="mariadb.org"; +--error ER_WRONG_VALUE_FOR_VAR +set global redirect_url="https://mariadb.org";
before doing all this fun, please remove redirect_url from the session_track_system_variables, or just set it to the empty string.
diff --git a/sql/sql_class.h b/sql/sql_class.h --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -3585,6 +3587,7 @@ class THD: public THD_count, /* this must be first */ */ LEX_CSTRING connection_name; char default_master_connection_buff[MAX_CONNECTION_NAME+1]; + char redirect_url_buff[MAX_CONNECTION_NAME+1];
This isn't such a good idea (I'm afraid, I missed it when default_master_connection_buff was added). A couple of years ago Monty have spent significant efforts trying to make THD smaller as it has grown to be too large. Now THD keeps growing again. Let's try not to encourage it, if we can help it.
uint8 password; /* 0, 1 or 2 */ uint8 failed_com_change_user; bool slave_thread; diff --git a/sql/sys_vars.inl b/sql/sys_vars.inl --- a/sql/sys_vars.inl +++ b/sql/sys_vars.inl @@ -637,6 +637,129 @@ class Sys_var_charptr_fscs: public Sys_var_charptr ... +class Sys_var_redirect_url: public Sys_var_charptr_base
please, no. new classes are needed only for variables with very special, unlike anything else, behavior. redirect_url is just a session string variable. See the comment at the top of sys_vars.cc But (!) there are no other session string variables yet. So I'd recomment to fix Sys_var_charptr to support session variables. And then create redirect_url out of it. Practically it might need no changes at all, if you put explicit strmake (or strdup/free) into THD::init like you did anyway. After that, I suspect, you might be able to remove Sys_var_charptr_base, Sys_var_sesvartrack, and Sys_var_session_lexstring, because Sys_var_charptr/Sys_var_lexstring will be able to serve all those use cases.
+{ + static const size_t max_length = MAX_CONNECTION_NAME; +public: + Sys_var_redirect_url(const char *name_arg, + const char *comment, + CMD_LINE getopt, + const char *def_val, PolyLock *lock= 0) : + Sys_var_charptr_base(name_arg, comment, + SESSION_VAR(redirect_url), + getopt, def_val, lock, + VARIABLE_NOT_IN_BINLOG, 0, 0, 0) + { + global_var(LEX_CSTRING).length= strlen(def_val); + option.var_type|= GET_STR; + *const_cast<SHOW_TYPE*>(&show_val_type)= SHOW_LEX_STRING; + }
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org