Require callers of coerce_to_domain() to supply base type/typmod.

tglsfdc · tglsfdc · commit ba0da16bd054 · 2025-01-29T15:42:25.000-05:00
In view of the issue fixed in commit 0da39aa, it no longer seems like a great idea for coerce_to_domain() to offer to perform a lookup that its caller probably should have done already. The caller should be providing a value of the domain's base type, so it's hard to envision a valid case where it hasn't looked up that type. After 0da39aa there is only one caller using the option for internal lookup, and that one can trivially be rearranged to not do that. So this seems more like a bug-encouraging misfeature than a useful shortcut; let's get rid of it (in HEAD only, there's no need to break any external callers in back branches). Discussion: https://postgr.es/m/1865579.1738113656@sss.pgh.pa.us
diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c
@@ -414,6 +414,12 @@ coerce_type(ParseState *pstate, Node *node,
 									 &funcId);
 	if (pathtype != COERCION_PATH_NONE)
 	{
+		Oid			baseTypeId;
+		int32		baseTypeMod;
+
+		baseTypeMod = targetTypeMod;
+		baseTypeId = getBaseTypeAndTypmod(targetTypeId, &baseTypeMod);
+
 		if (pathtype != COERCION_PATH_RELABELTYPE)
 		{
 			/*
@@ -423,12 +429,6 @@ coerce_type(ParseState *pstate, Node *node,
 			 * and we need to extract the correct typmod to use from the
 			 * domain's typtypmod.
 			 */
-			Oid			baseTypeId;
-			int32		baseTypeMod;
-
-			baseTypeMod = targetTypeMod;
-			baseTypeId = getBaseTypeAndTypmod(targetTypeId, &baseTypeMod);
-
 			result = build_coercion_expression(node, pathtype, funcId,
 											   baseTypeId, baseTypeMod,
 											   ccontext, cformat, location);
@@ -454,7 +454,8 @@ coerce_type(ParseState *pstate, Node *node,
 			 * that must be accounted for.  If the destination is a domain
 			 * then we won't need a RelabelType node.
 			 */
-			result = coerce_to_domain(node, InvalidOid, -1, targetTypeId,
+			result = coerce_to_domain(node, baseTypeId, baseTypeMod,
+									  targetTypeId,
 									  ccontext, cformat, location,
 									  false);
 			if (result == node)
@@ -660,10 +661,8 @@ can_coerce_type(int nargs, const Oid *input_typeids, const Oid *target_typeids,
  * Create an expression tree to represent coercion to a domain type.
  *
  * 'arg': input expression
- * 'baseTypeId': base type of domain, if known (pass InvalidOid if caller
- *		has not bothered to look this up)
- * 'baseTypeMod': base type typmod of domain, if known (pass -1 if caller
- *		has not bothered to look this up)
+ * 'baseTypeId': base type of domain
+ * 'baseTypeMod': base type typmod of domain
  * 'typeId': target type to coerce to
  * 'ccontext': context indicator to control coercions
  * 'cformat': coercion display format
@@ -679,9 +678,8 @@ coerce_to_domain(Node *arg, Oid baseTypeId, int32 baseTypeMod, Oid typeId,
 {
 	CoerceToDomain *result;
 
-	/* Get the base type if it hasn't been supplied */
-	if (baseTypeId == InvalidOid)
-		baseTypeId = getBaseTypeAndTypmod(typeId, &baseTypeMod);
+	/* We now require the caller to supply correct baseTypeId/baseTypeMod */
+	Assert(OidIsValid(baseTypeId));
 
 	/* If it isn't a domain, return the node as it was passed in */
 	if (baseTypeId == typeId)
