Arrange that no database accesses are attempted during parser() --- this

took some rejiggering of typename and ACL parsing, as well as moving parse_analyze call out of parser(). Restructure postgres.c processing so that parse analysis and rewrite are skipped when in abort-transaction state. Only COMMIT and ABORT statements will be processed beyond the raw parser() phase. This addresses problem of parser failing with database access errors while in aborted state (see pghackers discussions around 7/28/00). Also fix some bugs with COMMIT/ABORT statements appearing in the middle of a single query input string. Function, operator, and aggregate arguments/results can now use full TypeName production, in particular foo[] for array types. DROP OPERATOR and COMMENT ON OPERATOR were broken for unary operators. Allow CREATE AGGREGATE to accept unquoted numeric constants for initcond.
2025-12-22 17:42:17 +03:00 · 2000-10-07 00:58:23 +00:00
parent 4837270be9
commit fbd26d6984
20 changed files with 773 additions and 755 deletions
--- a/src/backend/commands/comment.c
+++ b/src/backend/commands/comment.c
@@ -25,6 +25,8 @@
 #include "commands/comment.h"
 #include "miscadmin.h"
 #include "parser/parse.h"
+#include "parser/parse_expr.h"
+#include "parser/parse_func.h"
 #include "rewrite/rewriteRemove.h"
 #include "utils/acl.h"
 #include "utils/fmgroids.h"
@@ -46,7 +48,7 @@ static void CommentAttribute(char *relation, char *attrib, char *comment);
 static void CommentDatabase(char *database, char *comment);
 static void CommentRewrite(char *rule, char *comment);
 static void CommentType(char *type, char *comment);
-static void CommentAggregate(char *aggregate, char *aggtype, char *comment);
+static void CommentAggregate(char *aggregate, List *arguments, char *comment);
 static void CommentProc(char *function, List *arguments, char *comment);
 static void CommentOperator(char *opname, List *arguments, char *comment);
 static void CommentTrigger(char *trigger, char *relation, char *comments);
@@ -92,7 +94,7 @@ CommentObject(int objtype, char *objname, char *objproperty,
 			CommentType(objname, comment);
 			break;
 		case (AGGREGATE):
-			CommentAggregate(objname, objproperty, comment);
+			CommentAggregate(objname, objlist, comment);
 			break;
 		case (FUNCTION):
 			CommentProc(objname, objlist, comment);
@@ -544,9 +546,10 @@ CommentType(char *type, char *comment)
 */

 static void
-CommentAggregate(char *aggregate, char *argument, char *comment)
+CommentAggregate(char *aggregate, List *arguments, char *comment)
 {
-
+	TypeName   *aggtype = (TypeName *) lfirst(arguments);
+	char	   *aggtypename = NULL;
 	HeapTuple	aggtuple;
 	Oid			baseoid,
 				oid;
@@ -554,11 +557,12 @@ CommentAggregate(char *aggregate, char *argument, char *comment)

 	/*** First, attempt to determine the base aggregate oid ***/

-	if (argument)
+	if (aggtype)
 	{
-		baseoid = TypeGet(argument, &defined);
+		aggtypename = TypeNameToInternalName(aggtype);
+		baseoid = TypeGet(aggtypename, &defined);
 		if (!OidIsValid(baseoid))
-			elog(ERROR, "aggregate type '%s' does not exist", argument);
+			elog(ERROR, "type '%s' does not exist", aggtypename);
 	}
 	else
 		baseoid = 0;
@@ -568,10 +572,10 @@ CommentAggregate(char *aggregate, char *argument, char *comment)
 #ifndef NO_SECURITY
 	if (!pg_aggr_ownercheck(GetUserId(), aggregate, baseoid))
 	{
-		if (argument)
+		if (aggtypename)
 		{
 			elog(ERROR, "you are not permitted to comment on aggregate '%s' %s '%s'",
-				 aggregate, "with type", argument);
+				 aggregate, "with type", aggtypename);
 		}
 		else
 		{
@@ -587,10 +591,10 @@ CommentAggregate(char *aggregate, char *argument, char *comment)
 								   ObjectIdGetDatum(baseoid), 0, 0);
 	if (!HeapTupleIsValid(aggtuple))
 	{
-		if (argument)
+		if (aggtypename)
 		{
 			elog(ERROR, "aggregate type '%s' does not exist for aggregate '%s'",
-				 argument, aggregate);
+				 aggtypename, aggregate);
 		}
 		else
 			elog(ERROR, "aggregate '%s' does not exist", aggregate);
@@ -622,7 +626,6 @@ CommentProc(char *function, List *arguments, char *comment)
 				functuple;
 	Oid			oid,
 				argoids[FUNC_MAX_ARGS];
-	char	   *argument;
 	int			i,
 				argcount;

@@ -635,18 +638,20 @@ CommentProc(char *function, List *arguments, char *comment)
 			 FUNC_MAX_ARGS);
 	for (i = 0; i < argcount; i++)
 	{
-		argument = strVal(lfirst(arguments));
+		TypeName   *t = (TypeName *) lfirst(arguments);
+		char	   *typnam = TypeNameToInternalName(t);
+
 		arguments = lnext(arguments);
-		if (strcmp(argument, "opaque") == 0)
-			argoids[i] = 0;
+
+		if (strcmp(typnam, "opaque") == 0)
+			argoids[i] = InvalidOid;
 		else
 		{
 			argtuple = SearchSysCacheTuple(TYPENAME,
-										   PointerGetDatum(argument),
+										   PointerGetDatum(typnam),
 										   0, 0, 0);
 			if (!HeapTupleIsValid(argtuple))
-				elog(ERROR, "function argument type '%s' does not exist",
-					 argument);
+				elog(ERROR, "CommentProc: type '%s' not found", typnam);
 			argoids[i] = argtuple->t_data->t_oid;
 		}
 	}
@@ -664,10 +669,9 @@ CommentProc(char *function, List *arguments, char *comment)
 	functuple = SearchSysCacheTuple(PROCNAME, PointerGetDatum(function),
 									Int32GetDatum(argcount),
 									PointerGetDatum(argoids), 0);
-
 	if (!HeapTupleIsValid(functuple))
-		elog(ERROR, "function '%s' with the supplied %s does not exist",
-			 function, "argument list");
+		func_error("CommentProc", function, argcount, argoids, NULL);
+
 	oid = functuple->t_data->t_oid;

 	/*** Call CreateComments() to create/drop the comments ***/
@@ -691,23 +695,24 @@ CommentProc(char *function, List *arguments, char *comment)
 static void
 CommentOperator(char *opername, List *arguments, char *comment)
 {
-
+	TypeName   *typenode1 = (TypeName *) lfirst(arguments);
+	TypeName   *typenode2 = (TypeName *) lsecond(arguments);
+	char		oprtype = 0,
+			   *lefttype = NULL,
+			   *righttype = NULL;
 	Form_pg_operator data;
 	HeapTuple	optuple;
 	Oid			oid,
 				leftoid = InvalidOid,
 				rightoid = InvalidOid;
 	bool		defined;
-	char		oprtype = 0,
-			   *lefttype = NULL,
-			   *righttype = NULL;

 	/*** Initialize our left and right argument types ***/

-	if (lfirst(arguments) != NULL)
-		lefttype = strVal(lfirst(arguments));
-	if (lsecond(arguments) != NULL)
-		righttype = strVal(lsecond(arguments));
+	if (typenode1 != NULL)
+		lefttype = TypeNameToInternalName(typenode1);
+	if (typenode2 != NULL)
+		righttype = TypeNameToInternalName(typenode2);

 	/*** Attempt to fetch the left oid, if specified ***/

@@ -732,9 +737,9 @@ CommentOperator(char *opername, List *arguments, char *comment)
 	if (OidIsValid(leftoid) && (OidIsValid(rightoid)))
 		oprtype = 'b';
 	else if (OidIsValid(leftoid))
-		oprtype = 'l';
-	else if (OidIsValid(rightoid))
 		oprtype = 'r';
+	else if (OidIsValid(rightoid))
+		oprtype = 'l';
 	else
 		elog(ERROR, "operator '%s' is of an illegal type'", opername);

@@ -769,7 +774,6 @@ CommentOperator(char *opername, List *arguments, char *comment)
 	/*** Call CreateComments() to create/drop the comments ***/

 	CreateComments(oid, comment);
-
 }

 /*------------------------------------------------------------------
--- a/src/backend/commands/define.c
+++ b/src/backend/commands/define.c
@@ -10,7 +10,7 @@
 *
 *
 * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/define.c,v 1.46 2000/07/22 03:34:26 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/define.c,v 1.47 2000/10/07 00:58:16 tgl Exp $
 *
 * DESCRIPTION
 *	  The "DefineFoo" routines take the parse tree and pick out the
@@ -49,6 +49,7 @@
 #include "commands/defrem.h"
 #include "fmgr.h"
 #include "optimizer/cost.h"
+#include "parser/parse_expr.h"
 #include "tcop/dest.h"
 #include "utils/builtins.h"
 #include "utils/syscache.h"
@@ -83,27 +84,15 @@ case_translate_language_name(const char *input, char *output)


 static void
-compute_return_type(const Node *returnType,
+compute_return_type(TypeName *returnType,
 					char **prorettype_p, bool *returnsSet_p)
 {
 /*---------------------------------------------------------------------------
   Examine the "returns" clause returnType of the CREATE FUNCTION statement
-   and return information about it as **prorettype_p and **returnsSet.
+   and return information about it as *prorettype_p and *returnsSet.
 ----------------------------------------------------------------------------*/
-	if (nodeTag(returnType) == T_TypeName)
-	{
-		/* a set of values */
-		TypeName   *setType = (TypeName *) returnType;
-
-		*prorettype_p = setType->name;
-		*returnsSet_p = setType->setof;
-	}
-	else
-	{
-		/* singleton */
-		*prorettype_p = strVal(returnType);
-		*returnsSet_p = false;
-	}
+	*prorettype_p = TypeNameToInternalName(returnType);
+	*returnsSet_p = returnType->setof;
 }


@@ -214,7 +203,7 @@ interpret_AS_clause(const char *languageName, const List *as,
 		*prosrc_str_p = strVal(lfirst(as));
 		*probin_str_p = "-";

-		if (lnext(as) != NULL)
+		if (lnext(as) != NIL)
 			elog(ERROR, "CREATE FUNCTION: only one AS item needed for %s language",
 				 languageName);
 	}
@@ -231,27 +220,23 @@ void
 CreateFunction(ProcedureStmt *stmt, CommandDest dest)
 {
 	char	   *probin_str;
-
 	/* pathname of executable file that executes this function, if any */
+
 	char	   *prosrc_str;
-
 	/* SQL that executes this function, if any */
+
 	char	   *prorettype;
-
 	/* Type of return value (or member of set of values) from function */
-	char		languageName[NAMEDATALEN];

+	char		languageName[NAMEDATALEN];
 	/*
 	 * name of language of function, with case adjusted: "C", "newC",
 	 * "internal", "newinternal", "sql", etc.
 	 */

 	bool		returnsSet;
-
 	/* The function returns a set of values, as opposed to a singleton. */

-	bool		lanisPL = false;
-
 	/*
 	 * The following are optional user-supplied attributes of the
 	 * function.
@@ -263,8 +248,12 @@ CreateFunction(ProcedureStmt *stmt, CommandDest dest)
 	bool		canCache,
 				isStrict;

+	/* Convert language name to canonical case */
 	case_translate_language_name(stmt->language, languageName);

+	/*
+	 * Apply appropriate security checks depending on language.
+	 */
 	if (strcmp(languageName, "C") == 0 ||
 		strcmp(languageName, "newC") == 0 ||
 		strcmp(languageName, "internal") == 0 ||
@@ -301,7 +290,7 @@ CreateFunction(ProcedureStmt *stmt, CommandDest dest)

 		/* Check that this language is a PL */
 		languageStruct = (Form_pg_language) GETSTRUCT(languageTuple);
-		if (!(languageStruct->lanispl))
+		if (!languageStruct->lanispl)
 			elog(ERROR,
 				 "Language '%s' isn't defined as PL", languageName);

@@ -316,11 +305,15 @@ CreateFunction(ProcedureStmt *stmt, CommandDest dest)
 				 "language.",
 				 languageName);
 		}
-
-		lanisPL = true;
 	}

-	compute_return_type(stmt->returnType, &prorettype, &returnsSet);
+	/*
+	 * Convert remaining parameters of CREATE to form wanted by
+	 * ProcedureCreate.
+	 */
+	Assert(IsA(stmt->returnType, TypeName));
+	compute_return_type((TypeName *) stmt->returnType,
+						&prorettype, &returnsSet);

 	compute_full_attributes(stmt->withClause,
 							&byte_pct, &perbyte_cpu, &percall_cpu,
@@ -345,7 +338,7 @@ CreateFunction(ProcedureStmt *stmt, CommandDest dest)
 					perbyte_cpu,
 					percall_cpu,
 					outin_ratio,
-					stmt->defArgs,
+					stmt->argTypes,
 					dest);
 }

@@ -389,49 +382,43 @@ DefineOperator(char *oprName,
 	{
 		DefElem    *defel = (DefElem *) lfirst(pl);

-		if (!strcasecmp(defel->defname, "leftarg"))
+		if (strcasecmp(defel->defname, "leftarg") == 0)
 		{
-			if ((nodeTag(defel->arg) == T_TypeName)
-				&& (((TypeName *) defel->arg)->setof))
-				elog(ERROR, "setof type not implemented for leftarg");
-
 			typeName1 = defGetString(defel);
-			if (typeName1 == NULL)
-				elog(ERROR, "type for leftarg is malformed.");
+			if (IsA(defel->arg, TypeName)
+				&& ((TypeName *) defel->arg)->setof)
+				elog(ERROR, "setof type not implemented for leftarg");
 		}
-		else if (!strcasecmp(defel->defname, "rightarg"))
+		else if (strcasecmp(defel->defname, "rightarg") == 0)
 		{
-			if ((nodeTag(defel->arg) == T_TypeName)
-				&& (((TypeName *) defel->arg)->setof))
-				elog(ERROR, "setof type not implemented for rightarg");
-
 			typeName2 = defGetString(defel);
-			if (typeName2 == NULL)
-				elog(ERROR, "type for rightarg is malformed.");
+			if (IsA(defel->arg, TypeName)
+				&& ((TypeName *) defel->arg)->setof)
+				elog(ERROR, "setof type not implemented for rightarg");
 		}
-		else if (!strcasecmp(defel->defname, "procedure"))
+		else if (strcasecmp(defel->defname, "procedure") == 0)
 			functionName = defGetString(defel);
-		else if (!strcasecmp(defel->defname, "precedence"))
+		else if (strcasecmp(defel->defname, "precedence") == 0)
 		{
 			/* NOT IMPLEMENTED (never worked in v4.2) */
 			elog(NOTICE, "CREATE OPERATOR: precedence not implemented");
 		}
-		else if (!strcasecmp(defel->defname, "associativity"))
+		else if (strcasecmp(defel->defname, "associativity") == 0)
 		{
 			/* NOT IMPLEMENTED (never worked in v4.2) */
 			elog(NOTICE, "CREATE OPERATOR: associativity not implemented");
 		}
-		else if (!strcasecmp(defel->defname, "commutator"))
+		else if (strcasecmp(defel->defname, "commutator") == 0)
 			commutatorName = defGetString(defel);
-		else if (!strcasecmp(defel->defname, "negator"))
+		else if (strcasecmp(defel->defname, "negator") == 0)
 			negatorName = defGetString(defel);
-		else if (!strcasecmp(defel->defname, "restrict"))
+		else if (strcasecmp(defel->defname, "restrict") == 0)
 			restrictionName = defGetString(defel);
-		else if (!strcasecmp(defel->defname, "join"))
+		else if (strcasecmp(defel->defname, "join") == 0)
 			joinName = defGetString(defel);
-		else if (!strcasecmp(defel->defname, "hashes"))
+		else if (strcasecmp(defel->defname, "hashes") == 0)
 			canHash = TRUE;
-		else if (!strcasecmp(defel->defname, "sort1"))
+		else if (strcasecmp(defel->defname, "sort1") == 0)
 		{
 			/* ----------------
 			 * XXX ( ... [ , sort1 = oprname ] [ , sort2 = oprname ] ... )
@@ -441,7 +428,7 @@ DefineOperator(char *oprName,
 			 */
 			sortName1 = defGetString(defel);
 		}
-		else if (!strcasecmp(defel->defname, "sort2"))
+		else if (strcasecmp(defel->defname, "sort2") == 0)
 			sortName2 = defGetString(defel);
 		else
 		{
@@ -562,72 +549,73 @@ DefineType(char *typeName, List *parameters)
 	char		delimiter = DEFAULT_TYPDELIM;
 	char	   *shadow_type;
 	List	   *pl;
-	char		alignment = 'i';/* default alignment */
-	char		storage = 'p'; /* default storage in TOAST */
+	char		alignment = 'i'; /* default alignment */
+	char		storage = 'p';	/* default storage in TOAST */

 	/*
-	 * Type names can only be 15 characters long, so that the shadow type
-	 * can be created using the 16th character as necessary.
+	 * Type names must be one character shorter than other names,
+	 * allowing room to create the corresponding array type name with
+	 * prepended "_".
 	 */
-	if (strlen(typeName) >= (NAMEDATALEN - 1))
+	if (strlen(typeName) > (NAMEDATALEN - 2))
 	{
 		elog(ERROR, "DefineType: type names must be %d characters or less",
-			 NAMEDATALEN - 1);
+			 NAMEDATALEN - 2);
 	}

 	foreach(pl, parameters)
 	{
 		DefElem    *defel = (DefElem *) lfirst(pl);

-		if (!strcasecmp(defel->defname, "internallength"))
+		if (strcasecmp(defel->defname, "internallength") == 0)
 			internalLength = defGetTypeLength(defel);
-		else if (!strcasecmp(defel->defname, "externallength"))
+		else if (strcasecmp(defel->defname, "externallength") == 0)
 			externalLength = defGetTypeLength(defel);
-		else if (!strcasecmp(defel->defname, "input"))
+		else if (strcasecmp(defel->defname, "input") == 0)
 			inputName = defGetString(defel);
-		else if (!strcasecmp(defel->defname, "output"))
+		else if (strcasecmp(defel->defname, "output") == 0)
 			outputName = defGetString(defel);
-		else if (!strcasecmp(defel->defname, "send"))
+		else if (strcasecmp(defel->defname, "send") == 0)
 			sendName = defGetString(defel);
-		else if (!strcasecmp(defel->defname, "delimiter"))
+		else if (strcasecmp(defel->defname, "delimiter") == 0)
 		{
 			char	   *p = defGetString(defel);

 			delimiter = p[0];
 		}
-		else if (!strcasecmp(defel->defname, "receive"))
+		else if (strcasecmp(defel->defname, "receive") == 0)
 			receiveName = defGetString(defel);
-		else if (!strcasecmp(defel->defname, "element"))
+		else if (strcasecmp(defel->defname, "element") == 0)
 			elemName = defGetString(defel);
-		else if (!strcasecmp(defel->defname, "default"))
+		else if (strcasecmp(defel->defname, "default") == 0)
 			defaultValue = defGetString(defel);
-		else if (!strcasecmp(defel->defname, "passedbyvalue"))
+		else if (strcasecmp(defel->defname, "passedbyvalue") == 0)
 			byValue = true;
-		else if (!strcasecmp(defel->defname, "alignment"))
+		else if (strcasecmp(defel->defname, "alignment") == 0)
 		{
 			char	   *a = defGetString(defel);

-			if (!strcasecmp(a, "double"))
+			if (strcasecmp(a, "double") == 0)
 				alignment = 'd';
-			else if (!strcasecmp(a, "int4"))
+			else if (strcasecmp(a, "int4") == 0)
 				alignment = 'i';
 			else
 			{
-				elog(ERROR, "DefineType: \"%s\" alignment  not recognized",
+				elog(ERROR, "DefineType: \"%s\" alignment not recognized",
 					 a);
 			}
 		}
-		else if (!strcasecmp(defel->defname, "storage"))
+		else if (strcasecmp(defel->defname, "storage") == 0)
 		{
 			char	   *a = defGetString(defel);

-			if (!strcasecmp(a, "plain"))
+			if (strcasecmp(a, "plain") == 0)
 				storage = 'p';
-			else if (!strcasecmp(a, "external"))
+			else if (strcasecmp(a, "external") == 0)
 				storage = 'e';
-			else if (!strcasecmp(a, "extended"))
+			else if (strcasecmp(a, "extended") == 0)
 				storage = 'x';
-			else if (!strcasecmp(a, "main"))
+			else if (strcasecmp(a, "main") == 0)
 				storage = 'm';
 			else
 			{
@@ -674,8 +662,8 @@ DefineType(char *typeName, List *parameters)
 			   storage);		/* TOAST strategy */

 	/* ----------------
-	 *	When we create a true type (as opposed to a complex type)
-	 *	we need to have an shadow array entry for it in pg_type as well.
+	 *	When we create a base type (as opposed to a complex type)
+	 *	we need to have an array entry for it in pg_type as well.
 	 * ----------------
 	 */
 	shadow_type = makeArrayTypeName(typeName);
@@ -702,19 +690,32 @@ DefineType(char *typeName, List *parameters)
 static char *
 defGetString(DefElem *def)
 {
-	char	   *string;
+	if (def->arg == NULL)
+		elog(ERROR, "Define: \"%s\" requires a parameter",
+			 def->defname);
+	switch (nodeTag(def->arg))
+	{
+		case T_Integer:
+		{
+			char *str = palloc(32);

-	if (nodeTag(def->arg) == T_String)
-		string = strVal(def->arg);
-	else if (nodeTag(def->arg) == T_TypeName)
-		string = ((TypeName *) def->arg)->name;
-	else
-		string = NULL;
-#if 0
-	elog(ERROR, "Define: \"%s\" = what?", def->defname);
-#endif
-
-	return string;
+			snprintf(str, 32, "%ld", (long) intVal(def->arg));
+			return str;
+		}
+		case T_Float:
+			/* T_Float values are kept in string form, so this type cheat
+			 * works (and doesn't risk losing precision)
+			 */
+			return strVal(def->arg);
+		case T_String:
+			return strVal(def->arg);
+		case T_TypeName:
+			return TypeNameToInternalName((TypeName *) def->arg);
+		default:
+			elog(ERROR, "Define: cannot interpret argument of \"%s\"",
+				 def->defname);
+	}
+	return NULL;				/* keep compiler quiet */
 }

 static double
@@ -739,15 +740,32 @@ defGetNumeric(DefElem *def)
 static int
 defGetTypeLength(DefElem *def)
 {
-	if (nodeTag(def->arg) == T_Integer)
-		return intVal(def->arg);
-	else if (nodeTag(def->arg) == T_String &&
-			!strcasecmp(strVal(def->arg), "variable"))
-		return -1;				/* variable length */
-	else if (nodeTag(def->arg) == T_TypeName &&
-			!strcasecmp(((TypeName *)(def->arg))->name, "variable"))
-		return -1;
-
-	elog(ERROR, "Define: \"%s\" = what?", def->defname);
-	return -1;
+	if (def->arg == NULL)
+		elog(ERROR, "Define: \"%s\" requires a parameter",
+			 def->defname);
+	switch (nodeTag(def->arg))
+	{
+		case T_Integer:
+			return intVal(def->arg);
+		case T_Float:
+			elog(ERROR, "Define: \"%s\" requires an integral value",
+				 def->defname);
+			break;
+		case T_String:
+			if (strcasecmp(strVal(def->arg), "variable") == 0)
+				return -1;		/* variable length */
+			break;
+		case T_TypeName:
+			/* cope if grammar chooses to believe "variable" is a typename */
+			if (strcasecmp(TypeNameToInternalName((TypeName *) def->arg),
+						   "variable") == 0)
+				return -1;		/* variable length */
+			break;
+		default:
+			elog(ERROR, "Define: cannot interpret argument of \"%s\"",
+				 def->defname);
+	}
+	elog(ERROR, "Define: invalid argument for \"%s\"",
+		 def->defname);
+	return 0;					/* keep compiler quiet */
 }
--- a/src/backend/commands/remove.c
+++ b/src/backend/commands/remove.c
@@ -8,7 +8,7 @@
 *
 *
 * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/Attic/remove.c,v 1.52 2000/09/12 16:48:55 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/Attic/remove.c,v 1.53 2000/10/07 00:58:16 tgl Exp $
 *
 *-------------------------------------------------------------------------
 */
@@ -22,6 +22,7 @@
 #include "commands/comment.h"
 #include "commands/defrem.h"
 #include "miscadmin.h"
+#include "parser/parse_expr.h"
 #include "parser/parse_func.h"
 #include "utils/acl.h"
 #include "utils/syscache.h"
@@ -39,8 +40,8 @@
 */
 void
 RemoveOperator(char *operatorName,		/* operator name */
-			   char *typeName1, /* first type name */
-			   char *typeName2) /* optional second type name */
+			   char *typeName1,			/* left argument type name */
+			   char *typeName2)			/* right argument type name */
 {
 	Relation	relation;
 	HeapTuple	tup;
@@ -53,28 +54,22 @@ RemoveOperator(char *operatorName,		/* operator name */
 	{
 		typeId1 = TypeGet(typeName1, &defined);
 		if (!OidIsValid(typeId1))
-		{
 			elog(ERROR, "RemoveOperator: type '%s' does not exist", typeName1);
-			return;
-		}
 	}

 	if (typeName2)
 	{
 		typeId2 = TypeGet(typeName2, &defined);
 		if (!OidIsValid(typeId2))
-		{
 			elog(ERROR, "RemoveOperator: type '%s' does not exist", typeName2);
-			return;
-		}
 	}

 	if (OidIsValid(typeId1) && OidIsValid(typeId2))
 		oprtype = 'b';
 	else if (OidIsValid(typeId1))
-		oprtype = 'l';
-	else
 		oprtype = 'r';
+	else
+		oprtype = 'l';

 	relation = heap_openr(OperatorRelationName, RowExclusiveLock);

@@ -94,7 +89,6 @@ RemoveOperator(char *operatorName,		/* operator name */
 				 operatorName);
 #endif

-
 		/*** Delete any comments associated with this operator ***/

 		DeleteComments(tup->t_data->t_oid);
@@ -308,13 +302,12 @@ RemoveType(char *typeName)		/* type name to be removed */
 */
 void
 RemoveFunction(char *functionName,		/* function name to be removed */
-			   int nargs,
-			   List *argNameList /* list of TypeNames */ )
+			   List *argTypes)	/* list of TypeName nodes */
 {
+	int			nargs = length(argTypes);
 	Relation	relation;
 	HeapTuple	tup;
 	Oid			argList[FUNC_MAX_ARGS];
-	char	   *typename;
 	int			i;

 	if (nargs > FUNC_MAX_ARGS)
@@ -323,19 +316,20 @@ RemoveFunction(char *functionName,		/* function name to be removed */
 	MemSet(argList, 0, FUNC_MAX_ARGS * sizeof(Oid));
 	for (i = 0; i < nargs; i++)
 	{
-		typename = strVal(lfirst(argNameList));
-		argNameList = lnext(argNameList);
+		TypeName   *t = (TypeName *) lfirst(argTypes);
+		char	   *typnam = TypeNameToInternalName(t);

-		if (strcmp(typename, "opaque") == 0)
-			argList[i] = 0;
+		argTypes = lnext(argTypes);
+
+		if (strcmp(typnam, "opaque") == 0)
+			argList[i] = InvalidOid;
 		else
 		{
 			tup = SearchSysCacheTuple(TYPENAME,
-									  PointerGetDatum(typename),
+									  PointerGetDatum(typnam),
 									  0, 0, 0);
-
 			if (!HeapTupleIsValid(tup))
-				elog(ERROR, "RemoveFunction: type '%s' not found", typename);
+				elog(ERROR, "RemoveFunction: type '%s' not found", typnam);
 			argList[i] = tup->t_data->t_oid;
 		}
 	}