Further cleanup around the edges of OPAQUE/pseudotype changes. Correct

the declarations of some index access method support functions.  Support
SQL functions returning VOID.

doc/src/sgml/xfunc.sgml

 <!--
-$Header: /cvsroot/pgsql/doc/src/sgml/xfunc.sgml,v 1.55 2002/08/22 00:01:40 tgl Exp $
+$Header: /cvsroot/pgsql/doc/src/sgml/xfunc.sgml,v 1.56 2002/08/23 16:41:37 tgl Exp $
 -->
 
  <chapter id="xfunc">
@@ -170,22 +170,26 @@ CREATE FUNCTION tp1 (integer, numeric) RETURNS numeric AS '
      <command>DELETE</command>) as well
      as <command>SELECT</command> queries.  However, the final command 
      must be a <command>SELECT</command> that returns whatever is
-     specified as the function's return type.
+     specified as the function's return type.  Alternatively, if you
+     want to define a SQL function that performs actions but has no
+     useful value to return, you can define it as returning <type>void</>.
+     In that case it must not end with a <command>SELECT</command>.
+     For example:
 
 <programlisting>
-CREATE FUNCTION clean_EMP () RETURNS integer AS '
+CREATE FUNCTION clean_EMP () RETURNS void AS '
     DELETE FROM EMP 
         WHERE EMP.salary &lt;= 0;
-    SELECT 1 AS ignore_this;
 ' LANGUAGE SQL;
 
 SELECT clean_EMP();
 </programlisting>
 
 <screen>
- x
----
- 1
+ clean_emp
+-----------
+
+(1 row)
 </screen>
     </para>
 

src/backend/catalog/pg_proc.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/catalog/pg_proc.c,v 1.89 2002/08/22 00:01:41 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/catalog/pg_proc.c,v 1.90 2002/08/23 16:41:37 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -266,24 +266,18 @@ ProcedureCreate(const char *procedureName,
 	recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
 
 	/* dependency on return type */
-	if (OidIsValid(returnType))
-	{
-		referenced.classId = RelOid_pg_type;
-		referenced.objectId = returnType;
-		referenced.objectSubId = 0;
-		recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
-	}
+	referenced.classId = RelOid_pg_type;
+	referenced.objectId = returnType;
+	referenced.objectSubId = 0;
+	recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
 
 	/* dependency on input types */
 	for (i = 0; i < parameterCount; i++)
 	{
-		if (OidIsValid(typev[i]))
-		{
-			referenced.classId = RelOid_pg_type;
-			referenced.objectId = typev[i];
-			referenced.objectSubId = 0;
-			recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
-		}
+		referenced.classId = RelOid_pg_type;
+		referenced.objectId = typev[i];
+		referenced.objectSubId = 0;
+		recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
 	}
 
 	heap_freetuple(tup);
@@ -324,10 +318,10 @@ checkretval(Oid rettype, char fn_typtype, List *queryTreeList)
 	int			rellogcols;		/* # of nondeleted columns in rel */
 	int			colindex;		/* physical column index */
 
-	/* guard against empty function body; OK only if no return type */
+	/* guard against empty function body; OK only if void return type */
 	if (queryTreeList == NIL)
 	{
-		if (rettype != InvalidOid)
+		if (rettype != VOIDOID)
 			elog(ERROR, "function declared to return %s, but no SELECT provided",
 				 format_type_be(rettype));
 		return;
@@ -340,13 +334,12 @@ checkretval(Oid rettype, char fn_typtype, List *queryTreeList)
 	tlist = parse->targetList;
 
 	/*
-	 * The last query must be a SELECT if and only if there is a return
-	 * type.
+	 * The last query must be a SELECT if and only if return type isn't VOID.
 	 */
-	if (rettype == InvalidOid)
+	if (rettype == VOIDOID)
 	{
 		if (cmd == CMD_SELECT)
-			elog(ERROR, "function declared with no return type, but final statement is a SELECT");
+			elog(ERROR, "function declared to return void, but final statement is a SELECT");
 		return;
 	}
 
@@ -573,13 +566,15 @@ fmgr_sql_validator(PG_FUNCTION_ARGS)
 	tuple = SearchSysCache(PROCOID, funcoid, 0, 0, 0);
 	if (!HeapTupleIsValid(tuple))
 		elog(ERROR, "cache lookup of function %u failed", funcoid);
-
 	proc = (Form_pg_proc) GETSTRUCT(tuple);
 
+	functyptype = get_typtype(proc->prorettype);
+
 	/* Disallow pseudotypes in arguments and result */
-	/* except that return type can be RECORD */
-	if (get_typtype(proc->prorettype) == 'p' &&
-		proc->prorettype != RECORDOID)
+	/* except that return type can be RECORD or VOID */
+	if (functyptype == 'p' &&
+		proc->prorettype != RECORDOID &&
+		proc->prorettype != VOIDOID)
 		elog(ERROR, "SQL functions cannot return type %s",
 			 format_type_be(proc->prorettype));
 
@@ -596,9 +591,6 @@ fmgr_sql_validator(PG_FUNCTION_ARGS)
 
 	prosrc = DatumGetCString(DirectFunctionCall1(textout, tmp));
 
-	/* check typtype to see if we have a predetermined return type */
-	functyptype = get_typtype(proc->prorettype);
-
 	querytree_list = pg_parse_and_rewrite(prosrc, proc->proargtypes, proc->pronargs);
 	checkretval(proc->prorettype, functyptype, querytree_list);
 

src/backend/commands/typecmds.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/typecmds.c,v 1.10 2002/08/22 00:01:42 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/typecmds.c,v 1.11 2002/08/23 16:41:37 tgl Exp $
  *
  * DESCRIPTION
  *	  The "DefineFoo" routines take the parse tree and pick out the
@@ -203,19 +203,27 @@ DefineType(List *names, List *parameters)
 	outputOid = findTypeIOFunction(outputName, typoid, true);
 
 	/*
-	 * Verify that I/O procs return the expected thing.  OPAQUE is an allowed
-	 * (but deprecated) alternative to the fully type-safe choices.
+	 * Verify that I/O procs return the expected thing.  OPAQUE is an allowed,
+	 * but deprecated, alternative to the fully type-safe choices.
 	 */
 	resulttype = get_func_rettype(inputOid);
-	if (!((OidIsValid(typoid) && resulttype == typoid) ||
-		  resulttype == OPAQUEOID))
-		elog(ERROR, "Type input function %s must return %s or OPAQUE",
-			 NameListToString(inputName), typeName);
+	if (!(OidIsValid(typoid) && resulttype == typoid))
+	{
+		if (resulttype == OPAQUEOID)
+			elog(NOTICE, "DefineType: OPAQUE is deprecated, instead declare I/O functions using their true datatypes");
+		else
+			elog(ERROR, "Type input function %s must return %s",
+				 NameListToString(inputName), typeName);
+	}
 	resulttype = get_func_rettype(outputOid);
-	if (!(resulttype == CSTRINGOID ||
-		  resulttype == OPAQUEOID))
-		elog(ERROR, "Type output function %s must return CSTRING or OPAQUE",
-			 NameListToString(outputName));
+	if (resulttype != CSTRINGOID)
+	{
+		if (resulttype == OPAQUEOID)
+			elog(NOTICE, "DefineType: OPAQUE is deprecated, instead declare I/O functions using their true datatypes");
+		else
+			elog(ERROR, "Type output function %s must return cstring",
+				 NameListToString(outputName));
+	}
 
 	/*
 	 * now have TypeCreate do all the real work.
@@ -671,6 +679,10 @@ findTypeIOFunction(List *procname, Oid typeOid, bool isOutput)
 		 * or two arguments (data value, element OID).  The signature
 		 * may use OPAQUE in place of the actual type name; this is the
 		 * only possibility if the type doesn't yet exist as a shell.
+		 *
+		 * Note: although we could throw a NOTICE in this routine if OPAQUE
+		 * is used, we do not because of the probability that it'd be
+		 * duplicate with a notice issued in DefineType.
 		 */
 		if (OidIsValid(typeOid))
 		{

src/backend/executor/functions.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/functions.c,v 1.54 2002/08/05 02:30:50 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/functions.c,v 1.55 2002/08/23 16:41:37 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -86,36 +86,35 @@ static void ShutdownSQLFunction(Datum arg);
 static execution_state *
 init_execution_state(char *src, Oid *argOidVect, int nargs)
 {
-	execution_state *newes;
-	execution_state *nextes;
+	execution_state *firstes;
 	execution_state *preves;
 	List	   *queryTree_list,
 			   *qtl_item;
 
-	newes = (execution_state *) palloc(sizeof(execution_state));
-	nextes = newes;
-	preves = (execution_state *) NULL;
-
 	queryTree_list = pg_parse_and_rewrite(src, argOidVect, nargs);
 
+	firstes = NULL;
+	preves = NULL;
+
 	foreach(qtl_item, queryTree_list)
 	{
 		Query	   *queryTree = lfirst(qtl_item);
 		Plan	   *planTree;
+		execution_state *newes;
 		EState	   *estate;
 
 		planTree = pg_plan_query(queryTree);
 
-		if (!nextes)
-			nextes = (execution_state *) palloc(sizeof(execution_state));
+		newes = (execution_state *) palloc(sizeof(execution_state));
 		if (preves)
-			preves->next = nextes;
-
-		nextes->next = NULL;
-		nextes->status = F_EXEC_START;
+			preves->next = newes;
+		else
+			firstes = newes;
 
-		nextes->qd = CreateQueryDesc(queryTree, planTree, None, NULL);
-		estate = CreateExecutorState();
+		newes->next = NULL;
+		newes->status = F_EXEC_START;
+		newes->qd = CreateQueryDesc(queryTree, planTree, None, NULL);
+		newes->estate = estate = CreateExecutorState();
 
 		if (nargs > 0)
 		{
@@ -124,7 +123,7 @@ init_execution_state(char *src, Oid *argOidVect, int nargs)
 
 			paramLI = (ParamListInfo) palloc((nargs + 1) * sizeof(ParamListInfoData));
 
-			MemSet(paramLI, 0, nargs * sizeof(ParamListInfoData));
+			MemSet(paramLI, 0, (nargs + 1) * sizeof(ParamListInfoData));
 
 			estate->es_param_list_info = paramLI;
 
@@ -139,12 +138,11 @@ init_execution_state(char *src, Oid *argOidVect, int nargs)
 		}
 		else
 			estate->es_param_list_info = (ParamListInfo) NULL;
-		nextes->estate = estate;
-		preves = nextes;
-		nextes = (execution_state *) NULL;
+
+		preves = newes;
 	}
 
-	return newes;
+	return firstes;
 }
 
 
@@ -195,7 +193,7 @@ init_sql_fcache(FmgrInfo *finfo)
 	 */
 	fcache->typlen = typeStruct->typlen;
 
-	if (typeStruct->typtype == 'b' || typeStruct->typtype == 'd')
+	if (typeStruct->typtype != 'c')
 	{
 		/* The return type is not a relation, so just use byval */
 		fcache->typbyval = typeStruct->typbyval;
@@ -484,7 +482,6 @@ fmgr_sql(PG_FUNCTION_ARGS)
 		fcache = (SQLFunctionCachePtr) fcinfo->flinfo->fn_extra;
 	}
 	es = fcache->func_state;
-	Assert(es);
 
 	/*
 	 * Find first unfinished query in function.
@@ -492,14 +489,12 @@ fmgr_sql(PG_FUNCTION_ARGS)
 	while (es && es->status == F_EXEC_DONE)
 		es = es->next;
 
-	Assert(es);
-
 	/*
 	 * Execute each command in the function one after another until we're
 	 * executing the final command and get a result or we run out of
 	 * commands.
 	 */
-	while (es != (execution_state *) NULL)
+	while (es)
 	{
 		result = postquel_execute(es, fcinfo, fcache);
 		if (es->status != F_EXEC_DONE)

src/test/regress/expected/opr_sanity.out

@@ -187,6 +187,19 @@ WHERE p1.oid != p2.oid AND
 -------------+-------------
 (0 rows)
 
+-- Look for functions that return type "internal" and do not have any
+-- "internal" argument.  Such a function would be a security hole since
+-- it might be used to call an internal function from an SQL command.
+-- As of 7.3 this query should find only internal_in.
+SELECT p1.oid, p1.proname
+FROM pg_proc as p1
+WHERE p1.prorettype = 'internal'::regtype AND NOT
+    ('(' || oidvectortypes(p1.proargtypes) || ')') ~ '[^a-z0-9_]internal[^a-z0-9_]';
+ oid  |   proname   
+------+-------------
+ 2304 | internal_in
+(1 row)
+
 -- **************** pg_cast ****************
 -- Look for casts from and to the same type.  This is not harmful, but
 -- useless.

src/test/regress/sql/opr_sanity.sql

@@ -148,6 +148,17 @@ WHERE p1.oid != p2.oid AND
     NOT p1.proisagg AND NOT p2.proisagg AND
     (p1.proargtypes[7] < p2.proargtypes[7]);
 
+-- Look for functions that return type "internal" and do not have any
+-- "internal" argument.  Such a function would be a security hole since
+-- it might be used to call an internal function from an SQL command.
+-- As of 7.3 this query should find only internal_in.
+
+SELECT p1.oid, p1.proname
+FROM pg_proc as p1
+WHERE p1.prorettype = 'internal'::regtype AND NOT
+    ('(' || oidvectortypes(p1.proargtypes) || ')') ~ '[^a-z0-9_]internal[^a-z0-9_]';
+
+
 -- **************** pg_cast ****************
 
 -- Look for casts from and to the same type.  This is not harmful, but