Spider variables that double as table params overriding mechanism is buggy

Spider has variables and table parameters, and some variables double as table parameters. Examples include spider_same_server_link, spider_casual_read and spider_read_only_mode. These are the variables we consider in this ticket.

All such variables are of the types of int, longlong or double.

For these variables, the existing overriding mechanism does the following: 1) if the var has value -1, defer to the table parameter value; and 2) if the var value is not -1, it overrides the table param value. The table param value is in turn set to default in spider_set_connect_info_default() if not specified. MDEV-27169 marks 1) as deprecated.

MDEV-27169 also changed the default value of the vars from -1 to the actual default values used in spider_set_connect_info_default(). As a consequence, table params are ignored unless the user sets the variable to -1 (say with the SET command) which would trigger a deprecation warning.

It is caused by functions like the following, which says that if the var is -1, then we use the table param, otherwise we use the var value.

int spider_param_read_only_mode(

  THD *thd,

  int read_only_mode

) {

  DBUG_ENTER("spider_param_read_only_mode");

  DBUG_RETURN(THDVAR(thd, read_only_mode) == -1 ? // default value is 0

    read_only_mode : THDVAR(thd, read_only_mode));

}

Assuming we still want to table params to take effect, this needs fixing. Whether we need to do a rollback or a fixup depends on the answer to the following question: do we want table params to override vars if both are set by the user? If not, then a rollback is sufficient. Otherwise, the existing logic (as shown in the code snippet above) would still be broken.

If we want to do a fixup, one idea would be to remove the assignment of default values in spider_set_connect_info_default() to these table params, so that they remain -1 which was assigned at the beginning of spider_parse_connect_info(). Then in functions like spider_param_read_only_mode(), replace the ternary expression with:

  DBUG_RETURN(read_only_mode == -1 ?

    THDVAR(thd, read_only_mode) : read_only_mode);

This way:

• the var is set to actual default value if the user does not set it
• if the user sets the table param but not the var, the table param would be used
• if the user sets the var but not the table param, the var would be used
• if the user sets both, the table param would be used

If we do a rollback, the fixversion should be 10.9+ because that is where MDEV-27169 is in. If we do a fixup, then we need to decide whether to fix it for all versions, or just 10.9+

[update on 2023-07-20]: we did a roll-up for all versions, and as a side-effect, the patches for version 10.4-6 contain changes to the effect of MDEV-27169.