*: fix writing null value into not null column in write-only state. #6249
Conversation
GetColDefaultValue returns null if the column is not public which cause writing null value into the not null column.
coocood
changed the title
coocood
changed the title
|
/run-all-tests |
| tk1.MustExec("alter table nn add column c int not null default 0") | ||
| close(closeCh) | ||
| wg.Wait() | ||
| c.Assert(tk2Err, IsNil) |
There was a problem hiding this comment.
Should we check the result of select count(*) from nn where c is null; is zero?
There was a problem hiding this comment.
It's not needed, the test fails every time before the fix.
| } | ||
| } | ||
| }() | ||
| tk1.MustExec("alter table nn add column c int not null default 0") |
There was a problem hiding this comment.
Can we use ddl hooker to sleep a while in write-only stage, like runTestInSchemaState does?
There was a problem hiding this comment.
It's not needed, the test fails every time before the fix.
There was a problem hiding this comment.
I'll add the test in another pr.
|
/run-all-tests |
executor/write.go
Outdated
| if err != nil { | ||
| return errors.Trace(err) | ||
| } | ||
| // Fill write-only columns with originDefaultValue if value is null. | ||
| for _, col := range cols { | ||
| if col.State != model.StatePublic && oldRow[col.Offset].IsNull() { |
There was a problem hiding this comment.
Do we need to check the null value is updated by other TiDBs?
executor/write.go
Outdated
| // Fill write-only columns with originDefaultValue if value is null. | ||
| for _, col := range cols { | ||
| if col.State != model.StatePublic && oldRow[col.Offset].IsNull() { | ||
| oldRow[col.Offset], err = table.GetColOriginDefaultValue(e.ctx, col.ToInfo()) |
There was a problem hiding this comment.
Cloud we put this logic to UpdateRecord in tables.go?
|
/run-all-tests |
executor/write.go
Outdated
| if err != nil { | ||
| return errors.Trace(err) | ||
| } | ||
| // Fill write-only columns with originDefaultValue if not found in oldValue is null. |
There was a problem hiding this comment.
Do we need to add write-reorg to this comment?
table/tables/tables.go
Outdated
| @@ -515,8 +515,7 @@ func (t *Table) RowWithCols(ctx sessionctx.Context, h int64, cols []*table.Colum | |||
|
|
|||
| // DecodeRawRowData decodes raw row data to a datum row. | |||
| func DecodeRawRowData(ctx sessionctx.Context, meta *model.TableInfo, h int64, cols []*table.Column, | |||
| value []byte) ([]types.Datum, | |||
| error) { | |||
| value []byte) ([]types.Datum, map[int64]types.Datum, error) { | |||
There was a problem hiding this comment.
Add comment for the new return value.
|
LGTM |
|
LGTM |
| @@ -506,17 +506,16 @@ func (t *Table) RowWithCols(ctx sessionctx.Context, h int64, cols []*table.Colum | |||
| if err != nil { | |||
| return nil, errors.Trace(err) | |||
| } | |||
| v, err := DecodeRawRowData(ctx, t.Meta(), h, cols, value) | |||
| v, _, err := DecodeRawRowData(ctx, t.Meta(), h, cols, value) | |||
There was a problem hiding this comment.
Do we need to get the default value once again as we did in duplicate update?
GetColDefaultValue returns null if the column is not public which cause writing null value into the not null column.