Skip to content
Open
Show file tree
Hide file tree
Changes from 6 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
28 changes: 20 additions & 8 deletions logservice/schemastore/persist_storage_ddl_handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -2870,15 +2870,9 @@ func buildDDLEventForCreateTables(rawEvent *PersistedDDLEvent, tableFilter filte
if allFiltered {
return commonEvent.DDLEvent{}, false, err
}
querys, err := commonEvent.SplitQueries(rawEvent.Query)
querys, err := splitCreateTablesQueries(rawEvent)
if err != nil {
log.Panic("split queries failed", zap.Error(err))
}
if len(querys) != len(rawEvent.MultipleTableInfos) {
log.Panic("query count not match table count",
zap.Int("queryCount", len(querys)),
zap.Int("tableCount", len(rawEvent.MultipleTableInfos)),
zap.String("query", rawEvent.Query))
return commonEvent.DDLEvent{}, false, err
}
ddlEvent.NeedAddedTables = make([]commonEvent.Table, 0, physicalTableCount)
addName := make([]commonEvent.SchemaTableName, 0, logicalTableCount)
Expand Down Expand Up @@ -2938,6 +2932,24 @@ func buildDDLEventForCreateTables(rawEvent *PersistedDDLEvent, tableFilter filte
return ddlEvent, true, err
}

func splitCreateTablesQueries(rawEvent *PersistedDDLEvent) ([]string, error) {
querys, err := commonEvent.SplitQueries(rawEvent.Query)
if err == nil && len(querys) == len(rawEvent.MultipleTableInfos) {
return querys, nil
}
fields := []zap.Field{
zap.Int("tableCount", len(rawEvent.MultipleTableInfos)),
zap.String("query", rawEvent.Query),
}
if err != nil {
fields = append(fields, zap.Error(err))
} else {
fields = append(fields, zap.Int("queryCount", len(querys)))
}
log.Warn("create tables query count not match table count", fields...)
return nil, cerror.WrapError(cerror.ErrTiDBUnexpectedJobMeta, errors.New("create tables query count not match table count"))
}

func buildDDLEventForAlterTablePartitioning(rawEvent *PersistedDDLEvent, tableFilter filter.Filter, tableID int64) (commonEvent.DDLEvent, bool, error) {
ddlEvent, ok, err := buildDDLEventCommon(rawEvent, tableFilter, WithoutTiDBOnly)
if err != nil {
Expand Down
42 changes: 42 additions & 0 deletions logservice/schemastore/persist_storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/pingcap/tidb/pkg/meta/model"
"github.com/pingcap/tidb/pkg/parser/ast"
"github.com/pingcap/tidb/pkg/parser/charset"
"github.com/pingcap/tidb/pkg/parser/mysql"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap"
Expand Down Expand Up @@ -3654,6 +3655,47 @@ func TestBuildDDLEventForNewTableDDL_CreateTableLikeBlockedTables(t *testing.T)
require.ElementsMatch(t, []int64{common.DDLSpanTableID, 111, 112}, ddlEvent.BlockedTables.TableIDs)
}

func TestBuildDDLEventForCreateTablesQueryCountMismatch(t *testing.T) {
tableInfo1 := newEligibleTableInfoForTest(201, "t_26")
tableInfo2 := newEligibleTableInfoForTest(202, "t_27")
tableInfo1.State = model.StatePublic
tableInfo2.State = model.StatePublic
tableInfo1.Columns[0].AddFlag(mysql.NotNullFlag)
tableInfo2.Columns[0].AddFlag(mysql.NotNullFlag)
for _, column := range tableInfo1.Columns {
column.State = model.StatePublic
}
for _, column := range tableInfo2.Columns {
column.State = model.StatePublic
}

job := &model.Job{
Type: model.ActionCreateTables,
SchemaID: 100,
Query: "CREATE TABLE `t_26` (`a` INT PRIMARY KEY,`b` INT)",
BinlogInfo: &model.HistoryInfo{
MultipleTableInfos: []*model.TableInfo{
tableInfo1,
tableInfo2,
},
},
}
originalQueries, err := commonEvent.SplitQueries(job.Query)
require.NoError(t, err)
require.Len(t, originalQueries, 1)

rawEvent := buildPersistedDDLEventForCreateTables(buildPersistedDDLEventFuncArgs{
job: job,
databaseMap: map[int64]*BasicDatabaseInfo{100: {Name: "test"}},
})
require.Equal(t, job.Query, rawEvent.Query)

ddlEvent, ok, err := buildDDLEventForCreateTables(&rawEvent, nil, 0)
require.Error(t, err)
require.False(t, ok)
require.Equal(t, commonEvent.DDLEvent{}, ddlEvent)
}

func TestUpdateDDLHistoryForAddDropTable_CreateTableLikeAddsReferTable(t *testing.T) {
args := updateDDLHistoryFuncArgs{
ddlEvent: &PersistedDDLEvent{
Expand Down
2 changes: 1 addition & 1 deletion tests/integration_tests/_utils/run_sql
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ if [ $# -gt 1 ]; then
other=${*}
fi

prepare="set global tidb_enable_clustered_index = 'int_only';"
prepare="set global tidb_enable_clustered_index = 'int_only'; set global tidb_enable_fast_create_table = OFF;"

# Check if OUT_DIR contains TEST_NAME, if so, truncate OUT_DIR to the parent directory
ADJUSTED_OUT_DIR="$OUT_DIR"
Expand Down
2 changes: 1 addition & 1 deletion tests/integration_tests/_utils/run_sql_file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

set -eu

prepare="set global tidb_enable_clustered_index = 'int_only';"
prepare="set global tidb_enable_clustered_index = 'int_only'; set global tidb_enable_fast_create_table = OFF;"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don't force fast_create_table off in the shared SQL-file runner.

This helper now overrides caller-selected global state on every invocation. In tests/integration_tests/batch_add_table/run.sh, run_with_fast_create_table enables the feature at Line 16, but the later run_sql_file calls at Lines 52-53 flip it back off before those SQL files run. Please make this opt-in per test instead of hard-coding it in the shared runner.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/integration_tests/_utils/run_sql_file` at line 8, The shared runner
currently forces fast_create_table off by embedding "set global
tidb_enable_fast_create_table = OFF;" into the prepare variable; remove that
hard-coded setting from prepare in tests/integration_tests/_utils/run_sql_file
and instead make it opt-in (e.g., accept an optional parameter or environment
flag like FAST_CREATE_TABLE or a prepare_overrides argument to run_sql_file) so
callers can choose to set tidb_enable_fast_create_table on/off; update
run_sql_file to append the fast_create_table SET only when the opt-in flag
indicates it, and update callers (e.g., run_with_fast_create_table in
tests/integration_tests/batch_add_table/run.sh) to pass the flag when they want
the feature enabled.


# Check if OUT_DIR contains TEST_NAME, if so, truncate OUT_DIR to the parent directory
ADJUSTED_OUT_DIR="$OUT_DIR"
Expand Down
2 changes: 1 addition & 1 deletion tests/integration_tests/_utils/run_sql_ignore_error
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ if [ $# -gt 1 ]; then
other=${*}
fi

prepare="set global tidb_enable_clustered_index = 'int_only';"
prepare="set global tidb_enable_clustered_index = 'int_only'; set global tidb_enable_fast_create_table = OFF;"

# Check if OUT_DIR contains TEST_NAME, if so, truncate OUT_DIR to the parent directory
ADJUSTED_OUT_DIR="$OUT_DIR"
Expand Down
2 changes: 2 additions & 0 deletions tests/integration_tests/batch_add_table/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ function run_without_fast_create_table() {

start_tidb_cluster --workdir $WORK_DIR

skip_if_tidb_version_less_than "v8.5.0"
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

run_sql "set global tidb_enable_fast_create_table=off" ${UP_TIDB_HOST} ${UP_TIDB_PORT}

run_cdc_server --workdir $WORK_DIR --binary $CDC_BINARY
Expand Down
Loading