Re: [BUGS] BUG #2129: dblink problem - Mailing list pgsql-patches

From Joe Conway
Subject Re: [BUGS] BUG #2129: dblink problem
Date
Msg-id 43BAD0D9.2010608@joeconway.com
Whole thread Raw
In response to Re: [BUGS] BUG #2129: dblink problem  (Joe Conway <mail@joeconway.com>)
Responses Re: [BUGS] BUG #2129: dblink problem  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-patches
Joe Conway wrote:
> Joe Conway wrote:
>> However, there is a remaining oddity with dblink_fetch(). Basically,
>> each time dblink_fetch() is called, the named cursor is advanced, even
>> though an error is thrown before returning any rows. Is there a simple
>> way to get the number of columns in the result, without actually
>> advancing the cursor?
>
> I thought I could work around this issue by obtaining the count returned
> for the FETCH using PQcmdTuples(), and then issuing a "MOVE BACWARD
> n..." in the case where the return tuple doesn't match. However I get an
> empty string:

The attached seems to work OK, but I'm concerned about these passages
from the docs:

"SCROLL specifies that the cursor may be used to retrieve rows in a
nonsequential fashion (e.g., backward). Depending upon the complexity of
the query's execution plan, specifying SCROLL may impose a performance
penalty on the query's execution time. NO SCROLL specifies that the
cursor cannot be used to retrieve rows in a nonsequential fashion."

" The SCROLL option should be specified when defining a cursor that will
be used to fetch backwards. This is required by the SQL standard.
However, for compatibility with earlier versions, PostgreSQL will allow
backward fetches without SCROLL, if the cursor's query plan is simple
enough that no extra overhead is needed to support it. However,
application developers are advised not to rely on using backward fetches
from a cursor that has not been created with SCROLL. If NO SCROLL is
specified, then backward fetches are disallowed in any case."

So it seems, to fix the cursor issue properly I'd have to force the
SCROLL option to be used, thereby imposing a performance penalty.

Should I just accept that the cursor advances on a row type mismatch
error, fix using the attached patch and adding SCROLL to dblink_open(),
or something else? Any opinions out there?

Thanks,

Joe
? .deps
? current.diff
? dblink.sql
? libdblink.so.0.0
? reproduce-bug.sql
? results
Index: README.dblink
===================================================================
RCS file: /cvsroot/pgsql/contrib/dblink/README.dblink,v
retrieving revision 1.12
diff -c -r1.12 README.dblink
*** README.dblink    1 Jan 2005 20:44:11 -0000    1.12
--- README.dblink    3 Jan 2006 18:14:55 -0000
***************
*** 8,14 ****
   * Darko Prenosil <Darko.Prenosil@finteh.hr>
   * Shridhar Daithankar <shridhar_daithankar@persistent.co.in>
   *
!  * Copyright (c) 2001-2005, PostgreSQL Global Development Group
   * ALL RIGHTS RESERVED;
   *
   * Permission to use, copy, modify, and distribute this software and its
--- 8,14 ----
   * Darko Prenosil <Darko.Prenosil@finteh.hr>
   * Shridhar Daithankar <shridhar_daithankar@persistent.co.in>
   *
!  * Copyright (c) 2001-2006, PostgreSQL Global Development Group
   * ALL RIGHTS RESERVED;
   *
   * Permission to use, copy, modify, and distribute this software and its
Index: dblink.c
===================================================================
RCS file: /cvsroot/pgsql/contrib/dblink/dblink.c,v
retrieving revision 1.50
diff -c -r1.50 dblink.c
*** dblink.c    22 Nov 2005 18:17:04 -0000    1.50
--- dblink.c    3 Jan 2006 18:14:56 -0000
***************
*** 8,14 ****
   * Darko Prenosil <Darko.Prenosil@finteh.hr>
   * Shridhar Daithankar <shridhar_daithankar@persistent.co.in>
   *
!  * Copyright (c) 2001-2005, PostgreSQL Global Development Group
   * ALL RIGHTS RESERVED;
   *
   * Permission to use, copy, modify, and distribute this software and its
--- 8,14 ----
   * Darko Prenosil <Darko.Prenosil@finteh.hr>
   * Shridhar Daithankar <shridhar_daithankar@persistent.co.in>
   *
!  * Copyright (c) 2001-2006, PostgreSQL Global Development Group
   * ALL RIGHTS RESERVED;
   *
   * Permission to use, copy, modify, and distribute this software and its
***************
*** 579,592 ****
          /* got results, keep track of them */
          funcctx->user_fctx = res;

-         /* fast track when no results */
-         if (funcctx->max_calls < 1)
-         {
-             if (res)
-                 PQclear(res);
-             SRF_RETURN_DONE(funcctx);
-         }
-
          /* get a tuple descriptor for our result type */
          switch (get_call_result_type(fcinfo, NULL, &tupdesc))
          {
--- 579,584 ----
***************
*** 609,614 ****
--- 601,647 ----
          /* make sure we have a persistent copy of the tupdesc */
          tupdesc = CreateTupleDescCopy(tupdesc);

+         /* check result and tuple descriptor have the same number of columns */
+         if (PQnfields(res) != tupdesc->natts)
+         {
+             if (funcctx->max_calls > 0)
+             {
+                 StringInfo    movestr = makeStringInfo();
+                 int        moveback;
+
+                 if (howmany == funcctx->max_calls)
+                     moveback = funcctx->max_calls;
+                 else
+                     moveback = funcctx->max_calls + 1;
+
+                 appendStringInfo(movestr, "MOVE BACKWARD %d FROM %s", moveback, curname);
+                 res = PQexec(conn, movestr->data);
+                 if (!res || PQresultStatus(res) != PGRES_COMMAND_OK)
+                 {
+                     if (fail)
+                         DBLINK_RES_ERROR("sql error");
+                     else
+                     {
+                         DBLINK_RES_ERROR_AS_NOTICE("sql error");
+                         SRF_RETURN_DONE(funcctx);
+                     }
+                 }
+             }
+
+             ereport(ERROR,
+                     (errcode(ERRCODE_DATATYPE_MISMATCH),
+                 errmsg("remote query result rowtype does not match "
+                         "the specified FROM clause rowtype")));
+         }
+
+         /* fast track when no results */
+         if (funcctx->max_calls < 1)
+         {
+             if (res)
+                 PQclear(res);
+             SRF_RETURN_DONE(funcctx);
+         }
+
          /* store needed metadata for subsequent calls */
          attinmeta = TupleDescGetAttInMetadata(tupdesc);
          funcctx->attinmeta = attinmeta;
***************
*** 778,791 ****
          if (freeconn)
              PQfinish(conn);

-         /* fast track when no results */
-         if (funcctx->max_calls < 1)
-         {
-             if (res)
-                 PQclear(res);
-             SRF_RETURN_DONE(funcctx);
-         }
-
          if (!is_sql_cmd)
          {
              /* get a tuple descriptor for our result type */
--- 811,816 ----
***************
*** 811,816 ****
--- 836,856 ----
              tupdesc = CreateTupleDescCopy(tupdesc);
          }

+         /* check result and tuple descriptor have the same number of columns */
+         if (PQnfields(res) != tupdesc->natts)
+             ereport(ERROR,
+                     (errcode(ERRCODE_DATATYPE_MISMATCH),
+                 errmsg("remote query result rowtype does not match "
+                         "the specified FROM clause rowtype")));
+
+         /* fast track when no results */
+         if (funcctx->max_calls < 1)
+         {
+             if (res)
+                 PQclear(res);
+             SRF_RETURN_DONE(funcctx);
+         }
+
          /* store needed metadata for subsequent calls */
          attinmeta = TupleDescGetAttInMetadata(tupdesc);
          funcctx->attinmeta = attinmeta;
Index: dblink.h
===================================================================
RCS file: /cvsroot/pgsql/contrib/dblink/dblink.h,v
retrieving revision 1.13
diff -c -r1.13 dblink.h
*** dblink.h    1 Jan 2005 05:43:05 -0000    1.13
--- dblink.h    3 Jan 2006 18:14:56 -0000
***************
*** 8,14 ****
   * Darko Prenosil <Darko.Prenosil@finteh.hr>
   * Shridhar Daithankar <shridhar_daithankar@persistent.co.in>
   *
!  * Copyright (c) 2001-2005, PostgreSQL Global Development Group
   * ALL RIGHTS RESERVED;
   *
   * Permission to use, copy, modify, and distribute this software and its
--- 8,14 ----
   * Darko Prenosil <Darko.Prenosil@finteh.hr>
   * Shridhar Daithankar <shridhar_daithankar@persistent.co.in>
   *
!  * Copyright (c) 2001-2006, PostgreSQL Global Development Group
   * ALL RIGHTS RESERVED;
   *
   * Permission to use, copy, modify, and distribute this software and its

pgsql-patches by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: Stats collector performance improvement
Next
From: Neil Conway
Date:
Subject: Re: TODO item: list prepared queries