sql: bugfixes around writing the old primary key in pk changes#44489
Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom Jan 30, 2020
Merged
sql: bugfixes around writing the old primary key in pk changes#44489craig[bot] merged 1 commit intocockroachdb:masterfrom
craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
Member
|
This change is |
solongordon
suggested changes
Jan 30, 2020
Contributor
solongordon
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @rohany and @solongordon)
pkg/sql/alter_table.go, line 402 at r1 (raw file):
// TODO (rohany): is there an easier way of checking if the existing primary index was the // automatically created one? if !(len(n.tableDesc.PrimaryIndex.ColumnNames) == 1 && n.tableDesc.PrimaryIndex.ColumnNames[0] == "rowid") {
Not ideal that this would also apply to a user who happened to create a table with a primary key called "rowid". Could you look for a more reliable way to detect this? Maybe also check that rowid is a hidden column?
pkg/sql/alter_table.go, line 407 at r1 (raw file):
"old_primary_key", nameExists, )
This can be addressed in a separate PR, but I don't love the "old_primary_key" name. What if we just named it the same way we name other indexes where the user doesn't specify a name? Like if you add INDEX (x, y) on a table named t, we call the index t_x_y_idx.
4c1cc39 to
765d816
Compare
Contributor
Author
|
PTAL |
solongordon
approved these changes
Jan 30, 2020
Contributor
solongordon
left a comment
There was a problem hiding this comment.
Reviewable status:
This PR fixes two bugs: * The logic for when to rewrite the old primary key was broken resulting in the old primary key not being rewritten in many cases. * The old primary key being created was not properly dealing with its dangling interleave information. Release note: None
765d816 to
fc51339
Compare
Contributor
Author
|
bors r+ |
Contributor
Build failed (retrying...) |
craig bot
pushed a commit
that referenced
this pull request
Jan 30, 2020
44489: sql: bugfixes around writing the old primary key in pk changes r=rohany a=rohany This PR fixes two bugs: * The logic for when to rewrite the old primary key was broken resulting in the old primary key not being rewritten in many cases. * The old primary key being created was not properly dealing with its dangling interleave information. This PR makes the design decision to not re-interleave the copy of the old primary index if it was interleaved. Release note: None 44551: jobs: always set start time of a job r=spaskob a=spaskob When starting a job via CreateAndStartJob if making the job started fails the job will stil be in system.jobs and can be adopted by another node later but the started time will be 0 in this case. We add a check and set it if necessary. Release note: none. 44553: engine: redefine targetSize and add maxSize to ExportToSst r=itsbilal a=ajwerner This PR is a follow-up of work from #44440 motivated by problems unearthed while typing #44482. The PR comes in two commits: 1) Re-define `targetSize` from being a target below which most requests would remain to being the size above which the export stops. 2) Add a `maxSize` parameter above which the `ExportToSst` call will fail. See the individual commits for more details. Co-authored-by: Rohan Yadav <rohany@alumni.cmu.edu> Co-authored-by: Spas Bojanov <spas@cockroachlabs.com> Co-authored-by: Andrew Werner <ajwerner@cockroachlabs.com>
Contributor
Build succeeded |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This PR fixes two bugs:
resulting in the old primary key not being rewritten in many cases.
its dangling interleave information.
This PR makes the design decision to not re-interleave the copy of the old primary index if it was interleaved.
Release note: None