Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis)

pkg/sql/delegate/show_functions.go line 75 at r1 (raw file):

WHERE true
%[2]s
GROUP BY n.nspname, p.proname, p.prorettype, p.prokind, p.provolatile

why do we need the GROUP BY? also, could we just do group by 1, 2, 3, 4

pkg/sql/sem/builtins/pg_builtins.go line 274 at r2 (raw file):

		FROM pg_catalog.pg_matviews v
		JOIN pg_catalog.pg_class c ON c.relname=v.matviewname
		WHERE c.oid=$1`,

looks like this should return NULL when there's no view with that OID. does this new version do that, or return 0 rows? (and is it already tested?)

pkg/sql/sem/builtins/pg_builtins.go line 287 at r2 (raw file):

		IsUDF:      true,
		Body: `SELECT COALESCE((SELECT condef FROM pg_catalog.pg_constraint WHERE oid=$1),
													  'unknown constraint (OID=' || $1 || ')')`,

is this right? instead of returning an error, this will return a string row.

also, it seems like the PG behavior is that null is returned in this case, so i'm not sure why the old code returned a pgerror

pkg/sql/sem/builtins/pg_builtins.go line 653 at r2 (raw file):

			ReturnType: tree.FixedReturnType(types.String),
			IsUDF:      true,
			Body: `SELECT trim('{}' FROM

nit: could you a comment saying how it's getting reformatted (like you added below)

pkg/sql/sem/builtins/pg_builtins.go line 873 at r2 (raw file):

		tree.Overload{
			Types:      tree.ParamTypes{{Name: "sequence_oid", Typ: types.Oid}},
			ReturnType: tree.FixedReturnType(types.String),

can the return type change to a record now?

pkg/sql/sem/builtins/pg_builtins.go line 877 at r2 (raw file):

			Body: `SELECT COALESCE (((seqstart, seqmin, seqmax, seqincrement, seqcycle, seqcache, seqtypeid)
             FROM pg_catalog.pg_sequence WHERE seqrelid=$1 LIMIT 1),
             'unknown sequence (OID=' || $1 || ')')`,

hm in PG this one actually returns an error, but now it will just return this message in the results

crazy how the pg_* builtins are inconsistent about how they behave with not found entities

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

pkg/sql/delegate/show_functions.go line 75 at r1 (raw file):

The presence of the array_agg requires the group by. Okay, that's a better pattern.

pkg/sql/sem/builtins/pg_builtins.go line 274 at r2 (raw file):

Returning 0 rows here is the same as returning NULL - all of the builtin functions like this one behave like that

pkg/sql/sem/builtins/pg_builtins.go line 287 at r2 (raw file):

Done.

pkg/sql/sem/builtins/pg_builtins.go line 873 at r2 (raw file):

Done.

pkg/sql/sem/builtins/pg_builtins.go line 877 at r2 (raw file):

Done.

cool!

Reviewed 1 of 2 files at r3, 2 of 3 files at r4, 1 of 1 files at r5.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis and @rharding6373)

pkg/sql/sem/builtins/pg_builtins.go line 1109 at r3 (raw file):

	),

	"obj_description": makeBuiltin(defProps(),

obj_description and col_description are also great candidates for this rewrite. we've seen both come up with performance issues in the past

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis and @rharding6373)

pkg/sql/sem/builtins/pg_builtins.go line 1109 at r3 (raw file):

#87947

but @mgartner is the UDF expert so I've added him to take a look.

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @jordanlewis, @mgartner, and @rafiss)

pkg/sql/sem/builtins/pg_builtins.go line 887 at r6 (raw file):

		// The real implementation returns a record; we fake it by returning a
		// comma-delimited string enclosed by parentheses.
		// TODO(jordan): convert this to return a record type once we support that.

nit: is this TODO obsolete now?

pkg/sql/sem/builtins/pg_builtins.go line 1003 at r6 (raw file):

			FROM pg_catalog.pg_description
			WHERE objoid=$1
		  AND objsubid = 0

nit: this body and the next look like they have odd whitespace formatting

assuming this fixes #94953 could you add a test in pkg/sql/logictest/testdata/logic_test/pg_builtins for pg_function_is_visible? see the analagous tests for pg_type_is_visible

Cool! LGTM