From b6bb94df298dff76bdcb18a608e590f67d4148e9 Mon Sep 17 00:00:00 2001 From: Benjamin2037 Date: Sat, 28 Feb 2026 22:00:25 +0800 Subject: [PATCH 1/5] table: fix CanSkip incorrectly omitting null column ID in row encoding (#61709) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the null-skip condition in CanSkip that violates Row Format v2 spec. When DefaultValue=nil, OriginDefaultValue=nil, and value is NULL, the column ID was omitted from the encoded row. After NOT NULL→NULL DDL changes, this caused TiFlash to misinterpret missing null column IDs as 'use default' instead of 'value is NULL', leading to data inconsistency between TiKV and TiFlash. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus --- pkg/table/tables/canskip_test.go | 253 +++++++++++++++++++++++++++++++ pkg/table/tables/tables.go | 15 +- 2 files changed, 263 insertions(+), 5 deletions(-) create mode 100644 pkg/table/tables/canskip_test.go diff --git a/pkg/table/tables/canskip_test.go b/pkg/table/tables/canskip_test.go new file mode 100644 index 0000000000000..893072774c2ef --- /dev/null +++ b/pkg/table/tables/canskip_test.go @@ -0,0 +1,253 @@ +// 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 tables_test + +import ( + "testing" + + "github.com/pingcap/tidb/pkg/meta/model" + pmodel "github.com/pingcap/tidb/pkg/parser/model" + "github.com/pingcap/tidb/pkg/parser/mysql" + "github.com/pingcap/tidb/pkg/table" + "github.com/pingcap/tidb/pkg/table/tables" + "github.com/pingcap/tidb/pkg/testkit" + ttypes "github.com/pingcap/tidb/pkg/types" + "github.com/stretchr/testify/require" +) + +// TestCanSkipNullColumnAfterNotNullToNullDDL tests that CanSkip does NOT skip +// encoding null column IDs for columns that changed from NOT NULL to NULL. +// +// This is the regression test for https://github.com/pingcap/tidb/issues/61709 +// +// Row Format v2 spec (docs/design/2018-07-19-row-format.md) states: +// +// "we can not omit the null column ID because if the column ID is not found +// in the row, we will use the default value in the schema which may not be null" +// +// After NOT NULL→NULL DDL, both DefaultValue and OriginDefaultValue are nil. +// The old CanSkip logic would skip encoding the null column ID in this case, +// causing downstream components (TiFlash) to misinterpret the missing column +// as "use default value" instead of "value is NULL". +func TestCanSkipNullColumnAfterNotNullToNullDDL(t *testing.T) { + // Simulate a column that was changed from NOT NULL to NULL via DDL. + // After the DDL, DefaultValue=nil, OriginDefaultValue=nil, and the column + // is now nullable (NotNullFlag is cleared). + colInfo := &model.ColumnInfo{ + ID: 1, + Name: pmodel.NewCIStr("c"), + } + colInfo.SetType(mysql.TypeLong) + colInfo.SetFlag(0) // NOT NULL flag is cleared after DDL + // After NOT NULL→NULL DDL, both defaults are nil + colInfo.DefaultValue = nil + colInfo.OriginDefaultValue = nil + colInfo.State = model.StatePublic + + col := &table.Column{ColumnInfo: colInfo} + + tblInfo := &model.TableInfo{ + ID: 1, + Name: pmodel.NewCIStr("t"), + Columns: []*model.ColumnInfo{colInfo}, + } + + // The value is NULL + nullDatum := ttypes.NewDatum(nil) + + // CanSkip should return false — the null column ID MUST be encoded + // so downstream can distinguish "value is NULL" from "column not in row". + result := tables.CanSkip(tblInfo, col, &nullDatum) + require.False(t, result, + "CanSkip should NOT skip null column encoding after NOT NULL→NULL DDL. "+ + "Row Format v2 requires null column IDs to be encoded so downstream "+ + "components can distinguish NULL values from missing columns.") +} + +// TestCanSkipNullColumnAlwaysNullable tests that CanSkip correctly handles +// columns that have always been nullable with nil defaults. +// This is the case where the optimization was originally intended to work. +func TestCanSkipNullColumnAlwaysNullable(t *testing.T) { + // A column that was always nullable, with nil defaults from creation. + colInfo := &model.ColumnInfo{ + ID: 2, + Name: pmodel.NewCIStr("d"), + } + colInfo.SetType(mysql.TypeLong) + colInfo.SetFlag(0) // nullable + colInfo.DefaultValue = nil + colInfo.OriginDefaultValue = nil + colInfo.State = model.StatePublic + + col := &table.Column{ColumnInfo: colInfo} + + tblInfo := &model.TableInfo{ + ID: 2, + Name: pmodel.NewCIStr("t2"), + Columns: []*model.ColumnInfo{colInfo}, + } + + nullDatum := ttypes.NewDatum(nil) + + // Even for always-nullable columns, per Row Format v2 spec, null column IDs + // should be encoded. The spec says "we can not omit the null column ID". + // This test documents the correct behavior after the fix. + result := tables.CanSkip(tblInfo, col, &nullDatum) + require.False(t, result, + "CanSkip should NOT skip null column encoding even for always-nullable columns. "+ + "Row Format v2 spec requires null column IDs to always be encoded.") +} + +// TestCanSkipPKColumn tests that CanSkip correctly skips PK handle columns. +func TestCanSkipPKColumn(t *testing.T) { + colInfo := &model.ColumnInfo{ + ID: 1, + Name: pmodel.NewCIStr("id"), + } + colInfo.SetType(mysql.TypeLong) + colInfo.SetFlag(mysql.PriKeyFlag | mysql.NotNullFlag) + colInfo.State = model.StatePublic + + col := &table.Column{ColumnInfo: colInfo} + + tblInfo := &model.TableInfo{ + ID: 3, + Name: pmodel.NewCIStr("t3"), + Columns: []*model.ColumnInfo{colInfo}, + PKIsHandle: true, + } + + datum := ttypes.NewIntDatum(1) + + // PK handle columns should always be skipped + result := tables.CanSkip(tblInfo, col, &datum) + require.True(t, result, "CanSkip should skip PK handle columns") +} + +// TestCanSkipVirtualGeneratedColumn tests that CanSkip correctly skips virtual generated columns. +func TestCanSkipVirtualGeneratedColumn(t *testing.T) { + colInfo := &model.ColumnInfo{ + ID: 3, + Name: pmodel.NewCIStr("v"), + GeneratedExprString: "`a` + 1", + GeneratedStored: false, // virtual + } + colInfo.SetType(mysql.TypeLong) + colInfo.State = model.StatePublic + + col := &table.Column{ColumnInfo: colInfo} + + tblInfo := &model.TableInfo{ + ID: 4, + Name: pmodel.NewCIStr("t4"), + Columns: []*model.ColumnInfo{colInfo}, + } + + datum := ttypes.NewIntDatum(42) + + // Virtual generated columns should always be skipped + result := tables.CanSkip(tblInfo, col, &datum) + require.True(t, result, "CanSkip should skip virtual generated columns") +} + +// TestCanSkipNonNullValueWithDefault tests that CanSkip does NOT skip +// columns with non-null values. +func TestCanSkipNonNullValueWithDefault(t *testing.T) { + colInfo := &model.ColumnInfo{ + ID: 4, + Name: pmodel.NewCIStr("e"), + } + colInfo.SetType(mysql.TypeLong) + colInfo.SetFlag(0) // nullable + colInfo.DefaultValue = nil + colInfo.OriginDefaultValue = nil + colInfo.State = model.StatePublic + + col := &table.Column{ColumnInfo: colInfo} + + tblInfo := &model.TableInfo{ + ID: 5, + Name: pmodel.NewCIStr("t5"), + Columns: []*model.ColumnInfo{colInfo}, + } + + // Non-null value should never be skipped + datum := ttypes.NewIntDatum(42) + result := tables.CanSkip(tblInfo, col, &datum) + require.False(t, result, "CanSkip should NOT skip columns with non-null values") +} + +// TestCanSkipNullColumnWithNonNilDefault tests that CanSkip does NOT skip +// columns where the value is null but the default is non-nil. +func TestCanSkipNullColumnWithNonNilDefault(t *testing.T) { + colInfo := &model.ColumnInfo{ + ID: 5, + Name: pmodel.NewCIStr("f"), + } + colInfo.SetType(mysql.TypeLong) + colInfo.SetFlag(0) // nullable + colInfo.DefaultValue = int64(0) + colInfo.OriginDefaultValue = int64(0) + colInfo.State = model.StatePublic + + col := &table.Column{ColumnInfo: colInfo} + + tblInfo := &model.TableInfo{ + ID: 6, + Name: pmodel.NewCIStr("t6"), + Columns: []*model.ColumnInfo{colInfo}, + } + + nullDatum := ttypes.NewDatum(nil) + + // Null value with non-nil default should NOT be skipped + result := tables.CanSkip(tblInfo, col, &nullDatum) + require.False(t, result, + "CanSkip should NOT skip null column when default value is non-nil") +} + +// TestCanSkipEndToEndNotNullToNull is an integration test that verifies +// the full DDL flow: create table with NOT NULL column, alter to NULL, +// insert NULL value, and verify the row encoding includes the null column ID. +func TestCanSkipEndToEndNotNullToNull(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + + // Step 1: Create table with NOT NULL column + tk.MustExec("CREATE TABLE t_canskip (id INT PRIMARY KEY, c INT NOT NULL DEFAULT 0)") + + // Step 2: ALTER column from NOT NULL to NULL + tk.MustExec("ALTER TABLE t_canskip MODIFY COLUMN c INT NULL") + + // Step 3: Insert a row with NULL value for column c + tk.MustExec("INSERT INTO t_canskip VALUES (1, NULL)") + + // Step 4: Verify the NULL value is correctly stored and retrievable + tk.MustQuery("SELECT id, c FROM t_canskip WHERE id = 1").Check( + testkit.Rows("1 "), + ) + + // Step 5: Insert another row with non-NULL value to verify normal operation + tk.MustExec("INSERT INTO t_canskip VALUES (2, 42)") + tk.MustQuery("SELECT id, c FROM t_canskip WHERE id = 2").Check( + testkit.Rows("2 42"), + ) + + // Step 6: Verify both rows + tk.MustQuery("SELECT id, c FROM t_canskip ORDER BY id").Check( + testkit.Rows("1 ", "2 42"), + ) +} diff --git a/pkg/table/tables/tables.go b/pkg/table/tables/tables.go index 8fba8268c54e9..1bd4aa105a15d 100644 --- a/pkg/table/tables/tables.go +++ b/pkg/table/tables/tables.go @@ -1523,8 +1523,16 @@ func (t *TableCommon) canSkip(col *table.Column, value *types.Datum) bool { // CanSkip is for these cases, we can skip the columns in encoded row: // 1. the column is included in primary key; -// 2. the column's default value is null, and the value equals to that but has no origin default; -// 3. the column is virtual generated. +// 2. the column is virtual generated. +// +// Note: Previously, columns with nil default, nil origin default, and null value +// were also skipped. This was removed because it violates the Row Format v2 spec +// (docs/design/2018-07-19-row-format.md) which states: +// "we can not omit the null column ID because if the column ID is not found +// in the row, we will use the default value in the schema which may not be null" +// After NOT NULL→NULL DDL changes, both defaults become nil, causing downstream +// components (TiFlash) to misinterpret missing null column IDs as "use default" +// instead of "value is NULL". See https://github.com/pingcap/tidb/issues/61709 func CanSkip(info *model.TableInfo, col *table.Column, value *types.Datum) bool { if col.IsPKHandleColumn(info) { return true @@ -1540,9 +1548,6 @@ func CanSkip(info *model.TableInfo, col *table.Column, value *types.Datum) bool return canSkip } } - if col.GetDefaultValue() == nil && value.IsNull() && col.GetOriginDefaultValue() == nil { - return true - } if col.IsVirtualGenerated() { return true } From 4a4d4c233074314a58eeef8b3760329a41425c01 Mon Sep 17 00:00:00 2001 From: Benjamin2037 Date: Sun, 1 Mar 2026 01:24:23 +0800 Subject: [PATCH 2/5] table: fix gofmt/gci comment formatting in CanSkip --- pkg/table/tables/tables.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/table/tables/tables.go b/pkg/table/tables/tables.go index 1bd4aa105a15d..7eae9f53c8eba 100644 --- a/pkg/table/tables/tables.go +++ b/pkg/table/tables/tables.go @@ -1528,8 +1528,10 @@ func (t *TableCommon) canSkip(col *table.Column, value *types.Datum) bool { // Note: Previously, columns with nil default, nil origin default, and null value // were also skipped. This was removed because it violates the Row Format v2 spec // (docs/design/2018-07-19-row-format.md) which states: -// "we can not omit the null column ID because if the column ID is not found -// in the row, we will use the default value in the schema which may not be null" +// +// "we can not omit the null column ID because if the column ID is not found +// in the row, we will use the default value in the schema which may not be null" +// // After NOT NULL→NULL DDL changes, both defaults become nil, causing downstream // components (TiFlash) to misinterpret missing null column IDs as "use default" // instead of "value is NULL". See https://github.com/pingcap/tidb/issues/61709 From b21d013db72d9d0638573247a3eef35ebd4a6872 Mon Sep 17 00:00:00 2001 From: Benjamin2037 Date: Sun, 1 Mar 2026 07:37:37 +0800 Subject: [PATCH 3/5] =?UTF-8?q?table:=20fix=20invalid=20import=20in=20cans?= =?UTF-8?q?kip=5Ftest.go=20(parser/model=20=E2=86=92=20parser/ast)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- pkg/table/tables/canskip_test.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/pkg/table/tables/canskip_test.go b/pkg/table/tables/canskip_test.go index 893072774c2ef..6dc5dfec69f7c 100644 --- a/pkg/table/tables/canskip_test.go +++ b/pkg/table/tables/canskip_test.go @@ -18,7 +18,7 @@ import ( "testing" "github.com/pingcap/tidb/pkg/meta/model" - pmodel "github.com/pingcap/tidb/pkg/parser/model" + "github.com/pingcap/tidb/pkg/parser/ast" "github.com/pingcap/tidb/pkg/parser/mysql" "github.com/pingcap/tidb/pkg/table" "github.com/pingcap/tidb/pkg/table/tables" @@ -47,7 +47,7 @@ func TestCanSkipNullColumnAfterNotNullToNullDDL(t *testing.T) { // is now nullable (NotNullFlag is cleared). colInfo := &model.ColumnInfo{ ID: 1, - Name: pmodel.NewCIStr("c"), + Name: ast.NewCIStr("c"), } colInfo.SetType(mysql.TypeLong) colInfo.SetFlag(0) // NOT NULL flag is cleared after DDL @@ -60,7 +60,7 @@ func TestCanSkipNullColumnAfterNotNullToNullDDL(t *testing.T) { tblInfo := &model.TableInfo{ ID: 1, - Name: pmodel.NewCIStr("t"), + Name: ast.NewCIStr("t"), Columns: []*model.ColumnInfo{colInfo}, } @@ -83,7 +83,7 @@ func TestCanSkipNullColumnAlwaysNullable(t *testing.T) { // A column that was always nullable, with nil defaults from creation. colInfo := &model.ColumnInfo{ ID: 2, - Name: pmodel.NewCIStr("d"), + Name: ast.NewCIStr("d"), } colInfo.SetType(mysql.TypeLong) colInfo.SetFlag(0) // nullable @@ -95,7 +95,7 @@ func TestCanSkipNullColumnAlwaysNullable(t *testing.T) { tblInfo := &model.TableInfo{ ID: 2, - Name: pmodel.NewCIStr("t2"), + Name: ast.NewCIStr("t2"), Columns: []*model.ColumnInfo{colInfo}, } @@ -114,7 +114,7 @@ func TestCanSkipNullColumnAlwaysNullable(t *testing.T) { func TestCanSkipPKColumn(t *testing.T) { colInfo := &model.ColumnInfo{ ID: 1, - Name: pmodel.NewCIStr("id"), + Name: ast.NewCIStr("id"), } colInfo.SetType(mysql.TypeLong) colInfo.SetFlag(mysql.PriKeyFlag | mysql.NotNullFlag) @@ -124,7 +124,7 @@ func TestCanSkipPKColumn(t *testing.T) { tblInfo := &model.TableInfo{ ID: 3, - Name: pmodel.NewCIStr("t3"), + Name: ast.NewCIStr("t3"), Columns: []*model.ColumnInfo{colInfo}, PKIsHandle: true, } @@ -140,7 +140,7 @@ func TestCanSkipPKColumn(t *testing.T) { func TestCanSkipVirtualGeneratedColumn(t *testing.T) { colInfo := &model.ColumnInfo{ ID: 3, - Name: pmodel.NewCIStr("v"), + Name: ast.NewCIStr("v"), GeneratedExprString: "`a` + 1", GeneratedStored: false, // virtual } @@ -151,7 +151,7 @@ func TestCanSkipVirtualGeneratedColumn(t *testing.T) { tblInfo := &model.TableInfo{ ID: 4, - Name: pmodel.NewCIStr("t4"), + Name: ast.NewCIStr("t4"), Columns: []*model.ColumnInfo{colInfo}, } @@ -167,7 +167,7 @@ func TestCanSkipVirtualGeneratedColumn(t *testing.T) { func TestCanSkipNonNullValueWithDefault(t *testing.T) { colInfo := &model.ColumnInfo{ ID: 4, - Name: pmodel.NewCIStr("e"), + Name: ast.NewCIStr("e"), } colInfo.SetType(mysql.TypeLong) colInfo.SetFlag(0) // nullable @@ -179,7 +179,7 @@ func TestCanSkipNonNullValueWithDefault(t *testing.T) { tblInfo := &model.TableInfo{ ID: 5, - Name: pmodel.NewCIStr("t5"), + Name: ast.NewCIStr("t5"), Columns: []*model.ColumnInfo{colInfo}, } @@ -194,7 +194,7 @@ func TestCanSkipNonNullValueWithDefault(t *testing.T) { func TestCanSkipNullColumnWithNonNilDefault(t *testing.T) { colInfo := &model.ColumnInfo{ ID: 5, - Name: pmodel.NewCIStr("f"), + Name: ast.NewCIStr("f"), } colInfo.SetType(mysql.TypeLong) colInfo.SetFlag(0) // nullable @@ -206,7 +206,7 @@ func TestCanSkipNullColumnWithNonNilDefault(t *testing.T) { tblInfo := &model.TableInfo{ ID: 6, - Name: pmodel.NewCIStr("t6"), + Name: ast.NewCIStr("t6"), Columns: []*model.ColumnInfo{colInfo}, } From 6764dfe62a7929f0d64e036f218d8b8e6dbc8a00 Mon Sep 17 00:00:00 2001 From: Benjamin2037 Date: Sun, 1 Mar 2026 07:47:59 +0800 Subject: [PATCH 4/5] table: update BUILD.bazel for canskip_test.go and shard_count --- pkg/table/tables/BUILD.bazel | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/table/tables/BUILD.bazel b/pkg/table/tables/BUILD.bazel index eb6dac9730867..25b602792f09a 100644 --- a/pkg/table/tables/BUILD.bazel +++ b/pkg/table/tables/BUILD.bazel @@ -71,6 +71,7 @@ go_test( "assertion_test.go", "bench_test.go", "cache_test.go", + "canskip_test.go", "export_test.go", "index_test.go", "main_test.go", @@ -80,7 +81,7 @@ go_test( ], embed = [":tables"], flaky = True, - shard_count = 43, + shard_count = 50, deps = [ "//pkg/ddl", "//pkg/domain", From 78c9343c9d2a49c8a26a3d5ab4b562613d3c5836 Mon Sep 17 00:00:00 2001 From: Benjamin2037 Date: Sun, 1 Mar 2026 08:06:39 +0800 Subject: [PATCH 5/5] table: fix e2e test to precisely hit nil/nil trigger condition The previous e2e test used CREATE TABLE (c INT NOT NULL DEFAULT 0) then ALTER TABLE MODIFY COLUMN c INT NULL. After this DDL, OriginDefaultValue remains '0' (not nil) because noReorgDataStrict returns true for same-type INT changes, so generateOriginDefaultValue is never called. This means the test never hit the actual bug trigger condition (DefaultValue=nil AND OriginDefaultValue=nil AND value=NULL). Fix: Use scenarios that actually produce nil/nil state: - CREATE TABLE (c INT NULL) without explicit DEFAULT - ALTER TABLE ADD COLUMN d INT NULL without explicit DEFAULT Also add InfoSchema assertions to verify the schema state after DDL, ensuring both DefaultValue and OriginDefaultValue are nil. --- pkg/table/tables/canskip_test.go | 59 ++++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 18 deletions(-) diff --git a/pkg/table/tables/canskip_test.go b/pkg/table/tables/canskip_test.go index 6dc5dfec69f7c..8807b6813dbb1 100644 --- a/pkg/table/tables/canskip_test.go +++ b/pkg/table/tables/canskip_test.go @@ -15,6 +15,7 @@ package tables_test import ( + "context" "testing" "github.com/pingcap/tidb/pkg/meta/model" @@ -219,35 +220,57 @@ func TestCanSkipNullColumnWithNonNilDefault(t *testing.T) { } // TestCanSkipEndToEndNotNullToNull is an integration test that verifies -// the full DDL flow: create table with NOT NULL column, alter to NULL, -// insert NULL value, and verify the row encoding includes the null column ID. +// the CanSkip behavior through the full DDL + DML flow. +// It tests scenarios that produce DefaultValue=nil and OriginDefaultValue=nil, +// which is the exact trigger condition for the bug in issue #61709. +// After the fix, inserting NULL must encode the null column ID in the row. func TestCanSkipEndToEndNotNullToNull(t *testing.T) { - store := testkit.CreateMockStore(t) + store, dom := testkit.CreateMockStoreAndDomain(t) tk := testkit.NewTestKit(t, store) tk.MustExec("use test") - // Step 1: Create table with NOT NULL column - tk.MustExec("CREATE TABLE t_canskip (id INT PRIMARY KEY, c INT NOT NULL DEFAULT 0)") + // Scenario 1: Column created as nullable with no explicit DEFAULT. + // This produces DefaultValue=nil, OriginDefaultValue=nil. + tk.MustExec("CREATE TABLE t_canskip1 (id INT PRIMARY KEY, c INT NULL)") + + // Verify schema state: both defaults must be nil to hit the bug trigger. + tbl1, err := dom.InfoSchema().TableByName(context.Background(), ast.NewCIStr("test"), ast.NewCIStr("t_canskip1")) + require.NoError(t, err) + colC := tbl1.Meta().Columns[1] + require.Equal(t, "c", colC.Name.L) + require.Nil(t, colC.DefaultValue, "DefaultValue should be nil for nullable column without DEFAULT") + require.Nil(t, colC.OriginDefaultValue, "OriginDefaultValue should be nil for nullable column without DEFAULT") + + // Insert NULL — this is the exact path that triggered the bug. + tk.MustExec("INSERT INTO t_canskip1 VALUES (1, NULL)") + tk.MustQuery("SELECT id, c FROM t_canskip1 WHERE id = 1").Check( + testkit.Rows("1 "), + ) - // Step 2: ALTER column from NOT NULL to NULL - tk.MustExec("ALTER TABLE t_canskip MODIFY COLUMN c INT NULL") + // Scenario 2: ADD COLUMN as nullable with no explicit DEFAULT. + tk.MustExec("CREATE TABLE t_canskip2 (id INT PRIMARY KEY)") + tk.MustExec("ALTER TABLE t_canskip2 ADD COLUMN d INT NULL") - // Step 3: Insert a row with NULL value for column c - tk.MustExec("INSERT INTO t_canskip VALUES (1, NULL)") + // Verify schema state after ADD COLUMN. + tbl2, err := dom.InfoSchema().TableByName(context.Background(), ast.NewCIStr("test"), ast.NewCIStr("t_canskip2")) + require.NoError(t, err) + colD := tbl2.Meta().Columns[1] + require.Equal(t, "d", colD.Name.L) + require.Nil(t, colD.DefaultValue, "DefaultValue should be nil after ADD COLUMN nullable") + require.Nil(t, colD.OriginDefaultValue, "OriginDefaultValue should be nil after ADD COLUMN nullable") - // Step 4: Verify the NULL value is correctly stored and retrievable - tk.MustQuery("SELECT id, c FROM t_canskip WHERE id = 1").Check( + tk.MustExec("INSERT INTO t_canskip2 VALUES (1, NULL)") + tk.MustQuery("SELECT id, d FROM t_canskip2 WHERE id = 1").Check( testkit.Rows("1 "), ) - // Step 5: Insert another row with non-NULL value to verify normal operation - tk.MustExec("INSERT INTO t_canskip VALUES (2, 42)") - tk.MustQuery("SELECT id, c FROM t_canskip WHERE id = 2").Check( - testkit.Rows("2 42"), + // Verify non-NULL values still work correctly. + tk.MustExec("INSERT INTO t_canskip1 VALUES (2, 42)") + tk.MustExec("INSERT INTO t_canskip2 VALUES (2, 42)") + tk.MustQuery("SELECT id, c FROM t_canskip1 ORDER BY id").Check( + testkit.Rows("1 ", "2 42"), ) - - // Step 6: Verify both rows - tk.MustQuery("SELECT id, c FROM t_canskip ORDER BY id").Check( + tk.MustQuery("SELECT id, d FROM t_canskip2 ORDER BY id").Check( testkit.Rows("1 ", "2 42"), ) }