Further code review for range types patch.

Fix some bugs in coercion logic and pg_dump; more comment cleanup;
minor cosmetic improvements.

Author: tglsfdc

doc/src/sgml/datatype.sgml

@@ -4506,15 +4506,21 @@ SELECT * FROM pg_attribute
         <entry>Indicates that a function accepts any input data type.</entry>
        </row>
 
+       <row>
+        <entry><type>anyelement</></entry>
+        <entry>Indicates that a function accepts any data type
+        (see <xref linkend="extend-types-polymorphic">).</entry>
+       </row>
+
        <row>
         <entry><type>anyarray</></entry>
         <entry>Indicates that a function accepts any array data type
         (see <xref linkend="extend-types-polymorphic">).</entry>
        </row>
 
        <row>
-        <entry><type>anyelement</></entry>
-        <entry>Indicates that a function accepts any data type
+        <entry><type>anynonarray</></entry>
+        <entry>Indicates that a function accepts any non-array data type
         (see <xref linkend="extend-types-polymorphic">).</entry>
        </row>
 
@@ -4532,12 +4538,6 @@ SELECT * FROM pg_attribute
         <xref linkend="rangetypes">).</entry>
        </row>
 
-       <row>
-        <entry><type>anynonarray</></entry>
-        <entry>Indicates that a function accepts any non-array data type
-        (see <xref linkend="extend-types-polymorphic">).</entry>
-       </row>
-
        <row>
         <entry><type>cstring</></entry>
         <entry>Indicates that a function accepts or returns a null-terminated C string.</entry>
@@ -4595,9 +4595,9 @@ SELECT * FROM pg_attribute
     languages all forbid use of a pseudo-type as argument type, and allow
     only <type>void</> and <type>record</> as a result type (plus
     <type>trigger</> when the function is used as a trigger).  Some also
-    support polymorphic functions using the types <type>anyarray</>,
-    <type>anyelement</>, <type>anyenum</>, <type>anyrange</>, and
-    <type>anynonarray</>.
+    support polymorphic functions using the types <type>anyelement</>,
+    <type>anyarray</>, <type>anynonarray</>, <type>anyenum</>, and
+    <type>anyrange</>.
    </para>
 
    <para>

src/backend/catalog/pg_proc.c

@@ -91,7 +91,7 @@ ProcedureCreate(const char *procedureName,
 	int			parameterCount;
 	int			allParamCount;
 	Oid		   *allParams;
-	char	   *modes = NULL;
+	char	   *paramModes = NULL;
 	bool		genericInParam = false;
 	bool		genericOutParam = false;
 	bool		anyrangeInParam = false;
@@ -130,6 +130,7 @@ ProcedureCreate(const char *procedureName,
 							   FUNC_MAX_ARGS)));
 	/* note: the above is correct, we do NOT count output arguments */
 
+	/* Deconstruct array inputs */
 	if (allParameterTypes != PointerGetDatum(NULL))
 	{
 		/*
@@ -169,28 +170,27 @@ ProcedureCreate(const char *procedureName,
 			ARR_HASNULL(modesArray) ||
 			ARR_ELEMTYPE(modesArray) != CHAROID)
 			elog(ERROR, "parameterModes is not a 1-D char array");
-		modes = (char *) ARR_DATA_PTR(modesArray);
+		paramModes = (char *) ARR_DATA_PTR(modesArray);
 	}
 
-
 	/*
-	 * Do not allow polymorphic return type unless at least one input argument
-	 * is polymorphic.	Also, do not allow return type INTERNAL unless at
-	 * least one input argument is INTERNAL.
+	 * Detect whether we have polymorphic or INTERNAL arguments.  The first
+	 * loop checks input arguments, the second output arguments.
 	 */
 	for (i = 0; i < parameterCount; i++)
 	{
 		switch (parameterTypes->values[i])
 		{
-			case ANYRANGEOID:
-				anyrangeInParam = true;
-				/* FALL THROUGH */
 			case ANYARRAYOID:
 			case ANYELEMENTOID:
 			case ANYNONARRAYOID:
 			case ANYENUMOID:
 				genericInParam = true;
 				break;
+			case ANYRANGEOID:
+				genericInParam = true;
+				anyrangeInParam = true;
+				break;
 			case INTERNALOID:
 				internalInParam = true;
 				break;
@@ -201,30 +201,37 @@ ProcedureCreate(const char *procedureName,
 	{
 		for (i = 0; i < allParamCount; i++)
 		{
-			if (modes == NULL ||
-				(modes[i] != PROARGMODE_OUT &&
-				 modes[i] != PROARGMODE_INOUT &&
-				 modes[i] != PROARGMODE_TABLE))
-				 continue;
+			if (paramModes == NULL ||
+				paramModes[i] == PROARGMODE_IN ||
+				paramModes[i] == PROARGMODE_VARIADIC)
+				continue;		/* ignore input-only params */
 
 			switch (allParams[i])
 			{
-				case ANYRANGEOID:
-					anyrangeOutParam = true;
-					/* FALL THROUGH */
 				case ANYARRAYOID:
 				case ANYELEMENTOID:
 				case ANYNONARRAYOID:
 				case ANYENUMOID:
 					genericOutParam = true;
 					break;
+				case ANYRANGEOID:
+					genericOutParam = true;
+					anyrangeOutParam = true;
+					break;
 				case INTERNALOID:
 					internalOutParam = true;
 					break;
 			}
 		}
 	}
 
+	/*
+	 * Do not allow polymorphic return type unless at least one input argument
+	 * is polymorphic.  ANYRANGE return type is even stricter: must have an
+	 * ANYRANGE input (since we can't deduce the specific range type from
+	 * ANYELEMENT).  Also, do not allow return type INTERNAL unless at least
+	 * one input argument is INTERNAL.
+	 */
 	if ((IsPolymorphicType(returnType) || genericOutParam)
 		&& !genericInParam)
 		ereport(ERROR,
@@ -259,7 +266,7 @@ ProcedureCreate(const char *procedureName,
 						procedureName,
 						format_type_be(parameterTypes->values[0]))));
 
-	if (modes != NULL)
+	if (paramModes != NULL)
 	{
 		/*
 		 * Only the last input parameter can be variadic; if it is, save its
@@ -268,7 +275,7 @@ ProcedureCreate(const char *procedureName,
 		 */
 		for (i = 0; i < allParamCount; i++)
 		{
-			switch (modes[i])
+			switch (paramModes[i])
 			{
 				case PROARGMODE_IN:
 				case PROARGMODE_INOUT:
@@ -298,7 +305,7 @@ ProcedureCreate(const char *procedureName,
 					}
 					break;
 				default:
-					elog(ERROR, "invalid parameter mode '%c'", modes[i]);
+					elog(ERROR, "invalid parameter mode '%c'", paramModes[i]);
 					break;
 			}
 		}

src/backend/commands/typecmds.c

@@ -654,9 +654,9 @@ RemoveTypeById(Oid typeOid)
 		EnumValuesDelete(typeOid);
 
 	/*
-	 * If it is a range type, delete the pg_range entries too; we don't bother
-	 * with making dependency entries for those, so it has to be done "by
-	 * hand" here.
+	 * If it is a range type, delete the pg_range entry too; we don't bother
+	 * with making a dependency entry for that, so it has to be done "by hand"
+	 * here.
 	 */
 	if (((Form_pg_type) GETSTRUCT(tup))->typtype == TYPTYPE_RANGE)
 		RangeDelete(typeOid);
@@ -1475,8 +1475,9 @@ makeRangeConstructor(char *name, Oid namespace, Oid rangeOid, Oid subtype)
 								  0.0); /* prorows */
 
 		/*
-		 * Make the constructor internally-dependent on the range type so that
-		 * the user doesn't have to treat them as separate objects.
+		 * Make the constructors internally-dependent on the range type so
+		 * that they go away silently when the type is dropped.  Note that
+		 * pg_dump depends on this choice to avoid dumping the constructors.
 		 */
 		myself.classId = ProcedureRelationId;
 		myself.objectId = procOid;