-
Notifications
You must be signed in to change notification settings - Fork 5.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
*: fix bad column offsets. #3754
Merged
Merged
Changes from 1 commit
Commits
Show all changes
37 commits
Select commit
Hold shift + click to select a range
669e2e8
*: fix bad column offsets.
hicqu 50c258c
fix ci.
hicqu 631a8ca
improve code.
hicqu c7a74c7
fix bug.
hicqu d2f8a91
Merge branch 'master' into qupeng/column-offset
hicqu b297477
Merge branch 'master' into qupeng/column-offset
hicqu 22bb700
Merge branch 'master' into qupeng/column-offset
hicqu 93240dd
address comments.
hicqu c8dde90
Merge branch 'master' into qupeng/column-offset
hicqu 96a667a
fix ci.
hicqu 397a44d
Merge branch 'master' into qupeng/column-offset
hicqu 5b1cddc
address comments.
hicqu 5846f7f
Merge branch 'master' into qupeng/column-offset
hicqu e20cb89
Merge branch 'master' into qupeng/column-offset
hicqu d65a7f3
address comments.
hicqu d8fb37b
Merge branch 'master' into qupeng/column-offset
hicqu f205635
address comments.
hicqu 3d88b0f
Merge branch 'master' into qupeng/column-offset
hicqu d76b1e3
Merge branch 'qupeng/column-offset' of github.com:pingcap/tidb into q…
hicqu 9d2b86a
Merge branch 'master' into qupeng/column-offset
hicqu 42b202c
address comments.
hicqu ac2aa3c
address comments.
hicqu 7383b86
Merge branch 'master' into qupeng/column-offset
hicqu 5b0883d
address comments.
hicqu cbba317
address comments.
hicqu bb70730
Merge branch 'master' into qupeng/column-offset
hicqu 906cf4e
fix bad comment.
hicqu 370c1e4
Merge branch 'master' into qupeng/column-offset
hicqu 1269693
Merge branch 'master' into qupeng/column-offset
hicqu f8f0df2
Merge branch 'master' into qupeng/column-offset
hicqu 5b9e9e5
address comments.
hicqu 7665b13
Merge branch 'master' into qupeng/column-offset
hicqu 2f7b85f
Merge branch 'master' into qupeng/column-offset
hicqu 1661ec3
ddl: fix ddl_db_change_test.go.
hicqu 42939df
address comments.
hicqu dbf52ea
Merge branch 'master' into qupeng/column-offset
zimulala 044478d
Merge branch 'master' into qupeng/column-offset
hicqu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
address comments.
In UpdateRecord and InsertRecord, we should set write-only columns origin default values, not default values.
- Loading branch information
commit 5b0883da5851d0e578cd6f5cdccd1ab7951de252
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -238,7 +238,7 @@ func (t *Table) UpdateRecord(ctx context.Context, h int64, oldData, newData []ty | |
if col.State != model.StatePublic { | ||
// if col is in write only or write reorganization state | ||
// and the value is not default, we should use default value. | ||
value, err = table.GetColDefaultValue(ctx, col.ToInfo()) | ||
value, err = table.GetColOriginDefaultValue(ctx, col.ToInfo()) | ||
if err != nil { | ||
return errors.Trace(err) | ||
} | ||
|
@@ -354,7 +354,7 @@ func (t *Table) AddRecord(ctx context.Context, r []types.Datum) (recordID int64, | |
var value types.Datum | ||
if col.State != model.StatePublic { | ||
// if col is in write only or write reorganization state, we must add it with its default value. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/if/If. |
||
value, err = table.GetColDefaultValue(ctx, col.ToInfo()) | ||
value, err = table.GetColOriginDefaultValue(ctx, col.ToInfo()) | ||
if err != nil { | ||
return 0, errors.Trace(err) | ||
} | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't understand the new logic.
The old logic is: if the column state is not public but writable, and there is no column value yet, assign default value to it. (similar to reorganization...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no value yet should mean the value fetched from KV is the default value, but not the value fetched from KV is null.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"the value fetched from KV is the default value" who set the value to default value?