*: fix data race of Column.GeneratedExpr (#48888)

br/pkg/lightning/backend/kv/sql2kv.go

@@ -114,7 +114,7 @@ func CollectGeneratedColumns(se *Session, meta *model.TableInfo, cols []*table.C
  var genCols []GeneratedCol
  for i, col := range cols {
  if col.GeneratedExpr != nil {
- expr, err := expression.RewriteAstExpr(se, col.GeneratedExpr, schema, names, true)
+ expr, err := expression.RewriteAstExpr(se, col.GeneratedExpr.Internal(), schema, names, true)
  if err != nil {
  return nil, err
  }

ddl/ddl_api.go

@@ -4874,7 +4874,8 @@ func processColumnOptions(ctx sessionctx.Context, col *table.Column, options []*
  col.GeneratedExprString = sb.String()
  col.GeneratedStored = opt.Stored
  col.Dependences = make(map[string]struct{})
- col.GeneratedExpr = opt.Expr
+ // Only used by checkModifyGeneratedColumn, there is no need to set a ctor for it.
+ col.GeneratedExpr = table.NewClonableExprNode(nil, opt.Expr)
  for _, colName := range FindColumnNamesInExpr(opt.Expr) {
  col.Dependences[colName.Name.L] = struct{}{}
  }
@@ -4935,6 +4936,60 @@ func (d *ddl) getModifiableColumnJob(ctx context.Context, sctx sessionctx.Contex
  return GetModifiableColumnJob(ctx, sctx, is, ident, originalColName, schema, t, spec)
 }
 
+<<<<<<< HEAD:ddl/ddl_api.go
+=======
+func checkModifyColumnWithGeneratedColumnsConstraint(allCols []*table.Column, oldColName model.CIStr) error {
+ for _, col := range allCols {
+ if col.GeneratedExpr == nil {
+ continue
+ }
+ dependedColNames := FindColumnNamesInExpr(col.GeneratedExpr.Internal())
+ for _, name := range dependedColNames {
+ if name.Name.L == oldColName.L {
+ if col.Hidden {
+ return dbterror.ErrDependentByFunctionalIndex.GenWithStackByArgs(oldColName.O)
+ }
+ return dbterror.ErrDependentByGeneratedColumn.GenWithStackByArgs(oldColName.O)
+ }
+ }
+ }
+ return nil
+}
+
+// ProcessColumnCharsetAndCollation process column charset and collation
+func ProcessColumnCharsetAndCollation(sctx sessionctx.Context, col *table.Column, newCol *table.Column, meta *model.TableInfo, specNewColumn *ast.ColumnDef, schema *model.DBInfo) error {
+ var chs, coll string
+ var err error
+ // TODO: Remove it when all table versions are greater than or equal to TableInfoVersion1.
+ // If newCol's charset is empty and the table's version less than TableInfoVersion1,
+ // we will not modify the charset of the column. This behavior is not compatible with MySQL.
+ if len(newCol.FieldType.GetCharset()) == 0 && meta.Version < model.TableInfoVersion1 {
+ chs = col.FieldType.GetCharset()
+ coll = col.FieldType.GetCollate()
+ } else {
+ chs, coll, err = getCharsetAndCollateInColumnDef(sctx.GetSessionVars(), specNewColumn)
+ if err != nil {
+ return errors.Trace(err)
+ }
+ chs, coll, err = ResolveCharsetCollation(sctx.GetSessionVars(),
+ ast.CharsetOpt{Chs: chs, Col: coll},
+ ast.CharsetOpt{Chs: meta.Charset, Col: meta.Collate},
+ ast.CharsetOpt{Chs: schema.Charset, Col: schema.Collate},
+ )
+ chs, coll = OverwriteCollationWithBinaryFlag(sctx.GetSessionVars(), specNewColumn, chs, coll)
+ if err != nil {
+ return errors.Trace(err)
+ }
+ }
+
+ if err = setCharsetCollationFlenDecimal(&newCol.FieldType, newCol.Name.O, chs, coll, sctx.GetSessionVars()); err != nil {
+ return errors.Trace(err)
+ }
+ decodeEnumSetBinaryLiteralToUTF8(&newCol.FieldType, chs)
+ return nil
+}
+
+>>>>>>> d3f399f25d8 (*: fix data race of Column.GeneratedExpr (#48888)):pkg/ddl/ddl_api.go
 // GetModifiableColumnJob returns a DDL job of model.ActionModifyColumn.
 func GetModifiableColumnJob(
  ctx context.Context,

ddl/generated_column.go

@@ -253,7 +253,7 @@ func checkModifyGeneratedColumn(sctx sessionctx.Context, schemaName model.CIStr,
 
  if newCol.IsGenerated() {
  // rule 3.
- if err := checkIllegalFn4Generated(newCol.Name.L, typeColumn, newCol.GeneratedExpr); err != nil {
+ if err := checkIllegalFn4Generated(newCol.Name.L, typeColumn, newCol.GeneratedExpr.Internal()); err != nil {
  return errors.Trace(err)
  }
 

ddl/schematracker/dm_tracker.go

@@ -609,7 +609,7 @@ func (d SchemaTracker) renameColumn(ctx sessionctx.Context, ident ast.Ident, spe
  if col.GeneratedExpr == nil {
  continue
  }
- dependedColNames := ddl.FindColumnNamesInExpr(col.GeneratedExpr)
+ dependedColNames := ddl.FindColumnNamesInExpr(col.GeneratedExpr.Internal())
  for _, name := range dependedColNames {
  if name.Name.L == oldColName.L {
  if col.Hidden {

pkg/table/BUILD.bazel

@@ -0,0 +1,70 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+
+go_library(
+ name = "table",
+ srcs = [
+ "column.go",
+ "constraint.go",
+ "index.go",
+ "table.go",
+ ],
+ importpath = "github.com/pingcap/tidb/pkg/table",
+ visibility = ["//visibility:public"],
+ deps = [
+ "//pkg/errno",
+ "//pkg/expression",
+ "//pkg/kv",
+ "//pkg/meta/autoid",
+ "//pkg/parser",
+ "//pkg/parser/ast",
+ "//pkg/parser/charset",
+ "//pkg/parser/model",
+ "//pkg/parser/mysql",
+ "//pkg/parser/types",
+ "//pkg/sessionctx",
+ "//pkg/sessionctx/stmtctx",
+ "//pkg/types",
+ "//pkg/util/chunk",
+ "//pkg/util/dbterror",
+ "//pkg/util/hack",
+ "//pkg/util/intest",
+ "//pkg/util/logutil",
+ "//pkg/util/mock",
+ "//pkg/util/sqlexec",
+ "//pkg/util/timeutil",
+ "//pkg/util/tracing",
+ "@com_github_pingcap_errors//:errors",
+ "@org_uber_go_zap//:zap",
+ ],
+)
+
+go_test(
+ name = "table_test",
+ timeout = "short",
+ srcs = [
+ "column_test.go",
+ "main_test.go",
+ "table_test.go",
+ ],
+ embed = [":table"],
+ flaky = True,
+ race = "on",
+ shard_count = 9,
+ deps = [
+ "//pkg/errno",
+ "//pkg/expression",
+ "//pkg/parser/ast",
+ "//pkg/parser/charset",
+ "//pkg/parser/model",
+ "//pkg/parser/mysql",
+ "//pkg/parser/terror",
+ "//pkg/sessionctx",
+ "//pkg/sessionctx/stmtctx",
+ "//pkg/testkit/testsetup",
+ "//pkg/types",
+ "//pkg/util/collate",
+ "//pkg/util/mock",
+ "@com_github_stretchr_testify//require",
+ "@org_uber_go_goleak//:goleak",
+ ],
+)

planner/core/logical_plan_builder.go

@@ -4913,7 +4913,7 @@ func (b *PlanBuilder) buildDataSource(ctx context.Context, tn *ast.TableName, as
  var err error
  originVal := b.allowBuildCastArray
  b.allowBuildCastArray = true
- expr, _, err = b.rewrite(ctx, columns[i].GeneratedExpr, ds, nil, true)
+ expr, _, err = b.rewrite(ctx, columns[i].GeneratedExpr.Clone(), ds, nil, true)
  b.allowBuildCastArray = originVal
  if err != nil {
  return nil, err
@@ -5882,7 +5882,7 @@ func (b *PlanBuilder) buildUpdateLists(ctx context.Context, tableList []*ast.Tab
  }
  virtualAssignments = append(virtualAssignments, &ast.Assignment{
  Column: &ast.ColumnName{Schema: tn.Schema, Table: tn.Name, Name: colInfo.Name},
- Expr: tableVal.Cols()[i].GeneratedExpr,
+ Expr: tableVal.Cols()[i].GeneratedExpr.Clone(),
  })
  }
  }

planner/core/planbuilder.go

@@ -3751,7 +3751,7 @@ func (b *PlanBuilder) resolveGeneratedColumns(ctx context.Context, columns []*ta
 
  originalVal := b.allowBuildCastArray
  b.allowBuildCastArray = true
- expr, _, err := b.rewrite(ctx, column.GeneratedExpr, mockPlan, nil, true)
+ expr, _, err := b.rewrite(ctx, column.GeneratedExpr.Clone(), mockPlan, nil, true)
  b.allowBuildCastArray = originalVal
  if err != nil {
  return igc, err

table/column.go

@@ -25,6 +25,7 @@ import (
  "time"
 
  "github.com/pingcap/errors"
+<<<<<<< HEAD:table/column.go
  "github.com/pingcap/tidb/expression"
  "github.com/pingcap/tidb/parser"
  "github.com/pingcap/tidb/parser/ast"
@@ -39,18 +40,64 @@ import (
  "github.com/pingcap/tidb/util/hack"
  "github.com/pingcap/tidb/util/logutil"
  "github.com/pingcap/tidb/util/timeutil"
+=======
+ "github.com/pingcap/tidb/pkg/expression"
+ "github.com/pingcap/tidb/pkg/parser"
+ "github.com/pingcap/tidb/pkg/parser/ast"
+ "github.com/pingcap/tidb/pkg/parser/charset"
+ "github.com/pingcap/tidb/pkg/parser/model"
+ "github.com/pingcap/tidb/pkg/parser/mysql"
+ field_types "github.com/pingcap/tidb/pkg/parser/types"
+ "github.com/pingcap/tidb/pkg/sessionctx"
+ "github.com/pingcap/tidb/pkg/sessionctx/stmtctx"
+ "github.com/pingcap/tidb/pkg/types"
+ "github.com/pingcap/tidb/pkg/util/chunk"
+ "github.com/pingcap/tidb/pkg/util/hack"
+ "github.com/pingcap/tidb/pkg/util/intest"
+ "github.com/pingcap/tidb/pkg/util/logutil"
+ "github.com/pingcap/tidb/pkg/util/timeutil"
+>>>>>>> d3f399f25d8 (*: fix data race of Column.GeneratedExpr (#48888)):pkg/table/column.go
  "go.uber.org/zap"
 )
 
 // Column provides meta data describing a table column.
 type Column struct {
  *model.ColumnInfo
  // If this column is a generated column, the expression will be stored here.
- GeneratedExpr ast.ExprNode
+ GeneratedExpr *ClonableExprNode
  // If this column has default expr value, this expression will be stored here.
  DefaultExpr ast.ExprNode
 }
 
+// ClonableExprNode is a wrapper for ast.ExprNode.
+type ClonableExprNode struct {
+ ctor func() ast.ExprNode
+ internal ast.ExprNode
+}
+
+// NewClonableExprNode creates a ClonableExprNode.
+func NewClonableExprNode(ctor func() ast.ExprNode, internal ast.ExprNode) *ClonableExprNode {
+ return &ClonableExprNode{
+ ctor: ctor,
+ internal: internal,
+ }
+}
+
+// Clone makes a "copy" of internal ast.ExprNode by reconstructing it.
+func (n *ClonableExprNode) Clone() ast.ExprNode {
+ intest.AssertNotNil(n.ctor)
+ if n.ctor == nil {
+ return n.internal
+ }
+ return n.ctor()
+}
+
+// Internal returns the reference of the internal ast.ExprNode.
+// Note: only use this method when you are sure that the internal ast.ExprNode is not modified concurrently.
+func (n *ClonableExprNode) Internal() ast.ExprNode {
+ return n.internal
+}
+
 // String implements fmt.Stringer interface.
 func (c *Column) String() string {
  ans := []string{c.Name.O, types.TypeToStr(c.GetType(), c.GetCharset())}

table/tables/tables.go

@@ -30,6 +30,7 @@ import (
 
  "github.com/pingcap/errors"
  "github.com/pingcap/failpoint"
+<<<<<<< HEAD:table/tables/tables.go
  "github.com/pingcap/tidb/kv"
  "github.com/pingcap/tidb/meta"
  "github.com/pingcap/tidb/meta/autoid"
@@ -52,6 +53,32 @@ import (
  "github.com/pingcap/tidb/util/stringutil"
  "github.com/pingcap/tidb/util/tableutil"
  "github.com/pingcap/tidb/util/tracing"
+=======
+ "github.com/pingcap/tidb/pkg/kv"
+ "github.com/pingcap/tidb/pkg/meta"
+ "github.com/pingcap/tidb/pkg/meta/autoid"
+ "github.com/pingcap/tidb/pkg/parser/ast"
+ "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/binloginfo"
+ "github.com/pingcap/tidb/pkg/sessionctx/stmtctx"
+ "github.com/pingcap/tidb/pkg/sessionctx/variable"
+ "github.com/pingcap/tidb/pkg/statistics"
+ "github.com/pingcap/tidb/pkg/table"
+ "github.com/pingcap/tidb/pkg/tablecodec"
+ "github.com/pingcap/tidb/pkg/types"
+ "github.com/pingcap/tidb/pkg/util"
+ "github.com/pingcap/tidb/pkg/util/chunk"
+ "github.com/pingcap/tidb/pkg/util/codec"
+ "github.com/pingcap/tidb/pkg/util/collate"
+ "github.com/pingcap/tidb/pkg/util/generatedexpr"
+ "github.com/pingcap/tidb/pkg/util/logutil"
+ "github.com/pingcap/tidb/pkg/util/rowcodec"
+ "github.com/pingcap/tidb/pkg/util/stringutil"
+ "github.com/pingcap/tidb/pkg/util/tableutil"
+ "github.com/pingcap/tidb/pkg/util/tracing"
+>>>>>>> d3f399f25d8 (*: fix data race of Column.GeneratedExpr (#48888)):pkg/table/tables/tables.go
  "github.com/pingcap/tipb/go-binlog"
  "github.com/pingcap/tipb/go-tipb"
  "go.uber.org/zap"
@@ -131,15 +158,21 @@ func TableFromMeta(allocs autoid.Allocators, tblInfo *model.TableInfo) (table.Ta
 
  col := table.ToColumn(colInfo)
  if col.IsGenerated() {
- expr, err := generatedexpr.ParseExpression(colInfo.GeneratedExprString)
+ genStr := colInfo.GeneratedExprString
+ expr, err := buildGeneratedExpr(tblInfo, genStr)
  if err != nil {
  return nil, err
  }
- expr, err = generatedexpr.SimpleResolveName(expr, tblInfo)
- if err != nil {
- return nil, err
- }
- col.GeneratedExpr = expr
+ col.GeneratedExpr = table.NewClonableExprNode(func() ast.ExprNode {
+ newExpr, err1 := buildGeneratedExpr(tblInfo, genStr)
+ if err1 != nil {
+ logutil.BgLogger().Warn("unexpected parse generated string error",
+ zap.String("generatedStr", genStr),
+ zap.Error(err1))
+ return expr
+ }
+ return newExpr
+ }, expr)
  }
  // default value is expr.
  if col.DefaultIsExpr {
@@ -166,6 +199,18 @@ func TableFromMeta(allocs autoid.Allocators, tblInfo *model.TableInfo) (table.Ta
  return newPartitionedTable(&t, tblInfo)
 }
 
+func buildGeneratedExpr(tblInfo *model.TableInfo, genExpr string) (ast.ExprNode, error) {
+ expr, err := generatedexpr.ParseExpression(genExpr)
+ if err != nil {
+ return nil, err
+ }
+ expr, err = generatedexpr.SimpleResolveName(expr, tblInfo)
+ if err != nil {
+ return nil, err
+ }
+ return expr, nil
+}
+
 // initTableCommon initializes a TableCommon struct.
 func initTableCommon(t *TableCommon, tblInfo *model.TableInfo, physicalTableID int64, cols []*table.Column, allocs autoid.Allocators) {
  t.tableID = tblInfo.ID