[WIP] sql: partition REGIONAL BY ROW on all indexes during region changes

[otan]

See individual commits for details.

Refs: #63462

[cockroach-teamcity]

This change is 

[otan]

thought about this more and now i think this is the wrong way to go about it. we should instead repartition after index creation/drop (even during failures). doing this on AllIndexes has a chance of e.g. for RBR->GLOBAL making partitions on the new indexes of the global table.

[ajstorm]

Added a few comments. Would like @arulajmani to have a look too though, as I'm still pretty new to these concurrent tests in go.

Reviewed 3 of 3 files at r1, 2 of 2 files at r2.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @otan)

---

pkg/ccl/multiregionccl/region_test.go, line 355 at r2 (raw file):

}

// TestDropIndexFailureDuringAddOrDropRegion a DROP INDEX failing during

Nit: TestDropIndexFailureDuringAddOrDropRegion tests a DROP INDEX...

---

pkg/ccl/multiregionccl/region_test.go, line 357 at r2 (raw file):

// TestDropIndexFailureDuringAddOrDropRegion a DROP INDEX failing during
// ADD/DROP REGION, in which case the dropped index should still contain
// the new indexes.

Nit: ...should still contain the proper set of regions?

---

pkg/ccl/multiregionccl/region_test.go, line 383 at r2 (raw file):

				SQLSchemaChanger: &sql.SchemaChangerTestingKnobs{
					RunBeforeBackfill: func() error {
						if block {

Do we need a mutex above this to ensure that we don't get both racers in this block (in which case the test would hang)?

---

pkg/ccl/multiregionccl/region_test.go, line 405 at r2 (raw file):

			require.NoError(t, err)

			block = true

I'm confused as to why we need to set this to true down here. Can't we just initialize it to true above? Is there any chance that the DROP/CREATE/CREATE sequence above will invoke the schema changer?

---

pkg/ccl/multiregionccl/region_test.go, line 421 at r2 (raw file):

			wg.Add(1)

			_, err = sqlDB.Exec(tc.regionChangeSQL)

Forgive me if this comment is noise, as I'm still pretty new to writing these types of concurrent go tests, but shouldn't we wait for the above go routine to make it to the type schema changer before we run this? As written, isn't it possible that the ADD/DROP region statement could make it into the error path before the DROP INDEX statement does?

[ajstorm]

Gah! And now I notice the WIP and closed 🤦

[otan]

haha sorry, not sure whether this should go through...