Skip to content

sql: add more detailed test for operations during a primary key change#45655

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
rohany:pk-ops
Mar 11, 2020
Merged

sql: add more detailed test for operations during a primary key change#45655
craig[bot] merged 1 commit intocockroachdb:masterfrom
rohany:pk-ops

Conversation

@rohany
Copy link
Copy Markdown
Contributor

@rohany rohany commented Mar 3, 2020

The existing operations test did not catch various bugs
uncovered in #45347, so this test expands upon the existing
schema change operations tests with a larger test case.

Release note: None

@rohany rohany requested a review from thoszhang March 3, 2020 17:43
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@rohany
Copy link
Copy Markdown
Contributor Author

rohany commented Mar 10, 2020

cc @lucy-zhang can you take a look at this?

@thoszhang
Copy link
Copy Markdown

Yeah, will do. Would you mind just rebasing this on master and making sure it still works, now that the schema change job changes are in?

@rohany rohany force-pushed the pk-ops branch 3 times, most recently from 6ac5ad1 to fd2f458 Compare March 11, 2020 15:06
@rohany
Copy link
Copy Markdown
Contributor Author

rohany commented Mar 11, 2020

Alright, rebased and everything passed

Copy link
Copy Markdown

@thoszhang thoszhang left a comment

Choose a reason for hiding this comment

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

Thanks!

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @rohany)


pkg/sql/schema_changer_test.go, line 2476 at r1 (raw file):

CREATE TABLE t.test (
	x INT PRIMARY KEY, y INT NOT NULL, z INT, a INT, b INT,
	c INT, d INT, FAMILY (x), FAMILY (y), FAMILY (z),

I think a comment briefly explaining what the logic is for these column families would be helpful.

The existing operations test did not catch various bugs
uncovered in cockroachdb#45347, so this test expands upon the existing
schema change operations tests with a larger test case.

Release justification: non production code change

Release note: None
@rohany
Copy link
Copy Markdown
Contributor Author

rohany commented Mar 11, 2020

bors r=lucy-zhang

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Mar 11, 2020

Build succeeded

@craig craig bot merged commit e70c4ee into cockroachdb:master Mar 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants