patch fixing the old RETURN NEXT bug
Hello All,
I'm proposing the fix of this bug:
http://archives.postgresql.org/pgsql-hackers/2005-02/msg00498.php
The exact SQL code exposing the error:
----------
create table usno (ra real, dec real, bmag real, rmag real,ipix int8);
CREATE OR REPLACE FUNCTION xxx(refcursor) RETURNS refcursor AS '
DECLARE query varchar;
BEGIN
query = ''SELECT * FROM usno'';
OPEN $1 FOR EXECUTE query;
RETURN $1;
END;
' LANGUAGE plpgsql;
CREATE OR REPLACE FUNCTION yyy() RETURNS SETOF usno AS '
DECLARE rec record;
DECLARE cur refcursor;
BEGIN
cur=xxx(''curs_name'');
LOOP
FETCH cur into rec;
EXIT WHEN NOT FOUND;
RETURN NEXT rec;
END LOOP;
RETURN;
END;
' LANGUAGE plpgsql;
insert into usno values(1,2,3,4);
select * from yyy();
alter table usno add column errbox box;
select * from yyy();
alter table usno drop column errbox;
select * from yyy();
-------
The problem with that is in fact in pl_exec.c in function
compatible_tupdesc(), which do not check for the deleted attributes.
The patch is attached.
Regards,
Sergey
*****************************************************
Sergey E. Koposov
Max Planck Institute for Astronomy
Web: http://lnfm1.sai.msu.ru/~math
E-mail: math@sai.msu.ru
Attachments:
pl_exec.difftext/plain; charset=US-ASCII; name=pl_exec.diffDownload
Index: pl_exec.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v
retrieving revision 1.160
diff -c -r1.160 pl_exec.c
*** pl_exec.c 10 Jan 2006 18:50:43 -0000 1.160
--- pl_exec.c 12 Feb 2006 17:13:39 -0000
***************
*** 4469,4483 ****
static bool
compatible_tupdesc(TupleDesc td1, TupleDesc td2)
{
! int i;
! if (td1->natts != td2->natts)
! return false;
!
! for (i = 0; i < td1->natts; i++)
{
! if (td1->attrs[i]->atttypid != td2->attrs[i]->atttypid)
return false;
}
return true;
--- 4469,4507 ----
static bool
compatible_tupdesc(TupleDesc td1, TupleDesc td2)
{
! int i = 0, j = 0, natts1 = td1->natts, natts2 = td2->natts;
! while ((i < natts1) && (j < natts2))
{
! /* We should skip the dropped columns */
! if (td1->attrs[i]->attisdropped)
! {
! i++;
! continue;
! }
! if (td2->attrs[j]->attisdropped)
! {
! j++;
! continue;
! }
!
! if (td1->attrs[i]->atttypid != td2->attrs[j]->atttypid)
! {
return false;
+ }
+ else
+ {
+ i++;
+ j++;
+ }
+ }
+
+ while ((i < natts1) && (td1->attrs[i]->attisdropped)) i++;
+ while ((j < natts2) && (td2->attrs[j]->attisdropped)) j++;
+
+ if ((i != natts1) || (j != natts2))
+ {
+ return false;
}
return true;
"Sergey E. Koposov" <math@sai.msu.ru> writes:
The problem with that is in fact in pl_exec.c in function
compatible_tupdesc(), which do not check for the deleted attributes.
Is that really the only problem?
regards, tom lane
On Sun, 12 Feb 2006, Tom Lane wrote:
"Sergey E. Koposov" <math@sai.msu.ru> writes:
The problem with that is in fact in pl_exec.c in function
compatible_tupdesc(), which do not check for the deleted attributes.Is that really the only problem?
I cannot be completely sure in that, but at least the submitted patch
eliminate the problem with the SQL code from my previous email.
Regards,
Sergey
*****************************************************
Sergey E. Koposov
Max Planck Institute for Astronomy
Web: http://lnfm1.sai.msu.ru/~math
E-mail: math@sai.msu.ru
On Sun, 12 Feb 2006, Tom Lane wrote:
"Sergey E. Koposov" <math@sai.msu.ru> writes:
The problem with that is in fact in pl_exec.c in function
compatible_tupdesc(), which do not check for the deleted attributes.Is that really the only problem?
Tom,
Are there any problems with that patch to not be applied ?
(Sorry if I'm hurrying)
Regards,
Sergey
*****************************************************
Sergey E. Koposov
Max Planck Institute for Astronomy
Web: http://lnfm1.sai.msu.ru/~math
E-mail: math@sai.msu.ru
"Sergey E. Koposov" <math@sai.msu.ru> writes:
Are there any problems with that patch to not be applied ?
Hasn't been reviewed yet ... see nearby discussions about shortage
of patch reviewers ...
(Sorry if I'm hurrying)
At the moment it's not unusual for nontrivial patches to sit around
for a month or two, unless they happen to attract special interest of
one of the committers.
regards, tom lane
[redirecting to -hackers]
Tom Lane wrote:
At the moment it's not unusual for nontrivial patches to sit around
for a month or two, unless they happen to attract special interest of
one of the committers.
Yeah. That's just horrible. It makes people lose interest and can hurt
because of bitrot too. I wish we could aim at some maximum time at least
for a first cut review. If there are areas where only one or two people
have sufficient expertise, that's also a worry.
cheers
andrew
On Sun, 2006-02-12 at 20:15 +0300, Sergey E. Koposov wrote:
I'm proposing the fix of this bug:
http://archives.postgresql.org/pgsql-hackers/2005-02/msg00498.php
I think the suggested logic for compatible_tupdesc() is still wrong. For
example, the patch rejects the following:
create table usno (ra real, dec real, bmag real, rmag real, ipix int8);
create function ret_next_check() returns setof usno as $$
declare
r record;
begin
for r in select * from usno loop
return next r;
end loop;
return;
end;
$$ language plpgsql;
insert into usno values (1.0, 2.0, 3.0, 4.0, 5);
select * from ret_next_check();
alter table usno drop column ipix;
select * from ret_next_check(); -- fails, should succeed
Also, this patch should include updates to the regression tests.
-Neil
Needs cleaning up.
Your patch has been added to the PostgreSQL unapplied patches list at:
http://momjian.postgresql.org/cgi-bin/pgpatches
It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.
---------------------------------------------------------------------------
Neil Conway wrote:
On Sun, 2006-02-12 at 20:15 +0300, Sergey E. Koposov wrote:
I'm proposing the fix of this bug:
http://archives.postgresql.org/pgsql-hackers/2005-02/msg00498.phpI think the suggested logic for compatible_tupdesc() is still wrong. For
example, the patch rejects the following:create table usno (ra real, dec real, bmag real, rmag real, ipix int8);
create function ret_next_check() returns setof usno as $$
declare
r record;
begin
for r in select * from usno loop
return next r;
end loop;
return;
end;
$$ language plpgsql;insert into usno values (1.0, 2.0, 3.0, 4.0, 5);
select * from ret_next_check();
alter table usno drop column ipix;
select * from ret_next_check(); -- fails, should succeedAlso, this patch should include updates to the regression tests.
-Neil
---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match
--
Bruce Momjian http://candle.pha.pa.us
SRA OSS, Inc. http://www.sraoss.com
+ If your life is a hard drive, Christ can be your backup. +
Added to TODO:
o Fix problems with RETURN NEXT on tables with
dropped/added columns after function creation
http://archives.postgresql.org/pgsql-patches/2006-02/msg00165$
---------------------------------------------------------------------------
Sergey E. Koposov wrote:
Hello All,
I'm proposing the fix of this bug:
http://archives.postgresql.org/pgsql-hackers/2005-02/msg00498.phpThe exact SQL code exposing the error:
----------
create table usno (ra real, dec real, bmag real, rmag real,ipix int8);
CREATE OR REPLACE FUNCTION xxx(refcursor) RETURNS refcursor AS '
DECLARE query varchar;
BEGIN
query = ''SELECT * FROM usno'';
OPEN $1 FOR EXECUTE query;
RETURN $1;
END;
' LANGUAGE plpgsql;CREATE OR REPLACE FUNCTION yyy() RETURNS SETOF usno AS '
DECLARE rec record;
DECLARE cur refcursor;
BEGIN
cur=xxx(''curs_name'');
LOOP
FETCH cur into rec;
EXIT WHEN NOT FOUND;
RETURN NEXT rec;
END LOOP;
RETURN;
END;
' LANGUAGE plpgsql;insert into usno values(1,2,3,4);
select * from yyy();
alter table usno add column errbox box;
select * from yyy();
alter table usno drop column errbox;
select * from yyy();-------
The problem with that is in fact in pl_exec.c in function
compatible_tupdesc(), which do not check for the deleted attributes.The patch is attached.
Regards,
Sergey*****************************************************
Sergey E. Koposov
Max Planck Institute for Astronomy
Web: http://lnfm1.sai.msu.ru/~math
E-mail: math@sai.msu.ru
Content-Description:
[ Attachment, skipping... ]
---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend
--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +