Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion pkg/ddl/column.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ import (
"github.com/pingcap/tidb/pkg/table"
"github.com/pingcap/tidb/pkg/tablecodec"
"github.com/pingcap/tidb/pkg/types"
"github.com/pingcap/tidb/pkg/util/codec"
"github.com/pingcap/tidb/pkg/util/collate"
contextutil "github.com/pingcap/tidb/pkg/util/context"
"github.com/pingcap/tidb/pkg/util/dbterror"
"github.com/pingcap/tidb/pkg/util/intest"
Expand Down Expand Up @@ -810,7 +812,8 @@ func (w *updateColumnWorker) getRowRecord(handle kv.Handle, recordKey []byte, ra
if w.checksumNeeded {
checksum = rowcodec.RawChecksum{Handle: handle}
}
newRowVal, err := tablecodec.EncodeRow(sysTZ, newRow, newColumnIDs, nil, nil, checksum, rd)
enc := codec.NewEncoder(collate.NewCollationEnabled())
newRowVal, err := tablecodec.EncodeRow(enc, sysTZ, newRow, newColumnIDs, nil, nil, checksum, rd)
err = ec.HandleError(err)
if err != nil {
return errors.Trace(err)
Expand Down
8 changes: 5 additions & 3 deletions pkg/ddl/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ import (
"github.com/pingcap/tidb/pkg/util"
"github.com/pingcap/tidb/pkg/util/backoff"
"github.com/pingcap/tidb/pkg/util/chunk"
"github.com/pingcap/tidb/pkg/util/collate"
"github.com/pingcap/tidb/pkg/util/dbterror"
"github.com/pingcap/tidb/pkg/util/engine"
"github.com/pingcap/tidb/pkg/util/generatedexpr"
Expand Down Expand Up @@ -2477,7 +2478,7 @@ func (w *baseIndexWorker) getIndexRecord(idxInfo *model.IndexInfo, handle kv.Han
idxVal[j] = idxColumnVal
}

rsData := tables.TryGetHandleRestoredDataWrapper(w.table.Meta(), nil, w.rowMap, idxInfo)
rsData := tables.TryGetHandleRestoredDataWrapper(collate.NewCollationEnabled(), w.table.Meta(), nil, w.rowMap, idxInfo)
idxRecord := &indexRecord{handle: handle, key: recordKey, vals: idxVal, rsData: rsData}
return idxRecord, nil
}
Expand Down Expand Up @@ -2731,11 +2732,12 @@ func writeChunk(
}()
needRestoreForIndexes := make([]bool, len(indexes))
restore, pkNeedRestore := false, false
useNewCollate := collate.NewCollationEnabled()
Comment thread
D3Hunter marked this conversation as resolved.
if c.PrimaryKeyInfo != nil && c.TableInfo.IsCommonHandle && c.TableInfo.CommonHandleVersion != 0 {
pkNeedRestore = tables.NeedRestoredData(c.PrimaryKeyInfo.Columns, c.TableInfo.Columns)
pkNeedRestore = tables.NeedRestoredData(useNewCollate, c.PrimaryKeyInfo.Columns, c.TableInfo.Columns)
}
for i, index := range indexes {
needRestore := pkNeedRestore || tables.NeedRestoredData(index.Meta().Columns, c.TableInfo.Columns)
needRestore := pkNeedRestore || tables.NeedRestoredData(useNewCollate, index.Meta().Columns, c.TableInfo.Columns)
needRestoreForIndexes[i] = needRestore
restore = restore || needRestore
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/executor/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"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/collate"
"github.com/pingcap/tidb/pkg/util/logutil"
"github.com/pingcap/tidb/pkg/util/ranger"
"github.com/pingcap/tidb/pkg/util/timeutil"
Expand Down Expand Up @@ -363,6 +364,7 @@ func (e *RecoverIndexExec) fetchRecoverRows(ctx context.Context, srcResult dists
idxValLen := len(e.index.Meta().Columns)
result.scanRowCount = 0

useNewCollate := collate.NewCollationEnabled()
for {
err := srcResult.Next(ctx, e.srcChunk)
if err != nil {
Expand Down Expand Up @@ -404,7 +406,7 @@ func (e *RecoverIndexExec) fetchRecoverRows(ctx context.Context, srcResult dists
return nil, err
}
e.idxValsBufs[result.scanRowCount] = idxVals
rsData := tables.TryGetHandleRestoredDataWrapper(e.table.Meta(), plannercore.GetCommonHandleDatum(e.handleCols, row), nil, e.index.Meta())
rsData := tables.TryGetHandleRestoredDataWrapper(useNewCollate, e.table.Meta(), plannercore.GetCommonHandleDatum(e.handleCols, row), nil, e.index.Meta())
e.recoverRows = append(e.recoverRows, recoverRows{handle: handle, idxVals: idxVals, rsData: rsData, skip: true})
result.scanRowCount++
result.currentHandle = handle
Expand Down
2 changes: 2 additions & 0 deletions pkg/executor/test/executor/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ go_test(
"//pkg/testkit/testfailpoint",
"//pkg/types",
"//pkg/util",
"//pkg/util/codec",
"//pkg/util/collate",
"//pkg/util/dbterror/exeerrors",
"//pkg/util/dbterror/plannererrors",
"//pkg/util/mock",
Expand Down
4 changes: 3 additions & 1 deletion pkg/executor/test/executor/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ import (
"github.com/pingcap/tidb/pkg/testkit/testfailpoint"
"github.com/pingcap/tidb/pkg/types"
"github.com/pingcap/tidb/pkg/util"
"github.com/pingcap/tidb/pkg/util/codec"
"github.com/pingcap/tidb/pkg/util/collate"
"github.com/pingcap/tidb/pkg/util/dbterror/exeerrors"
"github.com/pingcap/tidb/pkg/util/dbterror/plannererrors"
"github.com/pingcap/tidb/pkg/util/mock"
Expand Down Expand Up @@ -251,7 +253,7 @@ func setColValue(t *testing.T, txn kv.Transaction, key kv.Key, v types.Datum) {
colIDs := []int64{2, 3}
sc := stmtctx.NewStmtCtxWithTimeZone(time.Local)
rd := rowcodec.Encoder{Enable: true}
value, err := tablecodec.EncodeRow(sc.TimeZone(), row, colIDs, nil, nil, nil, &rd)
value, err := tablecodec.EncodeRow(codec.NewEncoder(collate.NewCollationEnabled()), sc.TimeZone(), row, colIDs, nil, nil, nil, &rd)
require.NoError(t, err)
err = txn.Set(key, value)
require.NoError(t, err)
Expand Down
2 changes: 2 additions & 0 deletions pkg/executor/write.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"github.com/pingcap/tidb/pkg/tablecodec"
"github.com/pingcap/tidb/pkg/types"
"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/dbterror/plannererrors"
"github.com/pingcap/tidb/pkg/util/memory"
Expand Down Expand Up @@ -410,6 +411,7 @@ func addUnchangedKeysForLockByRow(
}
}
unchangedUniqueKey, _, err := tablecodec.GenIndexKey(
codec.NewEncoder(collate.NewCollationEnabled()),
Comment thread
D3Hunter marked this conversation as resolved.
stmtCtx.TimeZone(),
idx.TableMeta(),
meta,
Expand Down
10 changes: 10 additions & 0 deletions pkg/expression/expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ type BuildOptions struct {
AllowCastArray bool
// TargetFieldType indicates to cast the expression to the target field type if it is not nil
TargetFieldType *types.FieldType
// UseNewCollate whether to use new collate when building expression.
Comment thread
D3Hunter marked this conversation as resolved.
UseNewCollate bool
}

// BuildOption is a function to apply optional settings
Expand Down Expand Up @@ -101,6 +103,14 @@ func WithCastExprTo(targetFt *types.FieldType) BuildOption {
}
}

// WithUseNewCollate fixes the collation mode used while building expressions
// that must stay consistent with table or index encoding created earlier.
func WithUseNewCollate(useNewCollate bool) BuildOption {
return func(options *BuildOptions) {
options.UseNewCollate = useNewCollate
}
}

// BuildSimpleExpr builds a simple expression from an ast node.
// This function is used to build some "simple" expressions with limited context.
// The below expressions are not supported:
Expand Down
6 changes: 0 additions & 6 deletions pkg/meta/model/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -653,12 +653,6 @@ func GetIdxChangingFieldType(idxCol *IndexColumn, col *ColumnInfo) *types.FieldT
return &col.FieldType
}

// ColumnNeedRestoredData checks whether a single index column needs restored data.
func ColumnNeedRestoredData(idxCol *IndexColumn, colInfos []*ColumnInfo) bool {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inlined

col := colInfos[idxCol.Offset]
return types.NeedRestoredData(GetIdxChangingFieldType(idxCol, col))
}

// TableNameInfo provides meta data describing a table name info.
type TableNameInfo struct {
ID int64 `json:"id"`
Expand Down
17 changes: 11 additions & 6 deletions pkg/planner/core/expression_rewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@ func buildSimpleExpr(ctx expression.BuildContext, node ast.ExprNode, opts ...exp
return nil, errors.New("expression node should be present")
}

var options expression.BuildOptions
options := expression.BuildOptions{
UseNewCollate: collate.NewCollationEnabled(),
}
for _, opt := range opts {
opt(&options)
}
Expand Down Expand Up @@ -150,6 +152,7 @@ func buildSimpleExpr(ctx expression.BuildContext, node ast.ExprNode, opts ...exp
sourceTable: options.SourceTable,
allowBuildCastArray: options.AllowCastArray,
asScalar: true,
useNewCollate: options.UseNewCollate,
}

if tbl := options.SourceTable; tbl != nil && rewriter.schema == nil {
Expand Down Expand Up @@ -258,7 +261,8 @@ func (b *PlanBuilder) getExpressionRewriter(ctx context.Context, p base.LogicalP
if len(b.rewriterPool) < b.rewriterCounter {
rewriter = &expressionRewriter{
sctx: b.ctx.GetExprCtx(), ctx: ctx,
planCtx: &exprRewriterPlanCtx{plan: p, builder: b, curClause: b.curClause, rollExpand: b.currentBlockExpand},
planCtx: &exprRewriterPlanCtx{plan: p, builder: b, curClause: b.curClause, rollExpand: b.currentBlockExpand},
useNewCollate: collate.NewCollationEnabled(),
}
b.rewriterPool = append(b.rewriterPool, rewriter)
return
Expand Down Expand Up @@ -375,7 +379,8 @@ type expressionRewriter struct {

astNodeStack []ast.Node

planCtx *exprRewriterPlanCtx
planCtx *exprRewriterPlanCtx
useNewCollate bool
}

func (er *expressionRewriter) ctxStackLen() int {
Expand Down Expand Up @@ -1847,7 +1852,7 @@ func (er *expressionRewriter) Leave(originInNode ast.Node) (retNode ast.Node, ok
}, types.EmptyName)
case *ast.SetCollationExpr:
arg := er.ctxStack[len(er.ctxStack)-1]
if collate.NewCollationEnabled() {
if er.useNewCollate {
var collInfo *charset.Collation
// TODO(bb7133): use charset.ValidCharsetAndCollation when its bug is fixed.
if collInfo, er.err = collate.GetCollationByName(v.Collate); er.err != nil {
Expand Down Expand Up @@ -2275,7 +2280,7 @@ func (er *expressionRewriter) castCollationForIn(colLen int, elemCnt int, stkLen
if colLen != 1 {
return
}
if !collate.NewCollationEnabled() {
if !er.useNewCollate {
// See https://github.com/pingcap/tidb/issues/52772
// This function will apply CoercibilityExplicit to the casted expression, but some checks(during ColumnSubstituteImpl) is missed when the new
// collation is disabled, then lead to panic.
Expand Down Expand Up @@ -2393,7 +2398,7 @@ func (er *expressionRewriter) patternLikeOrIlikeToExpression(v *ast.PatternLikeO
fieldType := &types.FieldType{}
isPatternExactMatch := false
// Treat predicate 'like' or 'ilike' the same way as predicate '=' when it is an exact match and new collation is not enabled.
if patExpression, ok := er.ctxStack[l-1].(*expression.Constant); ok && !collate.NewCollationEnabled() {
if patExpression, ok := er.ctxStack[l-1].(*expression.Constant); ok && !er.useNewCollate {
patString, isNull, err := patExpression.EvalString(er.sctx.GetEvalCtx(), chunk.Row{})
if err != nil {
er.err = err
Expand Down
1 change: 1 addition & 0 deletions pkg/server/handler/tests/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ go_test(
"//pkg/testkit/testsetup",
"//pkg/types",
"//pkg/util/codec",
"//pkg/util/collate",
"//pkg/util/deadlockhistory",
"//pkg/util/rowcodec",
"//pkg/util/topsql/state",
Expand Down
3 changes: 2 additions & 1 deletion pkg/server/handler/tests/http_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ import (
"github.com/pingcap/tidb/pkg/testkit/testfailpoint"
"github.com/pingcap/tidb/pkg/types"
"github.com/pingcap/tidb/pkg/util/codec"
"github.com/pingcap/tidb/pkg/util/collate"
"github.com/pingcap/tidb/pkg/util/rowcodec"
"github.com/stretchr/testify/require"
"github.com/tikv/client-go/v2/tikv"
Expand Down Expand Up @@ -655,7 +656,7 @@ func TestDecodeColumnValue(t *testing.T) {
}
rd := rowcodec.Encoder{Enable: true}
sc := stmtctx.NewStmtCtxWithTimeZone(time.UTC)
bs, err := tablecodec.EncodeRow(sc.TimeZone(), row, colIDs, nil, nil, nil, &rd)
bs, err := tablecodec.EncodeRow(codec.NewEncoder(collate.NewCollationEnabled()), sc.TimeZone(), row, colIDs, nil, nil, nil, &rd)
require.NoError(t, err)
require.NotNil(t, bs)
bin := base64.StdEncoding.EncodeToString(bs)
Expand Down
1 change: 1 addition & 0 deletions pkg/session/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ go_library(
"advisory_locks.go",
"bootstrap.go",
"contextimpl.go",
"global_init.go",
"mock_bootstrap.go",
"nontransactional.go",
"session.go",
Expand Down
79 changes: 79 additions & 0 deletions pkg/session/global_init.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Copyright 2026 PingCAP, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package session

import (
"context"

"github.com/pingcap/tidb/pkg/infoschema"
"github.com/pingcap/tidb/pkg/kv"
"github.com/pingcap/tidb/pkg/meta/metadef"
"github.com/pingcap/tidb/pkg/meta/model"
"github.com/pingcap/tidb/pkg/parser/mysql"
"github.com/pingcap/tidb/pkg/util/collate"
"github.com/pingcap/tidb/pkg/util/timeutil"
)

type systemDBFilter struct{}

func (systemDBFilter) SkipLoadDiff(*model.SchemaDiff, infoschema.InfoSchema) bool {
// when initialize global var, we don't start the domain, so this method
// will NOT be called, ok to return false here
return false
}

func (systemDBFilter) SkipLoadSchema(dbInfo *model.DBInfo) bool {
return !metadef.IsSystemDB(dbInfo.Name.L)
}

// we have to use a separate Domain to init the global variables, and then
// close it. as we are using a captured new_collate setting inside TableCommon
// now, otherwise, the captured new_collate setting in TableCommon is still
// cached in the Domain schema cache and is invalid.
func initGlobalVarFromSystemDB(ctx context.Context, store kv.Storage) error {
// the session used to load those global vars depends on the global vars
// themselves, which is a cyclic dependency. but luckily, mysql.tidb is
// created with DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin, it's safe to
// decode below rows without the correct TidbNewCollationEnabled initialized.
//
// Note: collate.newCollationEnabled is set to 1 in init(), so if the
// new_collate=false during first bootstrap, they mismatch.
dom, err := domap.GetOrCreateWithFilter(store, systemDBFilter{})
if err != nil {
return err
}
defer dom.Close()

sess, err := createSessionWithOpt(store, dom, dom.GetSchemaValidator(), dom.InfoCache(), nil)
if err != nil {
return err
}

// get system tz from mysql.tidb
tz, err := sess.getTableValue(ctx, mysql.TiDBTable, tidbSystemTZ)
Comment on lines +59 to +65

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Mark the temporary bootstrap session as restricted before reading mysql.tidb.

Line 59 creates a new internal session, but the old inline load ran after bootstrap sessions were marked InRestrictedSQL in pkg/session/session.go Lines 4393-4395. Preserve that contract before Line 65 reads system-table values.

Proposed fix
 	sess, err := createSessionWithOpt(store, dom, dom.GetSchemaValidator(), dom.InfoCache(), nil)
 	if err != nil {
 		return err
 	}
+	sess.GetSessionVars().InRestrictedSQL = true
 
 	// get system tz from mysql.tidb
 	tz, err := sess.getTableValue(ctx, mysql.TiDBTable, tidbSystemTZ)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
sess, err := createSessionWithOpt(store, dom, dom.GetSchemaValidator(), dom.InfoCache(), nil)
if err != nil {
return err
}
// get system tz from mysql.tidb
tz, err := sess.getTableValue(ctx, mysql.TiDBTable, tidbSystemTZ)
sess, err := createSessionWithOpt(store, dom, dom.GetSchemaValidator(), dom.InfoCache(), nil)
if err != nil {
return err
}
sess.GetSessionVars().InRestrictedSQL = true
// get system tz from mysql.tidb
tz, err := sess.getTableValue(ctx, mysql.TiDBTable, tidbSystemTZ)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/session/global_init.go` around lines 59 - 65, The temporary bootstrap
session created in createSessionWithOpt should be marked as restricted before it
reads mysql.tidb. Update the global init flow around sess so it preserves the
same contract as the bootstrap path in session.go by setting InRestrictedSQL on
the session before calling sess.getTableValue for tidbSystemTZ, then keep the
existing error handling unchanged.

if err != nil {
return err
}
timeutil.SetSystemTZ(tz)

// get the flag from `mysql`.`tidb` which indicating if new collations are enabled.
newCollationEnabled, err := loadCollationParameter(ctx, sess)
if err != nil {
return err
}
collate.SetNewCollationEnabledForTest(newCollationEnabled)

return nil
}
25 changes: 9 additions & 16 deletions pkg/session/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ import (
"github.com/pingcap/tidb/pkg/util/sqlescape"
"github.com/pingcap/tidb/pkg/util/sqlexec"
"github.com/pingcap/tidb/pkg/util/syncutil"
"github.com/pingcap/tidb/pkg/util/timeutil"
"github.com/pingcap/tidb/pkg/util/topsql"
topsqlstate "github.com/pingcap/tidb/pkg/util/topsql/state"
"github.com/pingcap/tidb/pkg/util/topsql/stmtstats"
Expand Down Expand Up @@ -4351,6 +4350,15 @@ func bootstrapSessionImpl(ctx context.Context, store kv.Storage, createSessionsI
return nil, err
}
}
skipInitGlobalVarFromSystemDB := false
failpoint.Inject("skipInitGlobalVarFromSystemDB", func(val failpoint.Value) {
skipInitGlobalVarFromSystemDB = val.(bool)
})
if !skipInitGlobalVarFromSystemDB {
if err = initGlobalVarFromSystemDB(ctx, store); err != nil {
return nil, err
}
}

// initiate disttask framework components which need a store
scheduler.RegisterSchedulerFactory(
Expand All @@ -4374,7 +4382,6 @@ func bootstrapSessionImpl(ctx context.Context, store kv.Storage, createSessionsI
if concurrency < 0 { // it is only for test, in the production, negative value is illegal.
concurrency = 0
}

ses, err := createSessionsImpl(store, 10)
if err != nil {
return nil, err
Expand All @@ -4393,20 +4400,6 @@ func bootstrapSessionImpl(ctx context.Context, store kv.Storage, createSessionsI
ses[i].GetSessionVars().InRestrictedSQL = true
}

// get system tz from mysql.tidb
tz, err := ses[0].getTableValue(ctx, mysql.TiDBTable, tidbSystemTZ)
if err != nil {
return nil, err
}
timeutil.SetSystemTZ(tz)

// get the flag from `mysql`.`tidb` which indicating if new collations are enabled.
newCollationEnabled, err := loadCollationParameter(ctx, ses[0])
if err != nil {
return nil, err
}
collate.SetNewCollationEnabledForTest(newCollationEnabled)

// only start the domain after we have initialized some global variables.
dom := domain.GetDomain(ses[0])
err = dom.Start(ddl.Normal)
Expand Down
1 change: 1 addition & 0 deletions pkg/store/mockstore/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ go_test(
"//pkg/testkit/testsetup",
"//pkg/types",
"//pkg/util/codec",
"//pkg/util/collate",
"//pkg/util/rowcodec",
"@com_github_pingcap_kvproto//pkg/kvrpcpb",
"@com_github_stretchr_testify//require",
Expand Down
Loading
Loading