sql: add test TestAlterRegionalByRowEnclosingRegionAddDrop#64017
sql: add test TestAlterRegionalByRowEnclosingRegionAddDrop#64017craig[bot] merged 2 commits intocockroachdb:masterfrom
Conversation
|
This change is |
8576d1e to
7deb073
Compare
| }, | ||
| { | ||
| name: "alter-from-explicit-regional", | ||
| setup: `CREATE TABLE db.t () LOCALITY REGIONAL IN "us-east2"`, |
There was a problem hiding this comment.
i'm convinced REGIONAL BY ROW -> GLOBAL / REGIONAL BY TABLE is broken when a sandwiched ADD/DROP REGION comes in in that it creates extraneous partitions / zone configs on the GLOBAL table because we repartition on all Non-Drop indexes, but looks like we're not there yet....
7deb073 to
469c2b2
Compare
ajstorm
left a comment
There was a problem hiding this comment.
Reviewed 1 of 2 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @arulajmani and @otan)
pkg/ccl/multiregionccl/multiregionccltestutils/testutils.go, line 69 at r3 (raw file):
// expected partitions. func TestingEnsureCorrectPartitioning( sqlDB *gosql.DB, expectedPartitions []string, expectedIndexes []string, tableFQN string,
Nit: for expected partitions here, does it make sense to derive that from a SHOW REGIONS FROM DATABASE command? That would make this both easier to call, less error prone (hopefully), and a truer validation.
469c2b2 to
d226f53
Compare
arulajmani
left a comment
There was a problem hiding this comment.
TFTR! bors r=ajstorm
Reviewable status:
pkg/ccl/multiregionccl/multiregionccltestutils/testutils.go, line 69 at r3 (raw file):
Previously, ajstorm (Adam Storm) wrote…
Nit: for expected partitions here, does it make sense to derive that from a SHOW REGIONS FROM DATABASE command? That would make this both easier to call, less error prone (hopefully), and a truer validation.
Done.
Multi-region tests check to ensure partitioning is sane from a few different tests. This pulls out that logic into its own function. Release note: None
This patch adds TestAlterRegionalByRowEnclosingRegionAddDrop, with the following sketch: - Client 1 performs an `ALTER TABLE ... SET LOCALITY REGIONAL BY ROW TABLE` - Let the user txn commit. - Block in the (table) schema changer. - Client 2 performs an ALTER ADD / DROP REGION. Let the operation complete (succeed or fail, depending on the particular setup). - Resume the ALTER operation in the schema changer. Currently, this ALTER operation is bound to fail because of Once the issue is addressed, this test should be updated to assert the correct partitioning values (in all cases). Release note: None
d226f53 to
5375f49
Compare
|
bors r+ |
|
Build succeeded: |
See individual commits for details.
There's more test scenarios I want to add where we sandwich an ADD/DROP region inside of an ALTER to REGIONAL BY ROW/or an operation on one of these tables, but here's a start. The error messages aren't wrapped yet, so they may appear a bit ugly in the jobs table. What this test ensures, which I think is quite valuable, is that the rollback is fine and no manual cleanup is required.