Re: plpgsql doesn't supply typmod for the Params it generates - Mailing list pgsql-hackers

I wrote:
> I think the appropriate fix is pretty clear: add a function similar to
> exec_get_datum_type that returns the datum's typmod, and use that to set
> paramtypmod properly.  What is worrying me is that it's not clear how
> much user-visible behavioral change will result, and therefore I'm not
> entirely comfortable with the idea of back-patching such a change into
> 9.0 --- and it wouldn't work at all in 8.4, where there simply isn't a
> good opportunity to set a typmod for parameters passed to the main
> executor (since the SPI interfaces plpgsql uses don't support that).

Attached is a proposed patch for HEAD that sets up the Param's typmod
sanely.  I've verified that this fixes the reported problem and does not
result in any changes in the regression tests, which makes me a bit more
optimistic about it ... but I'm still not convinced it'd be a good idea
to back-patch into 9.0.

            regards, tom lane

diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index 535fea9257c22e5cc54dc956ebd59a6865340a55..a80235cd2aaf76e493f125c80a15bae9bc1b11ce 100644
*** a/src/pl/plpgsql/src/pl_comp.c
--- b/src/pl/plpgsql/src/pl_comp.c
*************** make_datum_param(PLpgSQL_expr *expr, int
*** 1267,1275 ****
      param = makeNode(Param);
      param->paramkind = PARAM_EXTERN;
      param->paramid = dno + 1;
!     param->paramtype = exec_get_datum_type(estate, datum);
!     param->paramtypmod = -1;
!     param->paramcollid = exec_get_datum_collation(estate, datum);
      param->location = location;

      return (Node *) param;
--- 1267,1277 ----
      param = makeNode(Param);
      param->paramkind = PARAM_EXTERN;
      param->paramid = dno + 1;
!     exec_get_datum_type_info(estate,
!                              datum,
!                              ¶m->paramtype,
!                              ¶m->paramtypmod,
!                              ¶m->paramcollid);
      param->location = location;

      return (Node *) param;
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 1da5095507468c5c5c81fd0042e69a41edcef304..2a536b688d3efbdbde54d2e010cb706bb3fcab1f 100644
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
*************** exec_get_datum_type(PLpgSQL_execstate *e
*** 4307,4335 ****
  }

  /*
!  * exec_get_datum_collation                Get collation of a PLpgSQL_datum
   */
! Oid
! exec_get_datum_collation(PLpgSQL_execstate *estate,
!                          PLpgSQL_datum *datum)
  {
-     Oid            collid;
-
      switch (datum->dtype)
      {
          case PLPGSQL_DTYPE_VAR:
              {
                  PLpgSQL_var *var = (PLpgSQL_var *) datum;

!                 collid = var->datatype->collation;
                  break;
              }

          case PLPGSQL_DTYPE_ROW:
          case PLPGSQL_DTYPE_REC:
!             /* composite types are never collatable */
!             collid = InvalidOid;
!             break;

          case PLPGSQL_DTYPE_RECFIELD:
              {
--- 4307,4369 ----
  }

  /*
!  * exec_get_datum_type_info            Get datatype etc of a PLpgSQL_datum
!  *
!  * An extended version of exec_get_datum_type, which also retrieves the
!  * typmod and collation of the datum.
   */
! void
! exec_get_datum_type_info(PLpgSQL_execstate *estate,
!                          PLpgSQL_datum *datum,
!                          Oid *typeid, int32 *typmod, Oid *collation)
  {
      switch (datum->dtype)
      {
          case PLPGSQL_DTYPE_VAR:
              {
                  PLpgSQL_var *var = (PLpgSQL_var *) datum;

!                 *typeid = var->datatype->typoid;
!                 *typmod = var->datatype->atttypmod;
!                 *collation = var->datatype->collation;
                  break;
              }

          case PLPGSQL_DTYPE_ROW:
+             {
+                 PLpgSQL_row *row = (PLpgSQL_row *) datum;
+
+                 if (!row->rowtupdesc)    /* should not happen */
+                     elog(ERROR, "row variable has no tupdesc");
+                 /* Make sure we have a valid type/typmod setting */
+                 BlessTupleDesc(row->rowtupdesc);
+                 *typeid = row->rowtupdesc->tdtypeid;
+                 /* do NOT return the mutable typmod of a RECORD variable */
+                 *typmod = -1;
+                 /* composite types are never collatable */
+                 *collation = InvalidOid;
+                 break;
+             }
+
          case PLPGSQL_DTYPE_REC:
!             {
!                 PLpgSQL_rec *rec = (PLpgSQL_rec *) datum;
!
!                 if (rec->tupdesc == NULL)
!                     ereport(ERROR,
!                           (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
!                            errmsg("record \"%s\" is not assigned yet",
!                                   rec->refname),
!                            errdetail("The tuple structure of a not-yet-assigned record is indeterminate.")));
!                 /* Make sure we have a valid type/typmod setting */
!                 BlessTupleDesc(rec->tupdesc);
!                 *typeid = rec->tupdesc->tdtypeid;
!                 /* do NOT return the mutable typmod of a RECORD variable */
!                 *typmod = -1;
!                 /* composite types are never collatable */
!                 *collation = InvalidOid;
!                 break;
!             }

          case PLPGSQL_DTYPE_RECFIELD:
              {
*************** exec_get_datum_collation(PLpgSQL_execsta
*** 4350,4370 ****
                              (errcode(ERRCODE_UNDEFINED_COLUMN),
                               errmsg("record \"%s\" has no field \"%s\"",
                                      rec->refname, recfield->fieldname)));
!                 /* XXX there's no SPI_getcollid, as yet */
                  if (fno > 0)
!                     collid = rec->tupdesc->attrs[fno - 1]->attcollation;
                  else    /* no system column types have collation */
!                     collid = InvalidOid;
                  break;
              }

          default:
              elog(ERROR, "unrecognized dtype: %d", datum->dtype);
!             collid = InvalidOid;    /* keep compiler quiet */
              break;
      }
-
-     return collid;
  }

  /* ----------
--- 4384,4410 ----
                              (errcode(ERRCODE_UNDEFINED_COLUMN),
                               errmsg("record \"%s\" has no field \"%s\"",
                                      rec->refname, recfield->fieldname)));
!                 *typeid = SPI_gettypeid(rec->tupdesc, fno);
!                 /* XXX there's no SPI_gettypmod, for some reason */
                  if (fno > 0)
!                     *typmod = rec->tupdesc->attrs[fno - 1]->atttypmod;
!                 else
!                     *typmod = -1;
!                 /* XXX there's no SPI_getcollation either */
!                 if (fno > 0)
!                     *collation = rec->tupdesc->attrs[fno - 1]->attcollation;
                  else    /* no system column types have collation */
!                     *collation = InvalidOid;
                  break;
              }

          default:
              elog(ERROR, "unrecognized dtype: %d", datum->dtype);
!             *typeid = InvalidOid;    /* keep compiler quiet */
!             *typmod = -1;
!             *collation = InvalidOid;
              break;
      }
  }

  /* ----------
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index e50d7eb45fedec49a0f675fcd19359100fcc314b..89103aea8c5d3fa967ff2ab4a23105039dfe3a8d 100644
*** a/src/pl/plpgsql/src/plpgsql.h
--- b/src/pl/plpgsql/src/plpgsql.h
*************** extern void plpgsql_subxact_cb(SubXactEv
*** 905,912 ****
                     SubTransactionId parentSubid, void *arg);
  extern Oid exec_get_datum_type(PLpgSQL_execstate *estate,
                      PLpgSQL_datum *datum);
! extern Oid exec_get_datum_collation(PLpgSQL_execstate *estate,
!                          PLpgSQL_datum *datum);

  /* ----------
   * Functions for namespace handling in pl_funcs.c
--- 905,913 ----
                     SubTransactionId parentSubid, void *arg);
  extern Oid exec_get_datum_type(PLpgSQL_execstate *estate,
                      PLpgSQL_datum *datum);
! extern void exec_get_datum_type_info(PLpgSQL_execstate *estate,
!                          PLpgSQL_datum *datum,
!                          Oid *typeid, int32 *typmod, Oid *collation);

  /* ----------
   * Functions for namespace handling in pl_funcs.c