Hi Staale,
Staale.Smedseng@stripped wrote:
> #At file:///home/ss156133/z/b22891/
>
> 2705 staale.smedseng@stripped 2008-09-03
> A fix for Bug#22891 "session level max_allowed_packet can be set
> but is ignored".
>
> This patch makes @@session.max_allowed_packed read-only as suggested
> in the bug report. The user will have to use SET GLOBAL (and reconnect)
> to alter the value of max_allowed_packet. A new error code
> ER_SESSION_COUNTERPART_IS_READONLY is introduced.
>
> The implementation introduces a new class sys_var_max_allowed_packet,
> inherited from sys_var_thd. Tests are modified accordingly.
> modified:
> mysql-test/r/func_compress.result
> mysql-test/r/max_allowed_packet_basic.result
> mysql-test/r/max_allowed_packet_func.result
> mysql-test/r/packet.result
> mysql-test/r/union.result
> mysql-test/r/variables.result
> mysql-test/t/func_compress.test
> mysql-test/t/innodb_bug34300.test
> mysql-test/t/max_allowed_packet_basic.test
> mysql-test/t/max_allowed_packet_func.test
> mysql-test/t/packet.test
> mysql-test/t/union.test
> mysql-test/t/variables.test
> sql/set_var.cc
> sql/set_var.h
> sql/share/errmsg.txt
>
Skipping test case as the implementation needs some justification.
> === modified file 'sql/set_var.cc'
> --- a/sql/set_var.cc 2008-08-08 01:33:43 +0000
> +++ b/sql/set_var.cc 2008-09-03 13:15:00 +0000
> @@ -294,8 +294,16 @@ static sys_var_thd_bool sys_sql_low_prio
> &SV::low_priority_updates,
> fix_low_priority_updates);
> #endif
> -static sys_var_thd_ulong sys_max_allowed_packet(&vars, "max_allowed_packet",
> +/*@ss static sys_var_thd_ulong sys_max_allowed_packet(&vars,
> "max_allowed_packet",
> &SV::max_allowed_packet);
> +static uchar *get_max_allowed_packet(THD *thd)
> +{
> + return (uchar*) &thd->variables.max_allowed_packet;
> +}
> +static sys_var_readonly sys_max_allowed_packet(&vars, "max_allowed_packet",
> OPT_SESSION,
> + SHOW_LONG, get_max_allowed_packet);
> */
> +static sys_var_max_allowed_packet sys_max_allowed_packet(&vars,
> "max_allowed_packet");
> +
I don't see why you need to have the two variables. The check for read
only session value could be implemented in the check method of the
derived class...
> static sys_var_long_ptr sys_max_binlog_cache_size(&vars,
> "max_binlog_cache_size",
> &max_binlog_cache_size);
> static sys_var_long_ptr sys_max_binlog_size(&vars, "max_binlog_size",
> @@ -2835,6 +2843,72 @@ uchar *sys_var_max_user_conn::value_ptr(
> }
>
>
> +bool sys_var_max_allowed_packet::check(THD *thd, set_var *var)
> +{
> + if (var->type != OPT_GLOBAL)
> + {
> + /*
> + The session value of max_allowed_packet is read-only.
> + */
> + my_error(ER_SESSION_COUNTERPART_IS_READONLY, MYF(0), name);
Couldn't this check and error work on a similar way as it is done in
sys_var_thd_ulonglong ? Check only_global..
[..]
>
> === modified file 'sql/share/errmsg.txt'
> --- a/sql/share/errmsg.txt 2008-08-06 14:39:03 +0000
> +++ b/sql/share/errmsg.txt 2008-09-03 13:15:00 +0000
> @@ -6372,3 +6372,5 @@ ER_BACKUP_OBTAIN_NAME_LOCK_FAILED
> eng "Restore failed to obtain the name locks on the tables."
> ER_BACKUP_RELEASE_NAME_LOCK_FAILED
> eng "Restore failed to release the name locks on the tables."
> +ER_SESSION_COUNTERPART_IS_READONLY
> + eng "Session variable %s is a constant. Use SET GLOBAL to assign the global
> value"
>
Do we really need a new error? We already have ER_GLOBAL_VARIABLE,
ER_LOCAL_VARIABLE, ER_INCORRECT_GLOBAL_LOCAL_VAR, etc.
Regards,
-- Davi
