diff --git a/pkg/executor/set_test.go b/pkg/executor/set_test.go index 4dc2256cc5af2..447f031e8c4a0 100644 --- a/pkg/executor/set_test.go +++ b/pkg/executor/set_test.go @@ -672,15 +672,11 @@ func TestSetVar(t *testing.T) { tk.MustQuery("select @@tidb_enable_historical_stats").Check(testkit.Rows("0")) // test for tidb_enable_column_tracking - tk.MustQuery("select @@tidb_enable_column_tracking").Check(testkit.Rows("0")) - tk.MustExec("set global tidb_enable_column_tracking = 1") tk.MustQuery("select @@tidb_enable_column_tracking").Check(testkit.Rows("1")) tk.MustExec("set global tidb_enable_column_tracking = 0") - tk.MustQuery("select @@tidb_enable_column_tracking").Check(testkit.Rows("0")) - // When set tidb_enable_column_tracking off, we record the time of the setting operation. - tk.MustQuery("select count(1) from mysql.tidb where variable_name = 'tidb_disable_column_tracking_time' and variable_value is not null").Check(testkit.Rows("1")) - tk.MustExec("set global tidb_enable_column_tracking = 1") + tk.MustQuery("show warnings").Check(testkit.Rows("Warning 1681 The 'tidb_enable_column_tracking' variable is deprecated and will be removed in future versions of TiDB. It is always set to 'ON' now.")) tk.MustQuery("select @@tidb_enable_column_tracking").Check(testkit.Rows("1")) + tk.MustQuery("select count(1) from mysql.tidb where variable_name = 'tidb_disable_column_tracking_time' and variable_value is not null").Check(testkit.Rows("0")) require.Error(t, tk.ExecToErr("select @@session.tidb_enable_column_tracking")) require.Error(t, tk.ExecToErr("set tidb_enable_column_tracking = 0")) require.Error(t, tk.ExecToErr("set global tidb_enable_column_tracking = -1")) diff --git a/pkg/planner/core/collect_column_stats_usage.go b/pkg/planner/core/collect_column_stats_usage.go index 72201c64a9188..4ae6dfbc63917 100644 --- a/pkg/planner/core/collect_column_stats_usage.go +++ b/pkg/planner/core/collect_column_stats_usage.go @@ -349,15 +349,14 @@ func (c *columnStatsUsageCollector) collectFromPlan(lp base.LogicalPlan) { // First return value: predicate columns (nil if predicate is false) // Second return value: histogram-needed columns (nil if histNeeded is false) // Third return value: ds.PhysicalTableID from all DataSource (always collected) -func CollectColumnStatsUsage(lp base.LogicalPlan, predicate, histNeeded bool) ( +func CollectColumnStatsUsage(lp base.LogicalPlan, histNeeded bool) ( []model.TableItemID, []model.StatsLoadItem, *intset.FastIntSet, ) { var mode uint64 - if predicate { - mode |= collectPredicateColumns - } + // Always collect predicate columns. + mode |= collectPredicateColumns if histNeeded { mode |= collectHistNeededColumns } @@ -448,9 +447,7 @@ func CollectColumnStatsUsage(lp base.LogicalPlan, predicate, histNeeded bool) ( predicateCols []model.TableItemID histNeededCols []model.StatsLoadItem ) - if predicate { - predicateCols = maps.Keys(collector.predicateCols) - } + predicateCols = maps.Keys(collector.predicateCols) if histNeeded { histNeededCols = itemSet2slice(collector.histNeededCols) } diff --git a/pkg/planner/core/collect_column_stats_usage_test.go b/pkg/planner/core/collect_column_stats_usage_test.go index a7ab0bd1b92a4..72e976828fc14 100644 --- a/pkg/planner/core/collect_column_stats_usage_test.go +++ b/pkg/planner/core/collect_column_stats_usage_test.go @@ -79,7 +79,7 @@ func getStatsLoadItem(t *testing.T, is infoschema.InfoSchema, item model.StatsLo func checkColumnStatsUsageForPredicates(t *testing.T, is infoschema.InfoSchema, lp base.LogicalPlan, expected []string, comment string) { var tblColIDs []model.TableItemID - tblColIDs, _, _ = CollectColumnStatsUsage(lp, true, false) + tblColIDs, _, _ = CollectColumnStatsUsage(lp, false) cols := make([]string, 0, len(tblColIDs)) for _, tblColID := range tblColIDs { col := getColumnName(t, is, tblColID, comment) @@ -91,7 +91,7 @@ func checkColumnStatsUsageForPredicates(t *testing.T, is infoschema.InfoSchema, func checkColumnStatsUsageForStatsLoad(t *testing.T, is infoschema.InfoSchema, lp base.LogicalPlan, expected []string, comment string) { var loadItems []model.StatsLoadItem - _, loadItems, _ = CollectColumnStatsUsage(lp, false, true) + _, loadItems, _ = CollectColumnStatsUsage(lp, true) cols := make([]string, 0, len(loadItems)) for _, item := range loadItems { col := getStatsLoadItem(t, is, item, comment) diff --git a/pkg/planner/core/rule_collect_plan_stats.go b/pkg/planner/core/rule_collect_plan_stats.go index 7288d97f6397c..2983390083932 100644 --- a/pkg/planner/core/rule_collect_plan_stats.go +++ b/pkg/planner/core/rule_collect_plan_stats.go @@ -38,10 +38,9 @@ func (collectPredicateColumnsPoint) optimize(_ context.Context, plan base.Logica if plan.SCtx().GetSessionVars().InRestrictedSQL { return plan, planChanged, nil } - predicateNeeded := variable.EnableColumnTracking.Load() syncWait := plan.SCtx().GetSessionVars().StatsLoadSyncWait.Load() histNeeded := syncWait > 0 - predicateColumns, histNeededColumns, visitedPhysTblIDs := CollectColumnStatsUsage(plan, predicateNeeded, histNeeded) + predicateColumns, histNeededColumns, visitedPhysTblIDs := CollectColumnStatsUsage(plan, histNeeded) if len(predicateColumns) > 0 { plan.SCtx().UpdateColStatsUsage(predicateColumns) } diff --git a/pkg/sessionctx/variable/sysvar.go b/pkg/sessionctx/variable/sysvar.go index 29f92c264aa1c..1f15cdc67a8a0 100644 --- a/pkg/sessionctx/variable/sysvar.go +++ b/pkg/sessionctx/variable/sysvar.go @@ -995,13 +995,6 @@ var defaultSysVars = []*SysVar{ GetGlobal: func(_ context.Context, s *SessionVars) (string, error) { return BoolToOnOff(PersistAnalyzeOptions.Load()), nil }, - Validation: func(vars *SessionVars, normalizedValue string, originalValue string, scope ScopeFlag) (string, error) { - persist := TiDBOptOn(normalizedValue) - if !persist && EnableColumnTracking.Load() { - return "", errors.Errorf("tidb_persist_analyze_options option cannot be set to OFF when tidb_enable_column_tracking is ON, as this will result in the loss of column tracking information") - } - return normalizedValue, nil - }, SetGlobal: func(_ context.Context, s *SessionVars, val string) error { persist := TiDBOptOn(val) PersistAnalyzeOptions.Store(persist) @@ -1152,31 +1145,17 @@ var defaultSysVars = []*SysVar{ }, { Scope: ScopeGlobal, Name: TiDBEnableColumnTracking, - Value: BoolToOnOff(DefTiDBEnableColumnTracking), + Value: BoolToOnOff(true), Type: TypeBool, GetGlobal: func(_ context.Context, s *SessionVars) (string, error) { - return BoolToOnOff(EnableColumnTracking.Load()), nil + return BoolToOnOff(true), nil }, Validation: func(vars *SessionVars, normalizedValue string, originalValue string, scope ScopeFlag) (string, error) { - enabled := TiDBOptOn(normalizedValue) - persist := PersistAnalyzeOptions.Load() - if enabled && !persist { - return "", errors.Errorf("tidb_enable_column_tracking option cannot be set to ON when tidb_persist_analyze_options is set to OFF, as this will prevent the preservation of column tracking information") - } + // This variable is deprecated and will be removed in the future. + vars.StmtCtx.AppendWarning(ErrWarnDeprecatedSyntaxSimpleMsg.FastGen("The 'tidb_enable_column_tracking' variable is deprecated and will be removed in future versions of TiDB. It is always set to 'ON' now.")) return normalizedValue, nil }, SetGlobal: func(_ context.Context, s *SessionVars, val string) error { - enabled := TiDBOptOn(val) - // If this is a user initiated statement, - // we log that column tracking is disabled. - if s.StmtCtx.StmtType == "Set" && !enabled { - // Set the location to UTC to avoid time zone interference. - disableTime := time.Now().UTC().Format(types.UTCTimeFormat) - if err := setTiDBTableValue(s, TiDBDisableColumnTrackingTime, disableTime, "Record the last time tidb_enable_column_tracking is set off"); err != nil { - return err - } - } - EnableColumnTracking.Store(enabled) return nil }}, {Scope: ScopeGlobal, Name: RequireSecureTransport, Value: BoolToOnOff(DefRequireSecureTransport), Type: TypeBool, diff --git a/pkg/sessionctx/variable/sysvar_test.go b/pkg/sessionctx/variable/sysvar_test.go index 879c1e8ba6fc0..350375d595e6b 100644 --- a/pkg/sessionctx/variable/sysvar_test.go +++ b/pkg/sessionctx/variable/sysvar_test.go @@ -1631,69 +1631,3 @@ func TestTiDBLowResTSOUpdateInterval(t *testing.T) { require.NoError(t, err) require.Equal(t, "1000", val) } - -func TestSetEnableColumnTrackingAndPersistAnalyzeOptions(t *testing.T) { - vars := NewSessionVars(nil) - mock := NewMockGlobalAccessor4Tests() - mock.SessionVars = vars - vars.GlobalVarsAccessor = mock - - // Test EnableColumnTracking - val, err := mock.GetGlobalSysVar(TiDBEnableColumnTracking) - require.NoError(t, err) - require.Equal(t, Off, val) - err = mock.SetGlobalSysVar(context.Background(), TiDBEnableColumnTracking, On) - require.NoError(t, err) - val, err = mock.GetGlobalSysVar(TiDBEnableColumnTracking) - require.NoError(t, err) - require.Equal(t, On, val) - // Reset back. - err = mock.SetGlobalSysVar(context.Background(), TiDBEnableColumnTracking, Off) - require.NoError(t, err) - - // Test PersistAnalyzeOptions - val, err = mock.GetGlobalSysVar(TiDBPersistAnalyzeOptions) - require.NoError(t, err) - require.Equal(t, On, val) - err = mock.SetGlobalSysVar(context.Background(), TiDBPersistAnalyzeOptions, Off) - require.NoError(t, err) - val, err = mock.GetGlobalSysVar(TiDBPersistAnalyzeOptions) - require.NoError(t, err) - require.Equal(t, Off, val) - // Reset back - err = mock.SetGlobalSysVar(context.Background(), TiDBPersistAnalyzeOptions, On) - require.NoError(t, err) - - // Set EnableColumnTracking to true when PersistAnalyzeOptions is false - // Set to false first. - err = mock.SetGlobalSysVar(context.Background(), TiDBEnableColumnTracking, Off) - require.NoError(t, err) - err = mock.SetGlobalSysVar(context.Background(), TiDBPersistAnalyzeOptions, Off) - require.NoError(t, err) - val, err = mock.GetGlobalSysVar(TiDBPersistAnalyzeOptions) - require.NoError(t, err) - require.Equal(t, Off, val) - err = mock.SetGlobalSysVar(context.Background(), TiDBEnableColumnTracking, On) - require.Error(t, err, "enable column tracking requires to persist analyze options") - val, err = mock.GetGlobalSysVar(TiDBEnableColumnTracking) - require.NoError(t, err) - require.Equal(t, Off, val) - - // Set PersistAnalyzeOptions to false when EnableColumnTracking is true - // Set to true first. - err = mock.SetGlobalSysVar(context.Background(), TiDBPersistAnalyzeOptions, On) - require.NoError(t, err) - val, err = mock.GetGlobalSysVar(TiDBPersistAnalyzeOptions) - require.NoError(t, err) - require.Equal(t, On, val) - err = mock.SetGlobalSysVar(context.Background(), TiDBEnableColumnTracking, On) - require.NoError(t, err) - val, err = mock.GetGlobalSysVar(TiDBEnableColumnTracking) - require.NoError(t, err) - require.Equal(t, On, val) - err = mock.SetGlobalSysVar(context.Background(), TiDBPersistAnalyzeOptions, Off) - require.Error(t, err, "persist analyze options requires to enable column tracking") - val, err = mock.GetGlobalSysVar(TiDBPersistAnalyzeOptions) - require.NoError(t, err) - require.Equal(t, On, val) -} diff --git a/pkg/sessionctx/variable/tidb_vars.go b/pkg/sessionctx/variable/tidb_vars.go index 6f340fd874810..d71675d74e667 100644 --- a/pkg/sessionctx/variable/tidb_vars.go +++ b/pkg/sessionctx/variable/tidb_vars.go @@ -975,10 +975,12 @@ const ( // TiDBPersistAnalyzeOptions persists analyze options for later analyze and auto-analyze TiDBPersistAnalyzeOptions = "tidb_persist_analyze_options" // TiDBEnableColumnTracking enables collecting predicate columns. + // DEPRECATED: This variable is deprecated, please do not use this variable. TiDBEnableColumnTracking = "tidb_enable_column_tracking" // TiDBDisableColumnTrackingTime records the last time TiDBEnableColumnTracking is set off. // It is used to invalidate the collected predicate columns after turning off TiDBEnableColumnTracking, which avoids physical deletion. // It doesn't have cache in memory, and we directly get/set the variable value from/to mysql.tidb. + // DEPRECATED: This variable is deprecated, please do not use this variable. TiDBDisableColumnTrackingTime = "tidb_disable_column_tracking_time" // TiDBStatsLoadPseudoTimeout indicates whether to fallback to pseudo stats after load timeout. TiDBStatsLoadPseudoTimeout = "tidb_stats_load_pseudo_timeout" @@ -1342,7 +1344,6 @@ const ( DefEnableLegacyInstanceScope = true DefTiDBTableCacheLease = 3 // 3s DefTiDBPersistAnalyzeOptions = true - DefTiDBEnableColumnTracking = false DefTiDBStatsLoadSyncWait = 100 DefTiDBStatsLoadPseudoTimeout = true DefSysdateIsNow = false @@ -1527,7 +1528,6 @@ var ( VarTiDBSuperReadOnly = atomic.NewBool(DefTiDBSuperReadOnly) PersistAnalyzeOptions = atomic.NewBool(DefTiDBPersistAnalyzeOptions) TableCacheLease = atomic.NewInt64(DefTiDBTableCacheLease) - EnableColumnTracking = atomic.NewBool(DefTiDBEnableColumnTracking) StatsLoadSyncWait = atomic.NewInt64(DefTiDBStatsLoadSyncWait) StatsLoadPseudoTimeout = atomic.NewBool(DefTiDBStatsLoadPseudoTimeout) MemQuotaBindingCache = atomic.NewInt64(DefTiDBMemQuotaBindingCache) diff --git a/pkg/statistics/handle/updatetest/update_test.go b/pkg/statistics/handle/updatetest/update_test.go index 35a163a007e43..2bdb484726bd8 100644 --- a/pkg/statistics/handle/updatetest/update_test.go +++ b/pkg/statistics/handle/updatetest/update_test.go @@ -1026,7 +1026,7 @@ func TestCollectPredicateColumnsFromExecute(t *testing.T) { } } -func TestEnableAndDisableColumnTracking(t *testing.T) { +func TestColumnTracking(t *testing.T) { store, dom := testkit.CreateMockStoreAndDomain(t) tk := testkit.NewTestKit(t, store) h := dom.StatsHandle() @@ -1034,40 +1034,18 @@ func TestEnableAndDisableColumnTracking(t *testing.T) { tk.MustExec("drop table if exists t") tk.MustExec("create table t (a int, b int, c int)") - originalVal := tk.MustQuery("select @@tidb_enable_column_tracking").Rows()[0][0].(string) - defer func() { - tk.MustExec(fmt.Sprintf("set global tidb_enable_column_tracking = %v", originalVal)) - }() - - tk.MustExec("set global tidb_enable_column_tracking = 1") tk.MustExec("select * from t where b > 1") require.NoError(t, h.DumpColStatsUsageToKV()) rows := tk.MustQuery("show column_stats_usage where db_name = 'test' and table_name = 't' and last_used_at is not null").Rows() require.Len(t, rows, 1) require.Equal(t, "b", rows[0][3]) - tk.MustExec("set global tidb_enable_column_tracking = 0") - // After tidb_enable_column_tracking is set to 0, the predicate columns collected before are invalidated. - tk.MustQuery("show column_stats_usage where db_name = 'test' and table_name = 't' and last_used_at is not null").Check(testkit.Rows()) - - // Sleep for 1.5s to let `last_used_at` be larger than `tidb_disable_tracking_time`. - time.Sleep(1500 * time.Millisecond) - tk.MustExec("select * from t where a > 1") - require.NoError(t, h.DumpColStatsUsageToKV()) - // We don't collect predicate columns when tidb_enable_column_tracking = 0 - tk.MustQuery("show column_stats_usage where db_name = 'test' and table_name = 't' and last_used_at is not null").Check(testkit.Rows()) - - tk.MustExec("set global tidb_enable_column_tracking = 1") tk.MustExec("select * from t where b < 1 and c > 1") require.NoError(t, h.DumpColStatsUsageToKV()) rows = tk.MustQuery("show column_stats_usage where db_name = 'test' and table_name = 't' and last_used_at is not null").Sort().Rows() require.Len(t, rows, 2) require.Equal(t, "b", rows[0][3]) require.Equal(t, "c", rows[1][3]) - - // Test invalidating predicate columns again in order to check that tidb_disable_tracking_time can be updated. - tk.MustExec("set global tidb_enable_column_tracking = 0") - tk.MustQuery("show column_stats_usage where db_name = 'test' and table_name = 't' and last_used_at is not null").Check(testkit.Rows()) } func TestStatsLockUnlockForAutoAnalyze(t *testing.T) { diff --git a/pkg/statistics/handle/usage/predicate_column.go b/pkg/statistics/handle/usage/predicate_column.go index 1e45151aff01c..dbc5d23f7c84e 100644 --- a/pkg/statistics/handle/usage/predicate_column.go +++ b/pkg/statistics/handle/usage/predicate_column.go @@ -24,7 +24,6 @@ import ( "github.com/pingcap/tidb/pkg/parser/model" "github.com/pingcap/tidb/pkg/parser/mysql" "github.com/pingcap/tidb/pkg/sessionctx" - "github.com/pingcap/tidb/pkg/sessionctx/variable" "github.com/pingcap/tidb/pkg/statistics" statstypes "github.com/pingcap/tidb/pkg/statistics/handle/types" "github.com/pingcap/tidb/pkg/statistics/handle/usage/indexusage" @@ -82,10 +81,6 @@ func (u *statsUsageImpl) CollectColumnsInExtendedStats(tableID int64) (columnIDs // LoadColumnStatsUsage loads column stats usage information from disk. func LoadColumnStatsUsage(sctx sessionctx.Context, loc *time.Location) (map[model.TableItemID]statstypes.ColStatsTimeInfo, error) { - disableTime, err := getDisableColumnTrackingTime(sctx) - if err != nil { - return nil, errors.Trace(err) - } // Since we use another session from session pool to read mysql.column_stats_usage, which may have different @@time_zone, so we do time zone conversion here. rows, _, err := utilstats.ExecRows(sctx, "SELECT table_id, column_id, CONVERT_TZ(last_used_at, @@TIME_ZONE, '+00:00'), CONVERT_TZ(last_analyzed_at, @@TIME_ZONE, '+00:00') FROM mysql.column_stats_usage") if err != nil { @@ -103,12 +98,8 @@ func LoadColumnStatsUsage(sctx sessionctx.Context, loc *time.Location) (map[mode if err != nil { return nil, errors.Trace(err) } - // If `last_used_at` is before the time when `set global tidb_enable_column_tracking = 0`, we should ignore it because - // `set global tidb_enable_column_tracking = 0` indicates all the predicate columns collected before. - if disableTime == nil || gt.After(*disableTime) { - t := types.NewTime(types.FromGoTime(gt.In(loc)), mysql.TypeTimestamp, types.DefaultFsp) - statsUsage.LastUsedAt = &t - } + t := types.NewTime(types.FromGoTime(gt.In(loc)), mysql.TypeTimestamp, types.DefaultFsp) + statsUsage.LastUsedAt = &t } if !row.IsNull(3) { gt, err := row.GetTime(3).GoTime(time.UTC) @@ -130,11 +121,6 @@ func GetPredicateColumns(sctx sessionctx.Context, tableID int64) ([]int64, error if err != nil { return nil, errors.Trace(err) } - // This time is the time when `set global tidb_enable_column_tracking = 0`. - disableTime, err := getDisableColumnTrackingTime(sctx) - if err != nil { - return nil, errors.Trace(err) - } rows, _, err := utilstats.ExecRows( sctx, "SELECT column_id, CONVERT_TZ(last_used_at, @@TIME_ZONE, '+00:00') FROM mysql.column_stats_usage WHERE table_id = %? AND last_used_at IS NOT NULL", @@ -151,17 +137,7 @@ func GetPredicateColumns(sctx sessionctx.Context, tableID int64) ([]int64, error continue } colID := row.GetInt64(0) - gt, err := row.GetTime(1).GoTime(time.UTC) - if err != nil { - return nil, errors.Trace(err) - } - // If `last_used_at` is before the time when `set global tidb_enable_column_tracking = 0`, we don't regard the column as predicate column because - // `set global tidb_enable_column_tracking = 0` indicates all the predicate columns collected before. - // TODO: Why do we need to do this? If column tracking is already disabled, we should not collect any column usage. - // If this refers to re-enabling column tracking, shouldn't we retain the column usage data from before it was disabled? - if disableTime == nil || gt.After(*disableTime) { - columnIDs = append(columnIDs, colID) - } + columnIDs = append(columnIDs, colID) } return columnIDs, nil } @@ -193,37 +169,6 @@ func cleanupDroppedColumnStatsUsage(sctx sessionctx.Context, tableID int64) erro return err } -// getDisableColumnTrackingTime reads the value of tidb_disable_column_tracking_time from mysql.tidb if it exists. -// UTC time format is used to store the time. -func getDisableColumnTrackingTime(sctx sessionctx.Context) (*time.Time, error) { - rows, fields, err := utilstats.ExecRows( - sctx, - "SELECT variable_value FROM %n.%n WHERE variable_name = %?", - mysql.SystemDB, - mysql.TiDBTable, - variable.TiDBDisableColumnTrackingTime, - ) - if err != nil { - return nil, err - } - if len(rows) == 0 { - return nil, nil - } - - d := rows[0].GetDatum(0, &fields[0].Column.FieldType) - // The string represents the UTC time when tidb_enable_column_tracking is set to 0. - value, err := d.ToString() - if err != nil { - return nil, err - } - t, err := time.Parse(types.UTCTimeFormat, value) - if err != nil { - return nil, err - } - - return &t, nil -} - // CollectColumnsInExtendedStats returns IDs of the columns involved in extended stats. func CollectColumnsInExtendedStats(sctx sessionctx.Context, tableID int64) ([]int64, error) { const sql = "SELECT name, type, column_ids FROM mysql.stats_extended WHERE table_id = %? and status in (%?, %?)" diff --git a/pkg/statistics/handle/usage/session_stats_collect.go b/pkg/statistics/handle/usage/session_stats_collect.go index 6b1b2699bcd0f..da783971b4651 100644 --- a/pkg/statistics/handle/usage/session_stats_collect.go +++ b/pkg/statistics/handle/usage/session_stats_collect.go @@ -214,9 +214,6 @@ func (s *statsUsageImpl) dumpTableStatCountToKV(is infoschema.InfoSchema, physic // DumpColStatsUsageToKV sweeps the whole list, updates the column stats usage map and dumps it to KV. func (s *statsUsageImpl) DumpColStatsUsageToKV() error { - if !variable.EnableColumnTracking.Load() { - return nil - } s.SweepSessionStatsList() colMap := s.SessionStatsUsage().GetUsageAndReset() defer func() {