Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for tidb_enable_noop_sysvars #8895

Closed
morgo opened this issue Dec 31, 2018 · 1 comment · Fixed by #35496
Closed

Support for tidb_enable_noop_sysvars #8895

morgo opened this issue Dec 31, 2018 · 1 comment · Fixed by #35496
Assignees
Labels
proposal type/enhancement The issue or PR belongs to an enhancement.

Comments

@morgo
Copy link
Contributor

morgo commented Dec 31, 2018

Feature Request

Is your feature request related to a problem? Please describe:

Currently, it is hard to know if setting a variable in TiDB has an effect. There are a number of sysvars that are included for compatibility with MySQL, and their inclusion helps compatibility but hurts usability (I have caught myself a couple of times, only to read the manual or the source code and see something is not implemented).

Describe the feature you'd like:

Similar to tidb_enable_noop_functions, there can be tidb_enable_noop_sysvars. When enabled (default) the behavior is the same as currently. When disabled:

  • SHOW [GLOBAL] VARIABLES omits the variable
  • Setting the variable returns a warning
  • SELECT @@variable returns a warning

Because most ORMs that depend on noops will SELECT @@ the variable (for performance reasons they should be doing this), it will actually still work in most instances. But for compatibility, at least for now the default is still enabled.

I have a cleanup proposal here: https://docs.google.com/document/d/1i181NttnnQm-B7iq1GCfEFlFccZmZCMBXziqybLoTu8/edit?usp=sharing

Describe alternatives you've considered:

It could error instead of warn, but warn seems sufficient.

Teachability, Documentation, Adoption, Migration Strategy:

The name being similar to noop_funcs makes it very approachable. Unfortunately adoption will be difficult since the default still shows noops, but this is a starting point. It might be hard to completely remove noops because the compatibility testing requirements are so difficult.

@morgo morgo added type/enhancement The issue or PR belongs to an enhancement. proposal labels Dec 31, 2018
@morgo morgo self-assigned this Jan 18, 2019
@morgo
Copy link
Contributor Author

morgo commented Feb 10, 2019

I discussed this with the TiDB team following devcon this year, and we proposed a slightly different behavior: off by default, but produce a warning when updating a sysvar that is unsupported (irrespective of tidb_mysql_compatible_variable_info value). I will update the proposal to incorporate shortly.

@morgo morgo changed the title Proposal to cleanup sysvars Proposal to cleanup noop sysvars Feb 7, 2022
@morgo morgo changed the title Proposal to cleanup noop sysvars Support for tidb_enable_noop_sysvars Jun 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant