Merge #94771

94771: delegate: simplify / speed up SHOW FUNCTIONS; redefine several pg builtins as UDFs; fix pg_proc virtual index r=jordanlewis a=jordanlewis Closes #94746 Closes #94953 Remove a couple of unnecessary builtin function calls in SHOW FUNCTIONS. The query used to take 50 seconds, now it takes less than a second. Also, rewrite several pg compatibility builtins as UDFs for improved performance. Also, fix the pg_proc oid virtual index which didn't work for user-defined functions. Epic: None Co-authored-by: Jordan Lewis <[email protected]>
cockroachdb · Jan 15, 2023 · 04762b7 · 04762b7
2 parents 8866eca + ebe2686
commit 04762b7
Show file tree

Hide file tree

Showing 9 changed files with 511 additions and 445 deletions.
diff --git a/pkg/sql/delegate/show_functions.go b/pkg/sql/delegate/show_functions.go
@@ -55,8 +55,8 @@ func (d *delegator) delegateShowFunctions(n *tree.ShowFunctions) (tree.Statement
 	const getFunctionsQuery = `
 SELECT n.nspname as schema_name,
   p.proname as function_name,
-  pg_catalog.pg_get_function_result(p.oid) as result_data_type,
-  pg_catalog.pg_get_function_identity_arguments(p.oid) as argument_data_types,
+  p.prorettype::REGTYPE::TEXT as result_data_type,
+	COALESCE((SELECT trim('{}' FROM replace(array_agg(unnest(proargtypes)::REGTYPE::TEXT)::TEXT, ',', ', '))), '') as argument_data_types,
   CASE p.prokind
 	  WHEN 'a' THEN 'agg'
 	  WHEN 'w' THEN 'window'
@@ -72,6 +72,7 @@ FROM %[1]s.pg_catalog.pg_proc p
 LEFT JOIN %[1]s.pg_catalog.pg_namespace n ON n.oid = p.pronamespace
 WHERE true
 %[2]s
+GROUP BY 1, 2, 3, proargtypes, 5, 6
 ORDER BY 1, 2, 4;
 `
 	query := fmt.Sprintf(

diff --git a/pkg/sql/logictest/testdata/logic_test/pg_builtins b/pkg/sql/logictest/testdata/logic_test/pg_builtins
@@ -79,20 +79,26 @@ NULL
 statement ok
 CREATE TABLE is_visible(a int primary key);
 CREATE TYPE visible_type AS ENUM('a');
+CREATE FUNCTION visible_func() RETURNS INT LANGUAGE SQL AS $$ SELECT 1 $$;
 CREATE SCHEMA other;
 CREATE TABLE other.not_visible(a int primary key);
 CREATE TYPE other.not_visible_type AS ENUM('b');
+CREATE FUNCTION other.not_visible_func() RETURNS INT LANGUAGE SQL AS $$ SELECT 1 $$;
 CREATE DATABASE db2;
 SET DATABASE = db2;
 CREATE TABLE table_in_db2(a int primary key);
 CREATE TYPE type_in_db2 AS ENUM('c');
+CREATE FUNCTION func_in_db2() RETURNS INT LANGUAGE SQL AS $$ SELECT 1 $$;
 
 let $table_in_db2_id
 SELECT c.oid FROM pg_class c WHERE c.relname = 'table_in_db2';
 
 let $type_in_db2_id
 SELECT t.oid FROM pg_type t WHERE t.typname = 'type_in_db2';
 
+let $func_in_db2_id
+SELECT p.oid FROM pg_proc p WHERE p.proname = 'func_in_db2';
+
 statement ok
 SET DATABASE = test;
 
@@ -150,6 +156,31 @@ SELECT pg_type_is_visible(NULL)
 ----
 NULL
 
+query TB rowsort
+SELECT p.proname, pg_function_is_visible(p.oid)
+FROM pg_proc p
+WHERE p.proname IN ('array_length', 'visible_func', 'not_visible_func')
+----
+array_length      true
+visible_func      true
+not_visible_func  false
+
+# Looking up a function in a different database should return NULL.
+query B
+SELECT pg_function_is_visible($func_in_db2_id)
+----
+NULL
+
+# Looking up a non-existent OID should return NULL.
+query B
+SELECT pg_function_is_visible(1010101010)
+----
+NULL
+
+query B
+SELECT pg_function_is_visible(NULL)
+----
+NULL
 
 query TT
 SELECT pg_get_partkeydef(1), pg_get_partkeydef(NULL)
@@ -174,11 +205,11 @@ WHERE c.relname IN ('is_updatable', 'is_updatable_view', 'pg_class')
 ORDER BY c.oid, a.attnum
 ----
 relname            attname              oid         attnum  pg_relation_is_updatable  pg_column_is_updatable
-is_updatable       a                    120         1       28                        true
-is_updatable       b                    120         2       28                        true
-is_updatable       c                    120         3       28                        false
-is_updatable_view  a                    121         1       0                         false
-is_updatable_view  b                    121         2       0                         false
+is_updatable       a                    123         1       28                        true
+is_updatable       b                    123         2       28                        true
+is_updatable       c                    123         3       28                        false
+is_updatable_view  a                    124         1       0                         false
+is_updatable_view  b                    124         2       0                         false
 pg_class           oid                  4294967117  1       0                         false
 pg_class           relname              4294967117  2       0                         false
 pg_class           relnamespace         4294967117  3       0                         false

diff --git a/pkg/sql/logictest/testdata/logic_test/udf b/pkg/sql/logictest/testdata/logic_test/udf
@@ -174,6 +174,13 @@ FROM pg_catalog.pg_proc WHERE proname IN ('proc_f', 'proc_f_2');
 100119  proc_f    105  1546506610  14  true   true   false  i  2  25  25 20  {i,i}  {"",b}  SELECT 'hello';
 100121  proc_f_2  120  1546506610  14  false  false  false  v  1  25  25     {i}    NULL    SELECT 'hello';
 
+# Ensure that the pg_proc virtual index works properly.
+
+query TT
+SELECT oid, proname FROM pg_proc WHERE oid = 'sc.proc_f_2'::regproc
+----
+100121  proc_f_2
+
 statement ok
 USE defaultdb;
 

diff --git a/pkg/sql/opt/xform/testdata/external/liquibase b/pkg/sql/opt/xform/testdata/external/liquibase
diff --git a/pkg/sql/opt/xform/testdata/external/navicat b/pkg/sql/opt/xform/testdata/external/navicat
diff --git a/pkg/sql/pg_catalog.go b/pkg/sql/pg_catalog.go
@@ -2575,21 +2575,16 @@ https://www.postgresql.org/docs/9.5/catalog-pg-proc.html`,
 				coid := tree.MustBeDOid(unwrappedConstraint)
 				ooid := coid.Oid
 
-				name, overload, err := p.ResolveFunctionByOID(ctx, ooid)
-				if err != nil {
-					if errors.Is(err, tree.ErrFunctionUndefined) {
-						return false, nil //nolint:returnerrcheck
-					}
-					return false, err
-				}
-
 				if funcdesc.IsOIDUserDefinedFunc(ooid) {
-					fnDesc, err := p.Descriptors().ByID(p.Txn()).WithoutNonPublic().Get().Function(ctx, descpb.ID(overload.Oid))
+					fnDesc, err := p.Descriptors().ByID(p.Txn()).WithoutNonPublic().Get().Function(ctx, funcdesc.UserDefinedFunctionOIDToID(ooid))
 					if err != nil {
+						if errors.Is(err, tree.ErrFunctionUndefined) {
+							return false, nil //nolint:returnerrcheck
+						}
 						return false, err
 					}
 
-					scDesc, err := p.Descriptors().ByIDWithLeased(p.Txn()).WithoutNonPublic().Get().Schema(ctx, descpb.ID(ooid))
+					scDesc, err := p.Descriptors().ByIDWithLeased(p.Txn()).WithoutNonPublic().Get().Schema(ctx, fnDesc.GetParentSchemaID())
 					if err != nil {
 						return false, err
 					}
@@ -2604,7 +2599,15 @@ https://www.postgresql.org/docs/9.5/catalog-pg-proc.html`,
 					return true, nil
 
 				} else {
-					err := addPgProcBuiltinRow(name, addRow)
+					name, _, err := p.ResolveFunctionByOID(ctx, ooid)
+					if err != nil {
+						if errors.Is(err, tree.ErrFunctionUndefined) {
+							return false, nil //nolint:returnerrcheck
+						}
+						return false, err
+					}
+
+					err = addPgProcBuiltinRow(name, addRow)
 					if err != nil {
 						return false, err
 					}

diff --git a/pkg/sql/sem/builtins/BUILD.bazel b/pkg/sql/sem/builtins/BUILD.bazel
@@ -64,6 +64,7 @@ go_library(
         "//pkg/sql/lex",
         "//pkg/sql/lexbase",
         "//pkg/sql/memsize",
+        "//pkg/sql/oidext",
         "//pkg/sql/parser",
         "//pkg/sql/pgwire/pgcode",
         "//pkg/sql/pgwire/pgerror",

diff --git a/pkg/sql/sem/builtins/fixed_oids.go b/pkg/sql/sem/builtins/fixed_oids.go
@@ -1397,7 +1397,7 @@ var builtinOidsArray = []string{
 	1417: `pg_typeof(val: anyelement) -> string`,
 	1418: `pg_collation_for(str: anyelement) -> string`,
 	1419: `pg_get_userbyid(role_oid: oid) -> string`,
-	1420: `pg_sequence_parameters(sequence_oid: oid) -> string`,
+	1420: `pg_sequence_parameters(sequence_oid: oid) -> tuple{int AS start_value, int AS minimum_value, int AS maxmimum_value, int AS increment, bool AS cycle_option, int AS cache_size, oid AS data_type}`,
 	1421: `format_type(type_oid: oid, typemod: int) -> string`,
 	1422: `col_description(table_oid: oid, column_number: int) -> string`,
 	1423: `obj_description(object_oid: oid) -> string`,