Add prokind column, replacing proisagg and proiswindow

The new column distinguishes normal functions, procedures, aggregates,
and window functions.  This replaces the existing columns proisagg and
proiswindow, and replaces the convention that procedures are indicated
by prorettype == 0.  Also change prorettype to be VOIDOID for procedures.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Michael Paquier <michael@paquier.xyz>

Author: petere

doc/src/sgml/catalogs.sgml

@@ -5062,15 +5062,17 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
   </indexterm>
 
   <para>
-   The catalog <structname>pg_proc</structname> stores information about functions (or procedures).
-   See <xref linkend="sql-createfunction"/>
-   and <xref linkend="xfunc"/> for more information.
+   The catalog <structname>pg_proc</structname> stores information about
+   functions, procedures, aggregate functions, and window functions
+   (collectively also known as routines).  See <xref
+   linkend="sql-createfunction"/>, <xref linkend="sql-createprocedure"/>, and
+   <xref linkend="xfunc"/> for more information.
   </para>
 
   <para>
-   The table contains data for aggregate functions as well as plain functions.
-   If <structfield>proisagg</structfield> is true, there should be a matching
-   row in <structfield>pg_aggregate</structfield>.
+   If <structfield>prokind</structfield> indicates that the entry is for an
+   aggregate function, there should be a matching row in
+   <structfield>pg_aggregate</structfield>.
   </para>
 
   <table>
@@ -5157,17 +5159,12 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
      </row>
 
      <row>
-      <entry><structfield>proisagg</structfield></entry>
-      <entry><type>bool</type></entry>
-      <entry></entry>
-      <entry>Function is an aggregate function</entry>
-     </row>
-
-     <row>
-      <entry><structfield>proiswindow</structfield></entry>
-      <entry><type>bool</type></entry>
+      <entry><structfield>prokind</structfield></entry>
+      <entry><type>char</type></entry>
       <entry></entry>
-      <entry>Function is a window function</entry>
+      <entry><literal>f</literal> for a normal function, <literal>p</literal>
+      for a procedure, <literal>a</literal> for an aggregate function, or
+      <literal>w</literal> for a window function</entry>
      </row>
 
      <row>
@@ -5264,7 +5261,7 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
       <entry><structfield>prorettype</structfield></entry>
       <entry><type>oid</type></entry>
       <entry><literal><link linkend="catalog-pg-type"><structname>pg_type</structname></link>.oid</literal></entry>
-      <entry>Data type of the return value, or null for a procedure</entry>
+      <entry>Data type of the return value</entry>
      </row>
 
      <row>

src/backend/catalog/aclchk.c

@@ -830,21 +830,17 @@ objectsInSchemaToOids(ObjectType objtype, List *nspnames)
 								BTEqualStrategyNumber, F_OIDEQ,
 								ObjectIdGetDatum(namespaceId));
 
-					/*
-					 * When looking for functions, check for return type <>0.
-					 * When looking for procedures, check for return type ==0.
-					 * When looking for routines, don't check the return type.
-					 */
 					if (objtype == OBJECT_FUNCTION)
+						/* includes aggregates and window functions */
 						ScanKeyInit(&key[keycount++],
-									Anum_pg_proc_prorettype,
-									BTEqualStrategyNumber, F_OIDNE,
-									InvalidOid);
+									Anum_pg_proc_prokind,
+									BTEqualStrategyNumber, F_CHARNE,
+									CharGetDatum(PROKIND_PROCEDURE));
 					else if (objtype == OBJECT_PROCEDURE)
 						ScanKeyInit(&key[keycount++],
-									Anum_pg_proc_prorettype,
-									BTEqualStrategyNumber, F_OIDEQ,
-									InvalidOid);
+									Anum_pg_proc_prokind,
+									BTEqualStrategyNumber, F_CHAREQ,
+									CharGetDatum(PROKIND_PROCEDURE));
 
 					rel = heap_open(ProcedureRelationId, AccessShareLock);
 					scan = heap_beginscan_catalog(rel, keycount, key);

src/backend/catalog/information_schema.sql

@@ -1413,7 +1413,7 @@ CREATE VIEW routines AS
            CAST(current_database() AS sql_identifier) AS routine_catalog,
            CAST(n.nspname AS sql_identifier) AS routine_schema,
            CAST(p.proname AS sql_identifier) AS routine_name,
-           CAST(CASE WHEN p.prorettype <> 0 THEN 'FUNCTION' ELSE 'PROCEDURE' END
+           CAST(CASE p.prokind WHEN 'f' THEN 'FUNCTION' WHEN 'p' THEN 'PROCEDURE' END
              AS character_data) AS routine_type,
            CAST(null AS sql_identifier) AS module_catalog,
            CAST(null AS sql_identifier) AS module_schema,
@@ -1423,7 +1423,7 @@ CREATE VIEW routines AS
            CAST(null AS sql_identifier) AS udt_name,
 
            CAST(
-             CASE WHEN p.prorettype = 0 THEN NULL
+             CASE WHEN p.prokind = 'p' THEN NULL
                   WHEN t.typelem <> 0 AND t.typlen = -1 THEN 'ARRAY'
                   WHEN nt.nspname = 'pg_catalog' THEN format_type(t.oid, null)
                   ELSE 'USER-DEFINED' END AS character_data)
@@ -1442,14 +1442,14 @@ CREATE VIEW routines AS
            CAST(null AS cardinal_number) AS datetime_precision,
            CAST(null AS character_data) AS interval_type,
            CAST(null AS cardinal_number) AS interval_precision,
-           CAST(CASE WHEN p.prorettype <> 0 THEN current_database() END AS sql_identifier) AS type_udt_catalog,
+           CAST(CASE WHEN nt.nspname IS NOT NULL THEN current_database() END AS sql_identifier) AS type_udt_catalog,
            CAST(nt.nspname AS sql_identifier) AS type_udt_schema,
            CAST(t.typname AS sql_identifier) AS type_udt_name,
            CAST(null AS sql_identifier) AS scope_catalog,
            CAST(null AS sql_identifier) AS scope_schema,
            CAST(null AS sql_identifier) AS scope_name,
            CAST(null AS cardinal_number) AS maximum_cardinality,
-           CAST(0 AS sql_identifier) AS dtd_identifier,
+           CAST(CASE WHEN p.prokind <> 'p' THEN 0 END AS sql_identifier) AS dtd_identifier,
 
            CAST(CASE WHEN l.lanname = 'sql' THEN 'SQL' ELSE 'EXTERNAL' END AS character_data)
              AS routine_body,
@@ -1464,7 +1464,7 @@ CREATE VIEW routines AS
            CAST('GENERAL' AS character_data) AS parameter_style,
            CAST(CASE WHEN p.provolatile = 'i' THEN 'YES' ELSE 'NO' END AS yes_or_no) AS is_deterministic,
            CAST('MODIFIES' AS character_data) AS sql_data_access,
-           CAST(CASE WHEN p.prorettype <> 0 THEN
+           CAST(CASE WHEN p.prokind <> 'p' THEN
              CASE WHEN p.proisstrict THEN 'YES' ELSE 'NO' END END AS yes_or_no) AS is_null_call,
            CAST(null AS character_data) AS sql_path,
            CAST('YES' AS yes_or_no) AS schema_level_routine,
@@ -1511,7 +1511,7 @@ CREATE VIEW routines AS
           JOIN pg_language l ON p.prolang = l.oid)
          LEFT JOIN
          (pg_type t JOIN pg_namespace nt ON t.typnamespace = nt.oid)
-         ON p.prorettype = t.oid
+         ON p.prorettype = t.oid AND p.prokind <> 'p'
 
     WHERE (pg_has_role(p.proowner, 'USAGE')
            OR has_function_privilege(p.oid, 'EXECUTE'));

src/backend/catalog/objectaddress.c

@@ -4047,11 +4047,11 @@ getProcedureTypeDescription(StringInfo buffer, Oid procid)
 		elog(ERROR, "cache lookup failed for procedure %u", procid);
 	procForm = (Form_pg_proc) GETSTRUCT(procTup);
 
-	if (procForm->proisagg)
+	if (procForm->prokind == PROKIND_AGGREGATE)
 		appendStringInfoString(buffer, "aggregate");
-	else if (procForm->prorettype == InvalidOid)
+	else if (procForm->prokind == PROKIND_PROCEDURE)
 		appendStringInfoString(buffer, "procedure");
-	else
+	else						/* function or window function */
 		appendStringInfoString(buffer, "function");
 
 	ReleaseSysCache(procTup);

src/backend/catalog/pg_aggregate.c

@@ -616,8 +616,7 @@ AggregateCreate(const char *aggName,
 							 InvalidOid,	/* no validator */
 							 "aggregate_dummy", /* placeholder proc */
 							 NULL,	/* probin */
-							 true,	/* isAgg */
-							 false, /* isWindowFunc */
+							 PROKIND_AGGREGATE,
 							 false, /* security invoker (currently not
 									 * definable for agg) */
 							 false, /* isLeakProof */

src/backend/catalog/pg_proc.c

@@ -74,8 +74,7 @@ ProcedureCreate(const char *procedureName,
 				Oid languageValidator,
 				const char *prosrc,
 				const char *probin,
-				bool isAgg,
-				bool isWindowFunc,
+				char prokind,
 				bool security_definer,
 				bool isLeakProof,
 				bool isStrict,
@@ -335,8 +334,7 @@ ProcedureCreate(const char *procedureName,
 	values[Anum_pg_proc_prorows - 1] = Float4GetDatum(prorows);
 	values[Anum_pg_proc_provariadic - 1] = ObjectIdGetDatum(variadicType);
 	values[Anum_pg_proc_protransform - 1] = ObjectIdGetDatum(InvalidOid);
-	values[Anum_pg_proc_proisagg - 1] = BoolGetDatum(isAgg);
-	values[Anum_pg_proc_proiswindow - 1] = BoolGetDatum(isWindowFunc);
+	values[Anum_pg_proc_prokind - 1] = CharGetDatum(prokind);
 	values[Anum_pg_proc_prosecdef - 1] = BoolGetDatum(security_definer);
 	values[Anum_pg_proc_proleakproof - 1] = BoolGetDatum(isLeakProof);
 	values[Anum_pg_proc_proisstrict - 1] = BoolGetDatum(isStrict);
@@ -403,6 +401,21 @@ ProcedureCreate(const char *procedureName,
 			aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION,
 						   procedureName);
 
+		/* Not okay to change routine kind */
+		if (oldproc->prokind != prokind)
+			ereport(ERROR,
+					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+					 errmsg("cannot change routine kind"),
+					 (oldproc->prokind == PROKIND_AGGREGATE ?
+					  errdetail("\"%s\" is an aggregate function.", procedureName) :
+					  oldproc->prokind == PROKIND_FUNCTION ?
+					  errdetail("\"%s\" is a function.", procedureName) :
+					  oldproc->prokind == PROKIND_PROCEDURE ?
+					  errdetail("\"%s\" is a procedure.", procedureName) :
+					  oldproc->prokind == PROKIND_WINDOW ?
+					  errdetail("\"%s\" is a window function.", procedureName) :
+					  0)));
+
 		/*
 		 * Not okay to change the return type of the existing proc, since
 		 * existing rules, views, etc may depend on the return type.
@@ -535,34 +548,6 @@ ProcedureCreate(const char *procedureName,
 			}
 		}
 
-		/* Can't change aggregate or window-function status, either */
-		if (oldproc->proisagg != isAgg)
-		{
-			if (oldproc->proisagg)
-				ereport(ERROR,
-						(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-						 errmsg("function \"%s\" is an aggregate function",
-								procedureName)));
-			else
-				ereport(ERROR,
-						(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-						 errmsg("function \"%s\" is not an aggregate function",
-								procedureName)));
-		}
-		if (oldproc->proiswindow != isWindowFunc)
-		{
-			if (oldproc->proiswindow)
-				ereport(ERROR,
-						(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-						 errmsg("function \"%s\" is a window function",
-								procedureName)));
-			else
-				ereport(ERROR,
-						(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-						 errmsg("function \"%s\" is not a window function",
-								procedureName)));
-		}
-
 		/*
 		 * Do not change existing ownership or permissions, either.  Note
 		 * dependency-update code below has to agree with this decision.
@@ -857,8 +842,7 @@ fmgr_sql_validator(PG_FUNCTION_ARGS)
 
 	/* Disallow pseudotype result */
 	/* except for RECORD, VOID, or polymorphic */
-	if (proc->prorettype &&
-		get_typtype(proc->prorettype) == TYPTYPE_PSEUDO &&
+	if (get_typtype(proc->prorettype) == TYPTYPE_PSEUDO &&
 		proc->prorettype != RECORDOID &&
 		proc->prorettype != VOIDOID &&
 		!IsPolymorphicType(proc->prorettype))

src/backend/catalog/system_views.sql

@@ -332,9 +332,11 @@ WHERE
 UNION ALL
 SELECT
 	l.objoid, l.classoid, l.objsubid,
-	CASE WHEN pro.proisagg = true THEN 'aggregate'::text
-	     WHEN pro.proisagg = false THEN 'function'::text
-	END AS objtype,
+	CASE pro.prokind
+            WHEN 'a' THEN 'aggregate'::text
+            WHEN 'f' THEN 'function'::text
+            WHEN 'p' THEN 'procedure'::text
+            WHEN 'w' THEN 'window'::text END AS objtype,
 	pro.pronamespace AS objnamespace,
 	CASE WHEN pg_function_is_visible(pro.oid)
 	     THEN quote_ident(pro.proname)

src/backend/commands/dropcmds.c

@@ -92,7 +92,7 @@ RemoveObjects(DropStmt *stmt)
 		 */
 		if (stmt->removeType == OBJECT_FUNCTION)
 		{
-			if (get_func_isagg(address.objectId))
+			if (get_func_prokind(address.objectId) == PROKIND_AGGREGATE)
 				ereport(ERROR,
 						(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 						 errmsg("\"%s\" is an aggregate function",

src/backend/commands/functioncmds.c

@@ -1003,9 +1003,12 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt)
 
 	if (stmt->is_procedure)
 	{
+		/*
+		 * Sometime in the future, procedures might be allowed to return
+		 * results; for now, they all return VOID.
+		 */
 		Assert(!stmt->returnType);
-
-		prorettype = InvalidOid;
+		prorettype = VOIDOID;
 		returnsSet = false;
 	}
 	else if (stmt->returnType)
@@ -1097,8 +1100,7 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt)
 						   languageValidator,
 						   prosrc_str,	/* converted to text later */
 						   probin_str,	/* converted to text later */
-						   false,	/* not an aggregate */
-						   isWindowFunc,
+						   stmt->is_procedure ? PROKIND_PROCEDURE : (isWindowFunc ? PROKIND_WINDOW : PROKIND_FUNCTION),
 						   security,
 						   isLeakProof,
 						   isStrict,
@@ -1126,7 +1128,7 @@ RemoveFunctionById(Oid funcOid)
 {
 	Relation	relation;
 	HeapTuple	tup;
-	bool		isagg;
+	char		prokind;
 
 	/*
 	 * Delete the pg_proc tuple.
@@ -1137,7 +1139,7 @@ RemoveFunctionById(Oid funcOid)
 	if (!HeapTupleIsValid(tup)) /* should not happen */
 		elog(ERROR, "cache lookup failed for function %u", funcOid);
 
-	isagg = ((Form_pg_proc) GETSTRUCT(tup))->proisagg;
+	prokind = ((Form_pg_proc) GETSTRUCT(tup))->prokind;
 
 	CatalogTupleDelete(relation, &tup->t_self);
 
@@ -1148,7 +1150,7 @@ RemoveFunctionById(Oid funcOid)
 	/*
 	 * If there's a pg_aggregate tuple, delete that too.
 	 */
-	if (isagg)
+	if (prokind == PROKIND_AGGREGATE)
 	{
 		relation = heap_open(AggregateRelationId, RowExclusiveLock);
 
@@ -1203,13 +1205,13 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt)
 		aclcheck_error(ACLCHECK_NOT_OWNER, stmt->objtype,
 					   NameListToString(stmt->func->objname));
 
-	if (procForm->proisagg)
+	if (procForm->prokind == PROKIND_AGGREGATE)
 		ereport(ERROR,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("\"%s\" is an aggregate function",
 						NameListToString(stmt->func->objname))));
 
-	is_procedure = (procForm->prorettype == InvalidOid);
+	is_procedure = (procForm->prokind == PROKIND_PROCEDURE);
 
 	/* Examine requested actions. */
 	foreach(l, stmt->actions)
@@ -1525,14 +1527,10 @@ CreateCast(CreateCastStmt *stmt)
 					(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
 					 errmsg("cast function must not be volatile")));
 #endif
-		if (procstruct->proisagg)
+		if (procstruct->prokind != PROKIND_FUNCTION)
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-					 errmsg("cast function must not be an aggregate function")));
-		if (procstruct->proiswindow)
-			ereport(ERROR,
-					(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-					 errmsg("cast function must not be a window function")));
+					 errmsg("cast function must be a normal function")));
 		if (procstruct->proretset)
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
@@ -1777,14 +1775,10 @@ check_transform_function(Form_pg_proc procstruct)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
 				 errmsg("transform function must not be volatile")));
-	if (procstruct->proisagg)
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-				 errmsg("transform function must not be an aggregate function")));
-	if (procstruct->proiswindow)
+	if (procstruct->prokind != PROKIND_FUNCTION)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-				 errmsg("transform function must not be a window function")));
+				 errmsg("transform function must be a normal function")));
 	if (procstruct->proretset)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),