schemachanger: port over MR zone config helpers

[annrpom]

Some precursor work to remove RBR fallbacks for ADD COLUMN.

Epic: CRDB-31282
Informs: #80545

Release note: None

[blathers-crl]

Your pull request contains more than 1000 changes. It is strongly encouraged to split big PRs into smaller chunks.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[cockroach-teamcity]

This change is 

[github-actions]

⚪ Sysbench [SQL, 3node, oltp_read_write]

Metric	Old Commit	New Commit	Delta	Note	Threshold
⚪ sec/op	10.89m ±1%	10.88m ±1%	~	p=0.436 n=10	3.0%
⚪ errs/op	0.000 ±0%	0.000 ±0%	~	p=1.000 n=10	0.0%
⚪ allocs/op	10.20k ±0%	10.17k ±1%	~	p=0.255 n=10	2.0%
⚪ B/op	2.203Mi ±0%	2.203Mi ±0%	~	p=0.725 n=10	2.0%

Reproduce

benchdiff binaries:

mkdir -p benchdiff/7d6d787/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/7d6d787c7f43c40fd0b1825a93cfd0141f5188c8/bin/pkg_sql_tests benchdiff/7d6d787/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/7d6d787/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
mkdir -p benchdiff/09d4505/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/09d4505340c8e8399bade0da777dd1d4ae0ebc62/bin/pkg_sql_tests benchdiff/09d4505/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/09d4505/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests

benchdiff command:

benchdiff --run=^BenchmarkSysbench/SQL/3node/oltp_read_write$ --old=09d4505 --new=7d6d787 ./pkg/sql/tests

⚪ Sysbench [KV, 1node, local, oltp_read_only]

Metric	Old Commit	New Commit	Delta	Note	Threshold
⚪ sec/op	711.0µ ±1%	707.3µ ±1%	~	p=0.247 n=10	2.0%
⚪ errs/op	0.000 ±0%	0.000 ±0%	~	p=1.000 n=10	0.0%
⚪ allocs/op	446.0 ±0%	446.0 ±0%	~	p=1.000 n=10	1.5%
⚪ B/op	254.8Ki ±0%	254.7Ki ±0%	~	p=0.592 n=10	1.5%

Reproduce

benchdiff binaries:

mkdir -p benchdiff/7d6d787/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/7d6d787c7f43c40fd0b1825a93cfd0141f5188c8/bin/pkg_sql_tests benchdiff/7d6d787/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/7d6d787/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
mkdir -p benchdiff/09d4505/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/09d4505340c8e8399bade0da777dd1d4ae0ebc62/bin/pkg_sql_tests benchdiff/09d4505/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/09d4505/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests

benchdiff command:

benchdiff --run=^BenchmarkSysbench/KV/1node_local/oltp_read_only$ --old=09d4505 --new=7d6d787 ./pkg/sql/tests

⚪ Sysbench [KV, 1node, local, oltp_write_only]

Metric	Old Commit	New Commit	Delta	Note	Threshold
⚪ sec/op	1.416m ±0%	1.417m ±0%	~	p=0.796 n=10	2.5%
⚪ errs/op	0.000 ±0%	0.000 ±0%	~	p=1.000 n=10	0.0%
⚪ allocs/op	1.398k ±0%	1.399k ±0%	~	p=0.650 n=10	1.8%
⚪ B/op	290.1Ki ±0%	290.4Ki ±1%	~	p=0.105 n=10	1.8%

Reproduce

benchdiff binaries:

mkdir -p benchdiff/7d6d787/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/7d6d787c7f43c40fd0b1825a93cfd0141f5188c8/bin/pkg_sql_tests benchdiff/7d6d787/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/7d6d787/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
mkdir -p benchdiff/09d4505/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/09d4505340c8e8399bade0da777dd1d4ae0ebc62/bin/pkg_sql_tests benchdiff/09d4505/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/09d4505/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests

benchdiff command:

benchdiff --run=^BenchmarkSysbench/KV/1node_local/oltp_write_only$ --old=09d4505 --new=7d6d787 ./pkg/sql/tests

Artifacts

download:

mkdir -p new
gcloud storage cp gs://cockroach-microbench-ci/artifacts/7d6d787c7f43c40fd0b1825a93cfd0141f5188c8/13462004557-1/\* new/
mkdir -p old
gcloud storage cp gs://cockroach-microbench-ci/artifacts/09d4505340c8e8399bade0da777dd1d4ae0ebc62/13462004557-1/\* old/

Legend

• ⚪ Neutral: No significant performance change.
• 🟡 Warning: Slight degradation, likely due to variance, but still within thresholds.
• 🔴 Regression: Likely performance regression, requiring investigation.
• 🟢 Improvement: Possible performance gain.

No regressions detected!

built with commit: 7d6d787c7f43c40fd0b1825a93cfd0141f5188c8

[spilchen]

Reviewed 6 of 7 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @annrpom and @rafiss)

---

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/zone_config_helpers.go line 1260 at r1 (raw file):

	return b.QueryByID(tableID).FilterTemporaryIndex().
		Filter(func(_ scpb.Status, _ scpb.TargetStatus, e *scpb.TemporaryIndex) bool {
			return e.IndexID == indexID+1

Could we also lookup by checking e.SourceIndexID? That should be set for the index this temporary index was created for.

---

pkg/sql/regions/region_util.go line 0 at r1 (raw file):
Is this just a straight copy and rename from the pkg/sql/region_util.go? Or were there any logic changes?

[annrpom]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rafiss and @spilchen)

---

pkg/sql/regions/region_util.go line at r1 (raw file):

Previously, spilchen wrote…

Is this just a straight copy and rename from the pkg/sql/region_util.go? Or were there any logic changes?

straight copy and rename! done to resolve some dependency cycles; i just moved these to a new commit in this PR

---

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/zone_config_helpers.go line 1260 at r1 (raw file):

Previously, spilchen wrote…

Could we also lookup by checking e.SourceIndexID? That should be set for the index this temporary index was created for.

done

[spilchen]

Reviewed 2 of 2 files at r2, 1 of 2 files at r3, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rafiss)

---

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/zone_config_helpers.go line 1394 at r3 (raw file):

// ApplyZoneConfigForMultiRegionTable applies zone config settings based
// on the options provided and adds the scpb.TableZoneConfig to our builder.
func ApplyZoneConfigForMultiRegionTable(

nit: Maybe this will make sense in a follow-on PR, but will we need to export this function out of this package?

---

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/zone_config_helpers.go line 1482 at r3 (raw file):

		newZoneConfig.SubzoneSpans = nil
	}
	if newZoneConfig.IsSubzonePlaceholder() && len(newZoneConfig.Subzones) == 0 {

Is the placeholder subzone never materialized in the scpb elements?

---

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/zone_config_helpers.go line 1486 at r3 (raw file):

	}
	tzc := &scpb.TableZoneConfig{TableID: tableID, ZoneConfig: &newZoneConfig, SeqNum: 1}
	b.Add(tzc)

Do we need to delete the old table zone config? Or will that be handled by the caller in another PR.