Repository: pingcap/tidb
File: pkg/ddl/modify_column.go (Go)

Code before:

	var err error
	originalState := changingCol.State
	targetCol := changingCol.Clone()
	targetCol.Name = colName
	changingIdxs := buildRelatedIndexInfos(tblInfo, changingCol.ID)
	switch changingCol.State {
	case model.StateNone:
		err = validatePosition(tblInfo, oldCol, pos)
		if err != nil {
			job.State = model.JobStateRollingback
			return ver, errors.Trace(err)
		}
		// Column from null to not null.
		if isNullToNotNullChange(oldCol, changingCol) {
			oldCol.AddFlag(mysql.PreventNullInsertFlag)
		}
		// none -> delete only
		updateObjectState(changingCol, changingIdxs, model.StateDeleteOnly)
		job.ReorgMeta.Stage = model.ReorgStageModifyColumnUpdateColumn
		err = initForReorgIndexes(w, job, changingIdxs)
		if err != nil {
			job.State = model.JobStateRollingback
			return ver, errors.Trace(err)
		}
		failpoint.Inject("mockInsertValueAfterCheckNull", func(val failpoint.Value) {
			if valStr, ok := val.(string); ok {
				var sctx sessionctx.Context
				sctx, err := w.sessPool.Get()
				if err != nil {
					failpoint.Return(ver, err)
				}
				defer w.sessPool.Put(sctx)

				ctx := kv.WithInternalSourceType(context.Background(), kv.InternalTxnDDL)
				//nolint:forcetypeassert
				_, _, err = sctx.GetRestrictedSQLExecutor().ExecRestrictedSQL(ctx, nil, valStr)
				if err != nil {
					job.State = model.JobStateCancelled
					failpoint.Return(ver, err)
				}
			}
		})
		ver, err = updateVersionAndTableInfoWithCheck(jobCtx, job, tblInfo, originalState != changingCol.State)
		if err != nil {
			return ver, errors.Trace(err)
		}
		// Make sure job args change after `updateVersionAndTableInfoWithCheck`, otherwise, the job args will
		// be updated in `updateDDLJob` even if it meets an error in `updateVersionAndTableInfoWithCheck`.
		job.SchemaState = model.StateDeleteOnly
		metrics.GetBackfillProgressByLabel(metrics.LblModifyColumn, job.SchemaName, tblInfo.Name.String(), args.OldColumnName.O).Set(0)

Review comment (suggestion):
The declaration of `err` has been removed on line 968, but `err` is already declared on line 949 with `var err error`. This shadowing removal is correct, but the change appears incomplete. The variable is used throughout the function, so ensure this change doesn't introduce any unintended consequences.

Code after:
	tblInfo *model.TableInfo,
	changingCol, oldCol *model.ColumnInfo,
	args *model.ModifyColumnArgs,
) (ver int64, _ error) {
	colName, pos := args.Column.Name, args.Position

	var err error
	originalState := changingCol.State
	targetCol := changingCol.Clone()
	targetCol.Name = colName
	changingIdxs := buildRelatedIndexInfos(tblInfo, changingCol.ID)
	switch changingCol.State {
	case model.StateNone:
		err = validatePosition(tblInfo, oldCol, pos)
		if err != nil {
			job.State = model.JobStateRollingback
			return ver, errors.Trace(err)
		}
		// Column from null to not null.
		if isNullToNotNullChange(oldCol, changingCol) {
			oldCol.AddFlag(mysql.PreventNullInsertFlag)
		}
		// none -> delete only
		updateObjectState(changingCol, changingIdxs, model.StateDeleteOnly)
		job.ReorgMeta.Stage = model.ReorgStageModifyColumnUpdateColumn
		err = initForReorgIndexes(w, job, changingIdxs)
		if err != nil {
			job.State = model.JobStateRollingback
			return ver, errors.Trace(err)
		}
		failpoint.Inject("mockInsertValueAfterCheckNull", func(val failpoint.Value) {
			if valStr, ok := val.(string); ok {
				var sctx sessionctx.Context
				sctx, err := w.sessPool.Get()
				if err != nil {
					failpoint.Return(ver, err)
				}
				defer w.sessPool.Put(sctx)

				ctx := kv.WithInternalSourceType(context.Background(), kv.InternalTxnDDL)
				//nolint:forcetypeassert
				_, _, err = sctx.GetRestrictedSQLExecutor().ExecRestrictedSQL(ctx, nil, valStr)
				if err != nil {
					job.State = model.JobStateCancelled
					failpoint.Return(ver, err)
				}
			}
		})
		ver, err = updateVersionAndTableInfoWithCheck(jobCtx, job, tblInfo, originalState != changingCol.State)
		if err != nil {
			return ver, errors.Trace(err)