mod the nits

Reviewed 2 of 6 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @ajwerner and @chengxiong-ruan)

pkg/sql/crdb_internal.go line 2594 at r1 (raw file):

`,
	populate: func(ctx context.Context, p *planner, db catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) error {
		flags := tree.ObjectLookupFlags{CommonLookupFlags: tree.CommonLookupFlags{AvoidLeased: true}}

I think this is only reasonable to do because of the work @jasonmchan did such that by the time we're here, we have every descriptor 🎉 . I also think it means that you'd end up using a read descriptor rather than a leased one anyway. I guess one thing that might be cool would be to add an rttyanalysis test for a query against this table.

pkg/sql/create_function.go line 195 at r1 (raw file):

		}
		udfMutableDesc.DependsOnTypes = append(udfMutableDesc.DependsOnTypes, id)
	}

Use catalog.DescIDSet and build the first set and difference the second and then call Ordered?

Code quote:

	udfDesc.DependsOnTypes = []descpb.ID{}
	for id := range n.typeDeps {
		if implicitTypeTblIDs.Contains(int(id)) {
			continue
		}
		udfDesc.DependsOnTypes = append(udfDesc.DependsOnTypes, id)

pkg/sql/create_function.go line 299 at r1 (raw file):

	// tables used directly in function body or as implicit types.
	backrefTblIDs := util.MakeFastIntSet()
	implicitTypeTblIDs := util.MakeFastIntSet()

catalog.DescIDSet?

Code quote:

	backrefTblIDs := util.MakeFastIntSet()
	implicitTypeTblIDs := util.MakeFastIntSet()

pkg/sql/create_function.go line 376 at r1 (raw file):

	// Add forward references to UDF descriptor.
	udfDesc.DependsOn = make([]descpb.ID, 0, backrefTblIDs.Len())

udfDesc.DependsOn = backrefTblIDs.Ordered()?

Code quote:

	udfDesc.DependsOn = make([]descpb.ID, 0, backrefTblIDs.Len())
	backrefTblIDs.ForEach(func(id int) {
		udfDesc.DependsOn = append(udfDesc.DependsOn, descpb.ID(id))
	})

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @ajwerner and @jasonmchan)

pkg/sql/crdb_internal.go line 2594 at r1 (raw file):

Oh yeah, that's awesome work! Though I didn't do ReadAllDescriptors() here, should I do that? Do you think it's an overkill here given the number of schema and function could be low?

pkg/sql/create_function.go line 195 at r1 (raw file):

that saved my life, fixed, thanks!

pkg/sql/create_function.go line 299 at r1 (raw file):

ah, thanks, I kept forgetting this. fixed

pkg/sql/create_function.go line 376 at r1 (raw file):

yes, with ID set, this is much nicer. fixed

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @ajwerner and @jasonmchan)

pkg/sql/crdb_internal.go line 2594 at r1 (raw file):

Oh, I suppose that that's right. I don't know. It's one of those things where it's really painful if the nodes are far apart. I'm hopeful in 23.1 to adopt global tables, but for now it's going to be O(functions) round-trips which might be very slow. One approach might be to collect the IDs in one pass and then fetch them as a batch and then iterate separately.