Amending transaction accepts DDLs that changes column types but gives wrong result

[MyonKeminta]

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

set @@global.tidb_enable_change_column_type=true;
create table t (id int primary key, v varchar(10));

session 1	session 2
begin;
insert into t values (1, "123456789");
	alter table t modify column v varchar(5);
commit;
select * from t;		Gets (1, NULL)

2. What did you expect to see? (Required)

Transaction on session 1 fails, either because not support by amend txn or value of v too long.

3. What did you see instead (Required)

v = NULL in the final result.

4. What is your TiDB version? (Required)

master (c9288d2 )

[cfzjywxk]

The new feature on master branch Change column type should not be allowed for txn amend, this is a incompatibily for two new features on the master branch. We need to add check for this column type change doing amend.

[cfzjywxk]

We need to make it compatible with column type change new feature, and add some test cases to keep this compatibility.

[cfzjywxk]

The change column type is still in development phase, the severity could be lowerd. We need to add unit tests for the coupled module, such tests for the amend feature about this is needed.

[AilinKid]

In the my reproducing step, the begin stmt should be changed as begin pessimistic

the reason is that: column type change will add a brand new column (changing column), the dml which insert in the middle state like write only / rerog only should fill the two columns data (one is for the old column, one is for new column); the dml which insert in the public state of the type changed column should only fill the new column with the new id.

func (a *amendCollector) collectModifyColAmendOps(tblAtStart, tblAtCommit table.Table) ([]amendOp, error) {
	for _, colAtCommit := range tblAtCommit.Cols() {
		colAtStart := findColByID(tblAtStart, colAtCommit.ID)
                // here !!!
		if colAtStart != nil {
			err := colChangeAmendable(colAtStart.ColumnInfo, colAtCommit.ColumnInfo)
			if err != nil {
				return nil, err
			}
		}
	}
	return nil, nil
}

If column type change is finished, the new column won't find any related column with same id in the tblAtStart
The right thing for line here, is to find the old column data with old column id, casting it to the new column's field type.

[ti-srebot]

Please edit this comment or add a new comment to complete the following information

Not a bug

1. Remove the 'type/bug' label
2. Add notes to indicate why it is not a bug

Duplicate bug

1. Add the 'type/duplicate' label
2. Add the link to the original bug

Bug

Note: Make Sure that 'component', and 'severity' labels are added
Example for how to fill out the template: #20100

1. Root Cause Analysis (RCA) (optional)

The amend transaction checking for modify column is not compatible with new feature change column type.

2. Symptom (optional)

Incorrect read results.

3. All Trigger Conditions (optional)

Use change column type together with amend transaction, and some transactions are amended before commit.

4. Workaround (optional)

Do not use change column type together with amend transaction.

5. Affected versions

5.0
unreleased

6. Fixed versions

master

[ti-srebot]

( AffectedVersions ) fields are empty.
The values in ( AffectedVersions ) fields are incorrect.