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.

Sign up for GitHub

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

Jump to bottom

sessionctx/variable: make lc_time_names read only #30084

Merged
merged 5 commits into from
Nov 26, 2021
Merged

sessionctx/variable: make lc_time_names read only #30084

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
77cf828
Set` lc_time_names` variable to ReadOnly true
espresso98 Nov 23, 2021
00257e6
Add new test case and fix gofmt errors
espresso98 Nov 23, 2021
2038d79
Update TestLcTimeNamesReadOnly function
espresso98 Nov 23, 2021
5da98be
Merge branch 'master' into fix-lc_time_names
espresso98 Nov 23, 2021
401b472
Merge branch 'master' into fix-lc_time_names
ti-chi-bot Nov 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion sessionctx/variable/noop.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ var noopSysVars = []*SysVar{
{Scope: ScopeNone, Name: "tmpdir", Value: "/var/tmp/"},
{Scope: ScopeGlobal, Name: "innodb_thread_concurrency", Value: "0"},
{Scope: ScopeGlobal, Name: "innodb_buffer_pool_dump_pct", Value: ""},
{Scope: ScopeGlobal | ScopeSession, Name: "lc_time_names", Value: "en_US"},
{Scope: ScopeGlobal | ScopeSession, Name: "lc_time_names", Value: "en_US", ReadOnly: true},
{Scope: ScopeGlobal | ScopeSession, Name: "max_statement_time", Value: ""},
{Scope: ScopeGlobal | ScopeSession, Name: EndMarkersInJSON, Value: Off, Type: TypeBool, IsHintUpdatable: true},
{Scope: ScopeGlobal, Name: AvoidTemporalUpgrade, Value: Off, Type: TypeBool},
Expand Down
14 changes: 11 additions & 3 deletions sessionctx/variable/sysvar_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ func TestSkipInit(t *testing.T) {
require.True(t, sv.SkipInit())
}

// IsNoop is used by the documentation to auto-generate docs for real sysvars.
// TestIsNoop is used by the documentation to auto-generate docs for real sysvars.
func TestIsNoop(t *testing.T) {
sv := GetSysVar(TiDBMultiStatementMode)
require.False(t, sv.IsNoop)
Expand Down Expand Up @@ -639,7 +639,7 @@ func TestInstanceScopedVars(t *testing.T) {
require.Equal(t, vars.TxnScope.GetVarValue(), val)
}

// Test that sysvars defaults are logically valid. i.e.
// TestDefaultValuesAreSettable that sysvars defaults are logically valid. i.e.
// the default itself must validate without error provided the scope and read-only is correct.
// The default values should also be normalized for consistency.
func TestDefaultValuesAreSettable(t *testing.T) {
Expand All @@ -660,7 +660,7 @@ func TestDefaultValuesAreSettable(t *testing.T) {
}
}

// This tests that sysvars are logically correct with getter and setter functions.
// TestSettersandGetters tests that sysvars are logically correct with getter and setter functions.
// i.e. it doesn't make sense to have a SetSession function on a variable that is only globally scoped.
func TestSettersandGetters(t *testing.T) {
for _, sv := range GetSysVars() {
Expand Down Expand Up @@ -782,6 +782,14 @@ func TestIdentity(t *testing.T) {
require.Equal(t, val, "21")
}

func TestLcTimeNamesReadOnly(t *testing.T) {
sv := GetSysVar("lc_time_names")
vars := NewSessionVars()
vars.GlobalVarsAccessor = NewMockGlobalAccessor4Tests()
_, err := sv.Validate(vars, "newvalue", ScopeGlobal)
require.Error(t, err)
}

func TestDDLWorkers(t *testing.T) {
svWorkerCount, svBatchSize := GetSysVar(TiDBDDLReorgWorkerCount), GetSysVar(TiDBDDLReorgBatchSize)
vars := NewSessionVars()
Expand Down