sql: support SHOW GRANTS for functions

[chengxiong-ruan]

Backport resolves #88495

Release note (sql change): Previously SHOW GRANTS only supports
db, schema, table and types. This commit add supports for UDFs,
so that SHOW GRANTS returns UDFs privileges infos, and statements
like SHOW GRANTS ON FUNCTION <udf name/signatures> are now supported
Full function signature must be provided if the function name is
not unique.
Release justification: low risk GA blocker.

[cockroach-teamcity]

This change is 

[chengxiong-ruan]

I'm not including any builtin functions cause thinking it would just explode the SHOW GRANTS output without adding much user experience value since anyone can execute a builtin.

[rafiss]

look pretty good! just some small comments

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

---

pkg/sql/delegate/show_grants.go line 105 at r1 (raw file):

       )
`
	//

missing comment?

---

pkg/sql/delegate/show_grants.go line 116 at r1 (raw file):

         is_grantable::boolean
  FROM "".information_schema.role_routine_grants
  WHERE reverse(split_part(reverse(specific_name), '_', 1))::INT > 100000

could you make the 100000 reference CockroachPredefinedOIDMax just in case that constant ever changes?

---

pkg/sql/logictest/testdata/logic_test/udf line 627 at r1 (raw file):

query TTTTTTTTTT colnames
SELECT * FROM information_schema.role_routine_grants

is it useful to still keep the test on infromation_schema to make sure that metadata is correct too?

---

pkg/sql/logictest/testdata/logic_test/udf line 634 at r1 (raw file):

query TTTTTTB colnames
SHOW GRANTS ON FUNCTION test_priv_f1, test_priv_f2, test_priv_f3

hm, i am a bit confused. how did this work without any change to sql.y to support ON FUNCTION?

[chengxiong-ruan]

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

---

pkg/sql/delegate/show_grants.go line 105 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

missing comment?

fixed. good eyes.

---

pkg/sql/delegate/show_grants.go line 116 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

could you make the 100000 reference CockroachPredefinedOIDMax just in case that constant ever changes?

fixed, use CRDB_MAX_PREDEFINED_OID as a pattern in the query string to replace with and added a helper function to get the actual query.

---

pkg/sql/logictest/testdata/logic_test/udf line 627 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

is it useful to still keep the test on infromation_schema to make sure that metadata is correct too?

good point, I added them back.

---

pkg/sql/logictest/testdata/logic_test/udf line 634 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

hm, i am a bit confused. how did this work without any change to sql.y to support ON FUNCTION?

yeah, FUNCTION is one of object types in the grant_targets list :

cockroach/pkg/sql/parser/sql.y

Line 7812 in ae38d90

| FUNCTION function_with_argtypes_list

[rafiss]

nice!

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

---

pkg/sql/delegate/show_grants.go line 116 at r1 (raw file):

Previously, chengxiong-ruan (Chengxiong Ruan) wrote…

fixed, use CRDB_MAX_PREDEFINED_OID as a pattern in the query string to replace with and added a helper function to get the actual query.

i'll leave it up to you, but did you find that more readable than

var udfQuery = fmt.Sprintf(..., oidext.CockroachPredefinedOIDMax)

just want to make sure you considered that

---

pkg/sql/logictest/testdata/logic_test/udf line 634 at r1 (raw file):

Previously, chengxiong-ruan (Chengxiong Ruan) wrote…

yeah, FUNCTION is one of object types in the grant_targets list :

cockroach/pkg/sql/parser/sql.y

Line 7812 in ae38d90

| FUNCTION function_with_argtypes_list

ah thanks for explaining

[chengxiong-ruan]

pkg/sql/delegate/show_grants.go line 116 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

i'll leave it up to you, but did you find that more readable than

var udfQuery = fmt.Sprintf(..., oidext.CockroachPredefinedOIDMax)

just want to make sure you considered that

yeah, for some reason part of me wanted to be consistent with the other query strings being constants, but I guess that's not important. I'll change it to formatting. Thanks.

[chengxiong-ruan]

TFTR!
bors r+

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)