sql: make pg_description aware of builtin function descriptions

This also extends the completion rules to properly handle
functions in multiple namespaces.

Release note (bug fix): `pg_catalog.pg_description` and
`pg_catalog.obj_description()` are now able to retrieve the
descriptive help for built-in functions.

pkg/cli/clisqlshell/testdata/complete/composite_names

@@ -10,7 +10,7 @@ complete 0 32
 msg: ""
 completions:
 - "functions":
-  "crdb_internal.force_error(" (This function is used only by CockroachDB's developers for testing purposes.) -> "crdb_internal.force_error(" (0, 25)
+  "crdb_internal.force_error(" (This function is used only by CockroachDB's developers for testing purposes) -> "crdb_internal.force_error(" (0, 25)
 
 sql
 create schema my_schema
@@ -34,7 +34,7 @@ complete 0 9
 msg: ""
 completions:
 - "functions":
-  "my_func(" () -> "my_func(" (0, 2)
+  "my_func(" ((from schema public) ) -> "my_func(" (0, 2)
 - "schema":
   "my_schema" () -> "my_schema" (0, 2)
 
@@ -54,8 +54,8 @@ complete 0 25
 msg: ""
 completions:
 - "functions":
-  "pg_catalog.array_length(" (Calculates the length of `input` on the provided `array_dimension`. However, bec) -> "pg_catalog.array_length(" (0, 18)
-  "pg_catalog.array_lower(" (Calculates the minimum value of `input` on the provided `array_dimension`. Howev) -> "pg_catalog.array_lower(" (0, 18)
+  "pg_catalog.array_length(" (Calculates the length of `input` on the provided `array_dimension`) -> "pg_catalog.array_length(" (0, 18)
+  "pg_catalog.array_lower(" (Calculates the minimum value of `input` on the provided `array_dimension`) -> "pg_catalog.array_lower(" (0, 18)
 
 complete
 select "PG_CATALOG".@

pkg/cli/clisqlshell/testdata/complete/functions

@@ -0,0 +1,35 @@
+sql
+CREATE FUNCTION lalala(val INT) RETURNS INT LANGUAGE SQL AS $$ SELECT val $$;
+CREATE FUNCTION lastval(val INT) RETURNS INT LANGUAGE SQL AS $$ SELECT val $$;
+----
+ok
+
+complete
+SELECT la@
+----
+complete 0 9
+msg: ""
+completions:
+- "functions":
+  "lag(" ((from schema pg_catalog) Returns `val` evaluated at the previous row within current row's partition; if t) -> "lag(" (0, 2)
+  "lalala(" ((from schema public) ) -> "lalala(" (0, 2)
+  "last_value(" ((from schema pg_catalog) Returns `val` evaluated at the row that is the last row of the window frame) -> "last_value(" (0, 2)
+  "lastval(" ((from schema pg_catalog) Return value most recently obtained with nextval in this session) -> "lastval(" (0, 2)
+  "lastval(" ((from schema public) ) -> "lastval(" (0, 2)
+- "keyword":
+  "LABEL" (unreserved) -> "LABEL" (0, 2)
+  "LANGUAGE" (unreserved) -> "LANGUAGE" (0, 2)
+  "LAST" (unreserved) -> "LAST" (0, 2)
+  "LATERAL" (reserved) -> "LATERAL" (0, 2)
+  "LATEST" (unreserved) -> "LATEST" (0, 2)
+
+complete
+SELECT pg_catalog.la@
+----
+complete 0 20
+msg: ""
+completions:
+- "functions":
+  "pg_catalog.lag(" (Returns `val` evaluated at the previous row within current row's partition; if t) -> "pg_catalog.lag(" (0, 13)
+  "pg_catalog.last_value(" (Returns `val` evaluated at the row that is the last row of the window frame) -> "pg_catalog.last_value(" (0, 13)
+  "pg_catalog.lastval(" (Return value most recently obtained with nextval in this session) -> "pg_catalog.lastval(" (0, 13)

pkg/cli/clisqlshell/testdata/complete/sql

@@ -10,16 +10,16 @@ completions:
   "postgres" () -> "postgres" (0, 0)
   "system" () -> "system" (0, 0)
 - "functions":
-  "_st_contains(" (Returns true if no points of geometry_b lie in the exterior of geometry_a, and t) -> "_st_contains(" (0, 0)
-  "_st_containsproperly(" (Returns true if geometry_b intersects the interior of geometry_a but not the bou) -> "_st_containsproperly(" (0, 0)
-  "_st_coveredby(" (Returns true if no point in geometry_a is outside geometry_b.) -> "_st_coveredby(" (0, 0)
-  "_st_covers(" (Returns true if no point in geometry_b is outside geometry_a.) -> "_st_covers(" (0, 0)
-  "_st_crosses(" (Returns true if geometry_a has some - but not all - interior points in common wi) -> "_st_crosses(" (0, 0)
-  "_st_dfullywithin(" (Returns true if every pair of points comprising geometry_a and geometry_b are wi) -> "_st_dfullywithin(" (0, 0)
-  "_st_dfullywithinexclusive(" (Returns true if every pair of points comprising geometry_a and geometry_b are wi) -> "_st_dfullywithinexclusive(" (0, 0)
-  "_st_dwithin(" (Returns true if any of geometry_a is within distance units of geometry_b, inclus) -> "_st_dwithin(" (0, 0)
-  "_st_dwithinexclusive(" (Returns true if any of geometry_a is within distance units of geometry_b, exclus) -> "_st_dwithinexclusive(" (0, 0)
-  "_st_equals(" (Returns true if geometry_a is spatially equal to geometry_b, i.e. ST_Within(geom) -> "_st_equals(" (0, 0)
+  "\"current_date\"(" ((from schema pg_catalog) Returns the date of the current transaction) -> "\"current_date\"(" (0, 0)
+  "\"current_schema\"(" ((from schema pg_catalog) Returns the current schema) -> "\"current_schema\"(" (0, 0)
+  "\"current_time\"(" ((from schema pg_catalog) Returns the current transaction's time with time zone) -> "\"current_time\"(" (0, 0)
+  "\"current_timestamp\"(" ((from schema pg_catalog) Returns the time of the current transaction) -> "\"current_timestamp\"(" (0, 0)
+  "\"current_user\"(" ((from schema pg_catalog) Returns the current user) -> "\"current_user\"(" (0, 0)
+  "\"extract\"(" ((from schema pg_catalog) Extracts `element` from `input`) -> "\"extract\"(" (0, 0)
+  "\"extract_duration\"(" ((from schema pg_catalog) Extracts `element` from `input`) -> "\"extract_duration\"(" (0, 0)
+  "\"family\"(" ((from schema pg_catalog) Extracts the IP family of the value; 4 for IPv4, 6 for IPv6) -> "\"family\"(" (0, 0)
+  "\"greatest\"(" ((from schema pg_catalog) Returns the element with the greatest value) -> "\"greatest\"(" (0, 0)
+  "\"information_schema._pg_char_max_length\"(" ((from schema pg_catalog) Not usable; exposed only for compatibility with PostgreSQL) -> "\"information_schema._pg_char_max_length\"(" (0, 0)
   ... entries omitted ...
 - "keyword":
   "ABORT" (unreserved) -> "ABORT" (0, 0)
@@ -50,7 +50,7 @@ completions:
 - "database":
   "defaultdb" () -> "defaultdb" (0, 6)
 - "functions":
-  "default_to_database_primary_region(" (Returns the given region if the region has been added to the current database.) -> "default_to_database_primary_region(" (0, 6)
+  "default_to_database_primary_region(" ((from schema pg_catalog) Returns the given region if the region has been added to the current database) -> "default_to_database_primary_region(" (0, 6)
 - "keyword":
   "DEFAULT" (reserved) -> "DEFAULT" (0, 6)
   "DEFAULTS" (unreserved) -> "DEFAULTS" (0, 6)
@@ -110,5 +110,5 @@ complete 0 14
 msg: ""
 completions:
 - "functions":
-  "array_length(" (Calculates the length of `input` on the provided `array_dimension`. However, bec) -> "array_length(" (0, 7)
-  "array_lower(" (Calculates the minimum value of `input` on the provided `array_dimension`. Howev) -> "array_lower(" (0, 7)
+  "array_length(" ((from schema pg_catalog) Calculates the length of `input` on the provided `array_dimension`) -> "array_length(" (0, 7)
+  "array_lower(" ((from schema pg_catalog) Calculates the minimum value of `input` on the provided `array_dimension`) -> "array_lower(" (0, 7)

pkg/sql/comment_on_table_test.go

@@ -39,17 +39,17 @@ func TestCommentOnTable(t *testing.T) {
 		}{
 			{
 				`COMMENT ON TABLE t IS 'foo'`,
-				`SELECT obj_description('t'::regclass)`,
+				`SELECT obj_description('t'::regclass, 'pg_class')`,
 				gosql.NullString{String: `foo`, Valid: true},
 			},
 			{
 				`TRUNCATE t`,
-				`SELECT obj_description('t'::regclass)`,
+				`SELECT obj_description('t'::regclass, 'pg_class')`,
 				gosql.NullString{String: `foo`, Valid: true},
 			},
 			{
 				`COMMENT ON TABLE t IS NULL`,
-				`SELECT obj_description('t'::regclass)`,
+				`SELECT obj_description('t'::regclass, 'pg_class')`,
 				gosql.NullString{Valid: false},
 			},
 		}

pkg/sql/comprules/rules.go

@@ -139,29 +139,34 @@ func completeFunction(ctx context.Context, c compengine.Context) (compengine.Row
 	}
 
 	c.Trace("completing for %q (%d,%d) with schema %q", prefix, start, end, schemaName)
-	// TODO(knz): use the comment extraction functions from pg_catalog
-	// instead of crdb_internal. This requires exposing comments for
-	// built-in functions through pg_catalog.
+	// Note: we use min(p.oid) ... GROUP BY p.proname to cover the case
+	// there are multiple overloads. This ensures we have only one entry
+	// in the completion results for that function. Its reported
+	// description will also be the description for the first overload.
+	// Separately, we GROUP BY n.nspname to ensure that a UDF with
+	// the same name as a pg_catalog function gets reported as a
+	// separate completion entry.
 	const query = `
 WITH p AS (
+SELECT min(p.oid) AS oid, p.proname, n.nspname
+  FROM pg_catalog.pg_proc p
+  JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace
+ WHERE left(p.proname, length($1:::STRING)) = $1:::STRING
+ AND ((length($4) > 0 AND $4 = n.nspname)
+   OR (length($4) = 0 AND n.nspname = ANY current_schemas(true)))
+GROUP BY p.proname, n.nspname
+)
 SELECT DISTINCT
-       proname, nspname
-  FROM pg_catalog.pg_proc
-  JOIN pg_catalog.pg_namespace n ON n.oid = pronamespace)
-SELECT IF(length($4) > 0, pg_catalog.quote_ident($4:::STRING) || '.', '') ||
+       IF(length($4) > 0, pg_catalog.quote_ident($4:::STRING) || '.', '') ||
        pg_catalog.quote_ident(proname) || '(' AS completion,
        'functions' AS category,
-       substr(COALESCE((
-         SELECT details
-          FROM "".crdb_internal.builtin_functions f2
-         WHERE f2.function = p.proname AND f2.schema = p.nspname
-         LIMIT 1), ''), e'[^\n]{0,80}') AS description,
+       IF(length($4) = 0, '(from schema '||nspname||') ', '') ||
+       substr(COALESCE(pg_catalog.obj_description(oid, 'pg_proc'),''), e'[^.\n]{0,80}') AS description,
        $2:::INT AS start,
        $3:::INT AS end
   FROM p
- WHERE left(proname, length($1:::STRING)) = $1:::STRING
- AND ((length($4) > 0 AND $4 = nspname)
-   OR (length($4) = 0 AND nspname = ANY current_schemas(true)))`
+ORDER BY 1,2,3,4,5
+`
 	iter, err := c.Query(ctx, query, prefix, start, end, schemaName)
 	return iter, err
 }