optimizer_switch is read only

[Alkaagr81]

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

SET @@optimizer_switch="derived_merge=off";

2. What did you expect to see? (Required)

mysql> SET @@optimizer_switch="derived_merge=off";
Query OK, 0 rows affected (0.01 sec)

3. What did you see instead (Required)

mysql> SET @@optimizer_switch="derived_merge=off";
ERROR 1238 (HY000): Variable 'optimizer_switch' is a read only variable

4. What is your TiDB version? (Required)

| Release Version: v5.2.1
Edition: Community
Git Commit Hash: cd8fb24c5f7ebd9d479ed228bb41848bd5e97445
Git Branch: heads/refs/tags/v5.2.1
UTC Build Time: 2021-09-08 02:29:53
GoVersion: go1.16.4
Race Enabled: false
TiKV Min Version: v3.0.0-60965b006877ca7234adaced7890d7b029ed1306
Check Table Before Drop: false |

[morgo]

For context: this came up while we were looking at tests.

It is a noop sysvar in TiDB, but different from other noops it's read-only. This might be the correct behavior (given it is a noop), but I suggested we should create a bug report so we can discuss and have something to point to for future discussion.

If we know apps that are setting optimizer_switch (I think some custom ones do, but it's not that common), then we might want to change this to read/write, or read/write with a warning.

@dveeden @mjonss @ThomasYYYY what do you think?

[dveeden]

In MySQL the optimizer_switch options get get parsed and then they are validated. Error 1231(ER_WRONG_VALUE_FOR_VAR) is returned for unknown/invalid key/value pairs.

We could:

• Accept any value.
• Accept known values from MySQL 5.7 and MySQL 8.0, basically implement optimizer_switch, but noop the individual settings.
• Return error 1231 in all cases.

I think always returning an error is the most correct, but accepting either all or known values would increase compatibility.

The docs say that hash_join is 8.0.18 only and has no effect on 8.0.19 or later. This is similar to the second option I suggested.

 MySQL  127.0.0.1:18026+ ssl  SQL > SET GLOBAL optimizer_switch='index_merge=on';
Query OK, 0 rows affected (0.0003 sec)
 MySQL  127.0.0.1:18026+ ssl  SQL > SET GLOBAL optimizer_switch='foo_bar=on';
ERROR: 1231: Variable 'optimizer_switch' can't be set to the value of 'foo_bar=on'
 MySQL  127.0.0.1:18026+ ssl  SQL > SET GLOBAL optimizer_switch='index_merge=on,foo_bar=on';
ERROR: 1231: Variable 'optimizer_switch' can't be set to the value of 'foo_bar=on'
 MySQL  127.0.0.1:18026+ ssl  SQL > SELECT VERSION();
+-----------+
| VERSION() |
+-----------+
| 8.0.26    |
+-----------+
1 row in set (0.0002 sec)

[morgo]

I actually prefer read_only to returning err 1231. The reason being is that the sysvar framework tries to test the default values are permitted for sysvars, provided they are not read-only. If we make it read/write but prohibit any values, it would need an exception to this because the values are not insertable.

[mjonss]

So I assume it is statically set and not even changeable through a configuration file?

@@optimizer_switch: index_merge=on,index_merge_union=on,index_merge_sort_union=on,index_merge_intersection=on,engine_condition_pushdown=on,index_condition_pushdown=on,mrr=on,mrr_cost_based=on,block_nested_loop=on,batched_key_access=off,materialization=on,semijoin=on,loosescan=on,firstmatch=on,subquery_materialization_cost_based=on,use_index_extensions=on

And I also assume that these optimizer_switch values is as good description of the current TiDB optimizer.

If my assumptions are valid, then I agree on keeping it as read_only, since we will not change the optimizer behaviour due to the optimizer_switch anyway. Technically it would be better to return an error and prohibit any other value that what is already set, but I see no real benefit of such change.

[morgo]

So I assume it is statically set and not even changeable through a configuration file?

That's right. It's static, most likely based on the defaults from MySQL 5.6.

[ThomasYYYY]

I think "accept known values from MySQL" offers the best compatibility.
But we can keep it "readonly" until some case shows the real benefit.

[morgo]

OK, I will leave it open for now as is. If a user comes across this issue and has a case where we need to support modifying optimizer_switch, we can reconsider.