Revert patch to coerce 'unknown' type parameters in the backend. As Tom

pointed out, it would need a 2nd pass after the whole query is processed to
correctly check that an unknown Param is coerced to the same target type
everywhere. Adding the 2nd pass would add a lot more code, which doesn't
seem worth the risk given that there isn't much of a use case for passing
unknown Params in the first place. The code would work without that check,
but it might be confusing and the behavior would be different from the
varparams case.

Instead, just coerce all unknown params in a PL/pgSQL USING clause to text.
That's simple, and is usually what users expect.

Revert the patch in CVS HEAD and master, and backpatch the new solution to
8.4. Unlike the previous solution, this applies easily to 8.4 too.

diff --git a/src/backend/parser/parse_param.c b/src/backend/parser/parse_param.c
index 11244f7a3e3707fa0ad539a7b5aa6cc38148f8ce..2a6bff63fa2cc03de48f32af5ee5862420a8bf38 100644 (file)
--- a/src/backend/parser/parse_param.c
+++ b/src/backend/parser/parse_param.c
@@ -17,7 +17,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/parser/parse_param.c,v 2.5 2010/08/18 12:20:15 heikki Exp $
+ *       $PostgreSQL: pgsql/src/backend/parser/parse_param.c,v 2.6 2010/08/19 16:54:43 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -36,7 +36,6 @@ typedef struct FixedParamState
 {
        Oid                *paramTypes;         /* array of parameter type OIDs */
        int                     numParams;              /* number of array entries */
-       Oid                *unknownParamTypes; /* resolved types of 'unknown' params  */
 } FixedParamState;
 
 /*
@@ -56,9 +55,6 @@ static Node *variable_paramref_hook(ParseState *pstate, ParamRef *pref);
 static Node *variable_coerce_param_hook(ParseState *pstate, Param *param,
                                                   Oid targetTypeId, int32 targetTypeMod,
                                                   int location);
-static Node *fixed_coerce_param_hook(ParseState *pstate, Param *param,
-                                                  Oid targetTypeId, int32 targetTypeMod,
-                                                  int location);
 static bool check_parameter_resolution_walker(Node *node, ParseState *pstate);
 
 
@@ -73,10 +69,9 @@ parse_fixed_parameters(ParseState *pstate,
 
        parstate->paramTypes = paramTypes;
        parstate->numParams = numParams;
-       parstate->unknownParamTypes = NULL;
        pstate->p_ref_hook_state = (void *) parstate;
        pstate->p_paramref_hook = fixed_paramref_hook;
-       pstate->p_coerce_param_hook = fixed_coerce_param_hook;
+       /* no need to use p_coerce_param_hook */
 }
 
 /*
@@ -175,83 +170,6 @@ variable_paramref_hook(ParseState *pstate, ParamRef *pref)
        return (Node *) param;
 }
 
-/*
- * Coerce a Param to a query-requested datatype, in the fixed params case.
- *
- * 'unknown' type params are coerced to the type requested, analogous to the
- * coercion of unknown constants performed in coerce_type(). We can't change
- * the param types like we do in the varparams case, so the coercion is done
- * at runtime using CoerceViaIO nodes.
- */
-static Node *
-fixed_coerce_param_hook(ParseState *pstate, Param *param,
-                                               Oid targetTypeId, int32 targetTypeMode,
-                                               int location)
-{
-       if (param->paramkind == PARAM_EXTERN && param->paramtype == UNKNOWNOID)
-       {
-               FixedParamState *parstate = (FixedParamState *) pstate->p_ref_hook_state;
-               Oid *unknownParamTypes = parstate->unknownParamTypes;
-               int                     paramno = param->paramid;
-               CoerceViaIO *iocoerce;
-
-               if (paramno <= 0 ||             /* shouldn't happen, but... */
-                       paramno > parstate->numParams)
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_UNDEFINED_PARAMETER),
-                                        errmsg("there is no parameter $%d", paramno),
-                                        parser_errposition(pstate, param->location)));
-
-               /* Allocate the array on first use */
-               if (unknownParamTypes == NULL)
-               {
-                       unknownParamTypes = palloc0(parstate->numParams * sizeof(Oid));
-                       parstate->unknownParamTypes = unknownParamTypes;
-               }
-
-               /*
-                * If the same parameter is used multiple times in the query, make
-                * sure it's always resolved to the same type. The code would cope
-                * with differing interpretations, but it might lead to surprising
-                * results. The varparams code forbids that anyway, so better be
-                * consistent.
-                */
-               if (unknownParamTypes[paramno - 1] == InvalidOid)
-               {
-                       /* We've successfully resolved the type */
-                       unknownParamTypes[paramno - 1] = targetTypeId;
-               }
-               else if (unknownParamTypes[paramno - 1] == targetTypeId)
-               {
-                       /* We previously resolved the type, and it matches */
-               }
-               else
-               {
-                       /* Ooops */
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_AMBIGUOUS_PARAMETER),
-                                        errmsg("inconsistent types deduced for parameter $%d",
-                                                       paramno),
-                                        errdetail("%s versus %s",
-                                                          format_type_be(unknownParamTypes[paramno - 1]),
-                                                          format_type_be(targetTypeId)),
-                                        parser_errposition(pstate, param->location)));
-               }
-
-               /* Build a CoerceViaIO node */
-               iocoerce = makeNode(CoerceViaIO);
-               iocoerce->arg = (Expr *) param;
-               iocoerce->resulttype = targetTypeId;
-               iocoerce->coerceformat = COERCE_IMPLICIT_CAST;
-               iocoerce->location = location;
-
-               return (Node *) iocoerce;
-       }
-
-       /* Else signal to proceed with normal coercion */
-       return NULL;
-}
-
 /*
  * Coerce a Param to a query-requested datatype, in the varparams case.
  */

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index e6cde137318e78cfaacb56f4d38838ef305b9ca1..b19db02e2828a96339527213e29e36f19aa311d1 100644 (file)
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.263 2010/08/09 18:50:10 tgl Exp $
+ *       $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.264 2010/08/19 16:54:43 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -5521,8 +5521,22 @@ exec_eval_using_params(PLpgSQL_execstate *estate, List *params)
                ppd->nulls[i] = isnull ? 'n' : ' ';
                ppd->freevals[i] = false;
 
+               if (ppd->types[i] == UNKNOWNOID)
+               {
+                       /*
+                        * Treat 'unknown' parameters as text, that's what most people
+                        * would expect. The backend can coerce unknown constants in a
+                        * more intelligent way, but not unknown Params. 
+                        */
+                       ppd->types[i] = TEXTOID;
+                       if (!isnull)
+                       {
+                               ppd->values[i] = CStringGetTextDatum((char *) ppd->values[i]);
+                               ppd->freevals[i] = true;
+                       }
+               }
                /* pass-by-ref non null values must be copied into plpgsql context */
-               if (!isnull)
+               else if (!isnull)
                {
                        int16           typLen;
                        bool            typByVal;