Skip to content

Fix bug for ingesting data to a "pk is handle" table#2125

Merged
JaySon-Huang merged 3 commits intopingcap:masterfrom
JaySon-Huang:fix_snapshot_pk_is_handle
Jun 10, 2021
Merged

Fix bug for ingesting data to a "pk is handle" table#2125
JaySon-Huang merged 3 commits intopingcap:masterfrom
JaySon-Huang:fix_snapshot_pk_is_handle

Conversation

@JaySon-Huang
Copy link
Contributor

@JaySon-Huang JaySon-Huang commented Jun 9, 2021

What problem does this PR solve?

Issue Number: close #2118

Problem Summary:

DeltaTree add a "_tidb_rowid" column to the table even if "pk is handle" is true. We use the FunctionToInt64 to copy data from the primary key column to the handle column under this situation.

https://github.com/pingcap/tics/blob/1078d0b8199a1cfe54d73623ea8da636505f419b/dbms/src/Storages/DeltaMerge/DeltaMergeStore.cpp#L403-L410

However, if the types are identical, FunctionToInt64 only does a shallow copy and makes the primary key column and _tidb_rowid share the same column ptr when "pk is handle".

https://github.com/pingcap/tics/blob/1078d0b8199a1cfe54d73623ea8da636505f419b/dbms/src/Functions/FunctionsConversion.h#L832-L841

After that, we need to reorganize the boundary of blocks, this makes us append some rows into the pk column and handle column. Unfortunately, they share the same column pointer, which makes the columns inside one block don't align and make trouble for later processing.
https://github.com/pingcap/tics/blob/1078d0b8199a1cfe54d73623ea8da636505f419b/dbms/src/Storages/DeltaMerge/PKSquashingBlockInputStream.h#L75-L99

What is changed and how it works?

Use deep copy instead of shallow copy when the types are identical in DeltaMergeStore::addExtraColumnIfNeed.

Related changes

  • PR to update pingcap/docs/pingcap/docs-cn:
  • Need to cherry-pick to the release branch:

Check List

Tests

  • Unit test

Side effects

Release note

  • No release note

Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
@JaySon-Huang JaySon-Huang added priority/release-blocker This issue blocks a release. Please solve it ASAP. type/bugfix This PR fixes a bug. needs-cherry-pick-release-5.1 PR which needs to be cherry-picked to release-5.1 labels Jun 9, 2021
@JaySon-Huang JaySon-Huang requested a review from flowbehappy June 9, 2021 14:36
@JaySon-Huang JaySon-Huang self-assigned this Jun 9, 2021
@JaySon-Huang
Copy link
Contributor Author

/run-all-tests

@JaySon-Huang
Copy link
Contributor Author

/run-all-tests

@JaySon-Huang JaySon-Huang added needs-cherry-pick-release-4.0 PR which needs to be cherry-picked to release-4.0 needs-cherry-pick-release-5.0 PR which needs to be cherry-picked to release-5.0 labels Jun 10, 2021
Copy link
Contributor

@flowbehappy flowbehappy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Jun 10, 2021
@JaySon-Huang JaySon-Huang merged commit 46b8294 into pingcap:master Jun 10, 2021
@JaySon-Huang JaySon-Huang deleted the fix_snapshot_pk_is_handle branch June 10, 2021 05:34
@ti-srebot
Copy link
Collaborator

cherry pick to release-4.0 in PR #2127

@ti-srebot
Copy link
Collaborator

cherry pick to release-5.0 in PR #2128

@ti-srebot
Copy link
Collaborator

cherry pick to release-5.1 in PR #2129

JaySon-Huang added a commit that referenced this pull request Jun 11, 2021
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

Co-authored-by: JaySon <tshent@qq.com>
JaySon-Huang added a commit that referenced this pull request Jun 16, 2021
JaySon-Huang added a commit that referenced this pull request Jun 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-cherry-pick-release-4.0 PR which needs to be cherry-picked to release-4.0 needs-cherry-pick-release-5.0 PR which needs to be cherry-picked to release-5.0 needs-cherry-pick-release-5.1 PR which needs to be cherry-picked to release-5.1 priority/release-blocker This issue blocks a release. Please solve it ASAP. status/LGT1 Indicates that a PR has LGTM 1. type/bugfix This PR fixes a bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The ingesting sst files is broken while running schrodinger-test/bank

3 participants