Thread: Re: dblink - custom datatypes NOW work :)

Mark Gibson wrote:
> Joe Conway wrote:
>> Please give this a try and let me know what you think.
>
> Fantastic, works perfectly (well I've only actually tested it with
> the 'txtidx' datatype). That's so much better than my idea, i didn't
> like having the oid map much anyway. Oh well, I least I've learnt I
> little about PostgreSQL internals in the process. I can get back to
> what I was supposed to be doing now ;)
>
> I'm going to give this a much through testing now.

I'd like to consider the attached a bugfix and apply for the upcoming
7.3.6 and 7.4.2 releases, as well as cvs tip. Any comments/objections?
If not I'll apply in about 24 hours.

Thanks,

Joe
Index: contrib/dblink/dblink.c
===================================================================
RCS file: /opt/src/cvs/pgsql-server/contrib/dblink/dblink.c,v
retrieving revision 1.29
diff -c -r1.29 dblink.c
*** contrib/dblink/dblink.c    28 Nov 2003 05:03:01 -0000    1.29
--- contrib/dblink/dblink.c    13 Feb 2004 18:23:49 -0000
***************
*** 82,88 ****
  static int16 get_attnum_pk_pos(int16 *pkattnums, int16 pknumatts, int16 key);
  static HeapTuple get_tuple_of_interest(Oid relid, int16 *pkattnums, int16 pknumatts, char **src_pkattvals);
  static Oid    get_relid_from_relname(text *relname_text);
- static TupleDesc pgresultGetTupleDesc(PGresult *res);
  static char *generate_relation_name(Oid relid);

  /* Global */
--- 82,87 ----
***************
*** 395,400 ****
--- 394,400 ----
          StringInfo    str = makeStringInfo();
          char       *curname = NULL;
          int            howmany = 0;
+         ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;

          if (PG_NARGS() == 3)
          {
***************
*** 457,463 ****
          if (functyptype == 'c')
              tupdesc = TypeGetTupleDesc(functypeid, NIL);
          else if (functyptype == 'p' && functypeid == RECORDOID)
!             tupdesc = pgresultGetTupleDesc(res);
          else
              /* shouldn't happen */
              elog(ERROR, "return type must be a row type");
--- 457,472 ----
          if (functyptype == 'c')
              tupdesc = TypeGetTupleDesc(functypeid, NIL);
          else if (functyptype == 'p' && functypeid == RECORDOID)
!         {
!             if (!rsinfo)
!                 ereport(ERROR,
!                         (errcode(ERRCODE_SYNTAX_ERROR),
!                          errmsg("returning setof record is not " \
!                                 "allowed in this context")));
!
!             /* get the requested return tuple description */
!             tupdesc = CreateTupleDescCopy(rsinfo->expectedDesc);
!         }
          else
              /* shouldn't happen */
              elog(ERROR, "return type must be a row type");
***************
*** 550,555 ****
--- 559,565 ----
          char       *sql = NULL;
          char       *conname = NULL;
          remoteConn *rcon = NULL;
+         ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;

          /* create a function context for cross-call persistence */
          funcctx = SRF_FIRSTCALL_INIT();
***************
*** 620,626 ****
              if (functyptype == 'c')
                  tupdesc = TypeGetTupleDesc(functypeid, NIL);
              else if (functyptype == 'p' && functypeid == RECORDOID)
!                 tupdesc = pgresultGetTupleDesc(res);
              else
                  /* shouldn't happen */
                  elog(ERROR, "return type must be a row type");
--- 630,645 ----
              if (functyptype == 'c')
                  tupdesc = TypeGetTupleDesc(functypeid, NIL);
              else if (functyptype == 'p' && functypeid == RECORDOID)
!             {
!                 if (!rsinfo)
!                     ereport(ERROR,
!                             (errcode(ERRCODE_SYNTAX_ERROR),
!                              errmsg("returning setof record is not " \
!                                     "allowed in this context")));
!
!                 /* get the requested return tuple description */
!                 tupdesc = CreateTupleDescCopy(rsinfo->expectedDesc);
!             }
              else
                  /* shouldn't happen */
                  elog(ERROR, "return type must be a row type");
***************
*** 1801,1863 ****
      relation_close(rel, AccessShareLock);

      return relid;
- }
-
- static TupleDesc
- pgresultGetTupleDesc(PGresult *res)
- {
-     int            natts;
-     AttrNumber    attnum;
-     TupleDesc    desc;
-     char       *attname;
-     int32        atttypmod;
-     int            attdim;
-     bool        attisset;
-     Oid            atttypid;
-     int            i;
-
-     /*
-      * allocate a new tuple descriptor
-      */
-     natts = PQnfields(res);
-     if (natts < 1)
-         /* shouldn't happen */
-         elog(ERROR, "cannot create a description for empty results");
-
-     desc = CreateTemplateTupleDesc(natts, false);
-
-     attnum = 0;
-
-     for (i = 0; i < natts; i++)
-     {
-         /*
-          * for each field, get the name and type information from the
-          * query result and have TupleDescInitEntry fill in the attribute
-          * information we need.
-          */
-         attnum++;
-
-         attname = PQfname(res, i);
-         atttypid = PQftype(res, i);
-         atttypmod = PQfmod(res, i);
-
-         if (PQfsize(res, i) != get_typlen(atttypid))
-             ereport(ERROR,
-                     (errcode(ERRCODE_MOST_SPECIFIC_TYPE_MISMATCH),
-                      errmsg("field size mismatch"),
-                 errdetail("Size of remote field \"%s\" does not match " \
-                           "size of local type \"%s\".", attname,
-                           format_type_with_typemod(atttypid,
-                                                    atttypmod))));
-
-         attdim = 0;
-         attisset = false;
-
-         TupleDescInitEntry(desc, attnum, attname, atttypid,
-                            atttypmod, attdim, attisset);
-     }
-
-     return desc;
  }

  /*
--- 1820,1825 ----

Joe Conway <mail@joeconway.com> writes:
> I'd like to consider the attached a bugfix and apply for the upcoming
> 7.3.6 and 7.4.2 releases, as well as cvs tip. Any comments/objections?

Two nitpicks (each applying in 2 places):

> !             if (!rsinfo)
> !                 ereport(ERROR,
> !                         (errcode(ERRCODE_SYNTAX_ERROR),
> !                          errmsg("returning setof record is not " \
> !                                 "allowed in this context")));
> !

First, testing for null rsinfo isn't sufficient, since the resultinfo
mechanism could be used for other things; you need an IsA test too.
Second, is "syntax error" really the most appropriate classification for
this?  Compare the code in functions.c:

            ReturnSetInfo *rsi = (ReturnSetInfo *) fcinfo->resultinfo;

            if (rsi && IsA(rsi, ReturnSetInfo))
                rsi->isDone = ExprEndResult;
            else
                ereport(ERROR,
                        (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                         errmsg("set-valued function called in context that cannot accept a set")));

(Also, the errmsg text seems a bit out of line with the wording of
comparable errors, but I can't offer better text offhand.)

            regards, tom lane

Tom Lane wrote:
> Two nitpicks (each applying in 2 places):

> First, testing for null rsinfo isn't sufficient, since the resultinfo
> mechanism could be used for other things; you need an IsA test too.
> Second, is "syntax error" really the most appropriate classification for
> this?

> (Also, the errmsg text seems a bit out of line with the wording of
> comparable errors, but I can't offer better text offhand.)

Thanks for the feedback, Tom. Here's what I ended up with:

     if (!rsinfo || !IsA(rsinfo, ReturnSetInfo))
         ereport(ERROR,
                 (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                 errmsg("function returning record called in context "
                        "that cannot accept type record")));

Joe