From 2300ac6fe6cb29de119ccc04478ffaf4df463ffc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BA=8C=E6=89=8B=E6=8E=89=E5=8C=85=E5=B7=A5=E7=A8=8B?= =?UTF-8?q?=E5=B8=88?= Date: Mon, 27 May 2024 19:20:50 +0800 Subject: [PATCH] statistics: add comments and remove unnessary check (#53568) ref pingcap/tidb#53567 --- pkg/planner/core/planbuilder.go | 1 + .../handle/usage/predicate_column.go | 32 +++++++++++++++---- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/pkg/planner/core/planbuilder.go b/pkg/planner/core/planbuilder.go index da03aa9f89318..e7dbec8c15af1 100644 --- a/pkg/planner/core/planbuilder.go +++ b/pkg/planner/core/planbuilder.go @@ -1949,6 +1949,7 @@ func (b *PlanBuilder) getMustAnalyzedColumns(tbl *ast.TableName, cols *calcOnceM } func (b *PlanBuilder) getPredicateColumns(tbl *ast.TableName, cols *calcOnceMap) (map[int64]struct{}, error) { + // Already calculated in the previous call. if cols.calculated { return cols.data, nil } diff --git a/pkg/statistics/handle/usage/predicate_column.go b/pkg/statistics/handle/usage/predicate_column.go index 43c052ff2b44e..208917a3a6372 100644 --- a/pkg/statistics/handle/usage/predicate_column.go +++ b/pkg/statistics/handle/usage/predicate_column.go @@ -101,8 +101,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 enable_column_tracking = 0`, we should ignore it because - // `set global enable_column_tracking = 0` indicates all the predicate columns collected before. + // 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 @@ -123,17 +123,24 @@ func LoadColumnStatsUsage(sctx sessionctx.Context, loc *time.Location) (map[mode // GetPredicateColumns returns IDs of predicate columns, which are the columns whose stats are used(needed) when generating query plans. func GetPredicateColumns(sctx sessionctx.Context, tableID int64) ([]int64, error) { + // 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", tableID) + 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", + tableID, + ) if err != nil { return nil, errors.Trace(err) } columnIDs := make([]int64, 0, len(rows)) for _, row := range rows { - if row.IsNull(0) || row.IsNull(1) { + // Usually, it should not be NULL. + // This only happens when the last_used_at is not a valid time. + if row.IsNull(1) { continue } colID := row.GetInt64(0) @@ -141,8 +148,10 @@ func GetPredicateColumns(sctx sessionctx.Context, tableID int64) ([]int64, error if err != nil { return nil, errors.Trace(err) } - // If `last_used_at` is before the time when `set global enable_column_tracking = 0`, we don't regard the column as predicate column because - // `set global enable_column_tracking = 0` indicates all the predicate columns collected before. + // 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) } @@ -151,14 +160,22 @@ func GetPredicateColumns(sctx sessionctx.Context, tableID int64) ([]int64, error } // 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) + 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() @@ -169,6 +186,7 @@ func getDisableColumnTrackingTime(sctx sessionctx.Context) (*time.Time, error) { if err != nil { return nil, err } + return &t, nil }