Reviewed 54 of 146 files at r1.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt, @fqazi, and @postamar)

pkg/sql/catalog/descs/descriptor.go, line 297 at r1 (raw file):

	descs, err = tc.withReadFromStore(mutable, func() ([]catalog.MutableDescriptor, error) {
		uncommittedDB, _ := tc.uncommitted.getByID(parentID).(catalog.DatabaseDescriptor)
		version := tc.settings.Version.ActiveVersion(ctx)

shouldn't this use the version we already initialized and stored in the collection?

pkg/sql/catalog/desctestutils/descriptor_test_utils.go, line 35 at r1 (raw file):

// TestingGetDatabaseDescriptor retrieves a database descriptor directly from
// the kv layer.
func TestingGetDatabaseDescriptor(

Having reviewed a lot of this change, I wonder to what extent this plumbing here is making things a lot more painful. Should we make a variant which takes a version and just hard-code it to the latest version in the existing variant and plumb that through to the new variant? It feels like it would reduce the number of changes dramatically.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner, @dt, and @postamar)

pkg/sql/catalog/desctestutils/descriptor_test_utils.go, line 35 at r1 (raw file):

Done.

Made this the default behaviour which reduces the number of code changes by a lot.

Reviewed 22 of 146 files at r1, 1 of 1 files at r2, 80 of 108 files at r3, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @ajwerner, @dt, @fqazi, and @postamar)

pkg/ccl/changefeedccl/changefeed_stmt.go, line 897 at r3 (raw file):

	ctx context.Context, execCfg *sql.ExecutorConfig, txn *kv.Txn, desc catalog.TableDescriptor,
) (string, error) {
	col := execCfg.CollectionFactory.MakeCollection(ctx, nil)

nit: add back comment

pkg/ccl/changefeedccl/rowfetcher_cache.go, line 74 at r3 (raw file):

		codec:      codec,
		leaseMgr:   leaseMgr,
		collection: cf.NewCollection(nil /* TemporarySchemaProvider */),

nit: add back comment

pkg/cli/doctor.go, line 96 at r3 (raw file):

that may not exist on downlevel versions.
`,
		Args: cobra.MinimumNArgs(1),

nit: cobra.RangeArgs(1, 2)

pkg/sql/create_view.go, line 209 at r3 (raw file):

		// currently relied on in import and restore code and tests.
		var creationTime hlc.Timestamp
		desc, err := makeViewTableDesc(params.ctx, viewName, n.viewQuery, n.dbDesc.GetID(), schema.GetID(), id, n.columns, creationTime, privs, &params.p.semaCtx, params.p.EvalContext(), params.p.EvalContext().Settings, n.persistence, n.dbDesc.IsMultiRegion(), params.p)

nit: consider wrapping this

pkg/sql/temporary_schema.go, line 524 at r3 (raw file):

	// Build a set of all databases with temporary objects.
	var allDbDescs []catalog.DatabaseDescriptor
	descsCol := c.collectionFactory.NewCollection(ctx, nil)

nit: add back comment

pkg/sql/virtual_schema.go, line 239 at r3 (raw file):

) (descpb.TableDescriptor, error) {
	stmt, err := parser.ParseOne(v.schema)

nit: newline

pkg/sql/virtual_schema.go, line 254 at r3 (raw file):

		create.Name.Table(),
		tree.AsStringWithFlags(create.AsSource, tree.FmtParsable),
		0, /* parentID */

nit: add back comments