Thread: Re: [BUGS] BUG #2129: dblink problem
Akio Iwaasa wrote:
> The following bug has been logged online:
>
> Bug reference: 2129
> Logged by: Akio Iwaasa
>
> "postgres" process terminated with "signal 11"
> because of my wrong SQL statement using "dblink".
>
> --- SQL statement(Select statement a function) ---
> select into RET *
> from dblink(''select C1,C2,C3 from TABLE01 where ... '') <<<<< 3 column
> as LINK_TABLE01(LC1 varchar(5),LC2 varchar(5),
> LC3 varchar(5),LC4 varchar(5)) ; <<<<< 4 column
The attached patch (against cvs HEAD) fixes the reported issue.
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?
If no one thinks the above is a problem, I'll commit the attached
against HEAD and stable branches back to 7.3.
Joe
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 02:20:43 -0000
***************
*** 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,621 ----
/* 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)
+ 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 */
--- 785,790 ----
***************
*** 811,816 ****
--- 810,830 ----
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;
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: (gdb) p str->data $34 = 0x8a4e5a8 "FETCH 2 FROM rmt_foo_cursor" (gdb) p PQcmdStatus(res) $35 = 0x8a447c8 "FETCH" (gdb) p PQcmdTuples(res) $36 = 0x29dada "" Any ideas why this isn't working? Thanks, Joe
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
Joe Conway <mail@joeconway.com> writes:
> 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?
I would go with accepting (and documenting) the cursor advance. Trying
to undo it seems too risky, and it's not like this is a situation that
would arise in a properly-debugged application anyway. I can't see
expending a great amount of effort on it --- protecting against a crash
seems sufficient.
regards, tom lane
Tom Lane wrote: > Joe Conway <mail@joeconway.com> writes: > >>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? > > I would go with accepting (and documenting) the cursor advance. Trying > to undo it seems too risky, and it's not like this is a situation that > would arise in a properly-debugged application anyway. I can't see > expending a great amount of effort on it --- protecting against a crash > seems sufficient. > OK -- applied back to 7.3. Thanks, Joe