thank you for your suggestions. I followed them, except of the read-only
versus global error message. Please see below.
Marc Alff, 17.11.2008 20:13:
> Ingo Struewing wrote:
>> 2751 Ingo Struewing 2008-10-04
>> Bug#28234 - global/session scope - documentation vs implementation
>> Several system variables did not behave like system
>> variables should do.
>> When trying to SET them or use them in SELECT, they were
>> reported as
>> "unknown system variable". But they appeared in SHOW VARIABLES.
>> This has been fixed by removing the "fixed_vars" array of
>> and integrating the variables into the normal system variables
>> All of these variables do now behave as read-only global-only
>> variables. Trying to SET them tells they are read-only, trying to
>> SELECT the session value tells they are global only. Selecting the
>> global value works. It delivers the same value as SHOW VARIABLES.
>> +SET @@session.log_slave_updates= true;
>> +ERROR HY000: Variable 'log_slave_updates' is a read only variable
> I don't agree here.
> This error message is confusing, since by saying that the server
> implicitly agreed that a variable named "@@session.log_slave_updates"
> does exist, which is not the case.
> I would much rather prefer the following error instead:
> ERROR HY000: Variable 'log_slave_updates' is a GLOBAL variable
> I did not look at the code to fix this, I assume it's only a matter of
> changing the order of tests.
Yes, it is trivial to change the order of tests in set_var::check(). But
I don't want to do it:
1. The full message for the global variable is: "Variable ... is a
GLOBAL variable and should be set with SET GLOBAL". The user would then
try to set it globally and receive the "read only" message. I think this
is confusing too.
2. Changing the test order would not only require to update a couple of
test cases, but might also affect user's scripts. It is kind of a
behavioral change. It is too late to do it in 5.1.
>> @@ -1762,7 +1835,7 @@ Item *sys_var::item(THD *thd, enum_var_t
>> value= *(uint*) value_ptr(thd, var_type, base);
>> - return new Item_uint((ulonglong) value);
>> + DBUG_RETURN(new Item_uint((ulonglong) value));
> Thanks for adding DBUG code here, but please don't do that.
> result= new Item_uint((ulonglong) value)
> It makes debugging step by step much easier, for the same binary anyway.
I agree, but have to note that my proposal was based on normal MySQL style.
Anyway, I found a way to improve it even more. Every branch of the
switch had its own return. I changed them all to end with result= and
return the result after the switch.
> Overall comments:
> With that, support for SHOW_FUNC for sys_var_const is not needed
> anymore, and support for SHOW_FUNC should in fact never be needed,
> otherwise the system variable won't really be a "constant" one.
> This should simplify greatly the implementation of:
> - sys_var::check
You probably mean sys_var::item(), where I don't need to add SHOW_FUNC?
The new patch: http://lists.mysql.com/commits/59410
Ingo Strüwing, Database Group
Sun Microsystems GmbH, Sonnenallee 1, D-85551 Kirchheim-Heimstetten
Geschäftsführer: Thomas Schröder, Wolfgang Engels, Dr. Roland Bömer
Vorsitzender des Aufsichtsrates: Martin Häring HRB München 161028