Hi,
2013-08-19 21:52 keltezéssel, Boszormenyi Zoltan írta:
2013-08-19 21:21 keltezéssel, Karol Trzcionka írta:
W dniu 19.08.2013 19:56, Boszormenyi Zoltan pisze:
* Does it apply cleanly to the current git master?
No. There's a reject in src/backend/optimizer/plan/initsplan.c
Thank you, merged in attached version.
* Does it include reasonable tests?
Yes but the test fails after trying to fix the rejected chunk of the
patch.
It fails because the "HINT" was changed, fixed.
That version merges some nested "ifs" left over from earlier work.
I tried to compile your v5 patch and I got:
initsplan.c: In function ‘add_vars_to_targetlist’:
initsplan.c:216:26: warning: ‘rel’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
rel->reltargetlist = lappend(rel->reltargetlist,
^
You shouldn't change the assignment at declaration:
- RelOptInfo *rel = find_base_rel(root, var->varno);
+ RelOptInfo *rel;
...
+ if (root->parse->commandType == CMD_UPDATE)
+ {
... (code using rel)
+ }
+ rel = find_base_rel(root, varno);
Let me say it again: the new code in initsplan.c::add_vars_to_targetlist() is
fishy.
The compiler says that "rel" used on line 216 may be uninitialized.
Keeping it that way passes "make check", perhaps "rel" was initialized
in a previous iteration of "foreach(temp, vars)", possibly in the
else if (IsA(node, PlaceHolderVar))
branch, which means that "PlaceHolderInfo *phinfo" may be interpreted
as RelOptInfo *, stomping on memory.
Moving the assignment back to the declaration makes "make check"
fail with the attached regression.diffs file.
Initializing it as "RelOptInfo *rel = NULL;" makes the regression check
die with a segfault, obviously.
Change the code to avoid the warning and still produce the wanted effect.
Best regards,
Zoltán Böszörményi
--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/
***
/home/zozo/crosscolumn/review/returning-before-after/postgresql/src/test/regress/expected/returning_before_after.out
2013-08-20 15:01:03.365314482 +0200
---
/home/zozo/crosscolumn/review/returning-before-after/postgresql/src/test/regress/results/returning_before_after.out
2013-08-20 15:30:21.091641454 +0200
***************
*** 6,47 ****
);
INSERT INTO foo VALUES (1, 'x'),(2,'y');
UPDATE foo SET bar1=bar1+1 RETURNING before.*, bar1, bar2;
! bar1 | bar2 | bar1 | bar2
! ------+------+------+------
! 1 | x | 2 | x
! 2 | y | 3 | y
! (2 rows)
!
UPDATE foo SET bar1=bar1-1 RETURNING after.bar1, before.bar1*2;
! bar1 | ?column?
! ------+----------
! 1 | 4
! 2 | 6
! (2 rows)
!
UPDATE foo SET bar1=bar1+1, bar2=bar2 || 'z' RETURNING before.*, after.*;
! bar1 | bar2 | bar1 | bar2
! ------+------+------+------
! 1 | x | 2 | xz
! 2 | y | 3 | yz
! (2 rows)
!
-- check single after
UPDATE foo SET bar1=bar1+1, bar2=bar2 || 'a' RETURNING after.*;
! bar1 | bar2
! ------+------
! 3 | xza
! 4 | yza
! (2 rows)
!
-- check single before
UPDATE foo SET bar1=bar1+1, bar2=bar2 || 'b' RETURNING before.*;
! bar1 | bar2
! ------+------
! 3 | xza
! 4 | yza
! (2 rows)
!
-- it should fail
UPDATE foo SET bar1=bar1+before.bar1 RETURNING before.*;
ERROR: missing FROM-clause entry for table "before"
--- 6,22 ----
);
INSERT INTO foo VALUES (1, 'x'),(2,'y');
UPDATE foo SET bar1=bar1+1 RETURNING before.*, bar1, bar2;
! ERROR: no relation entry for relid 2
UPDATE foo SET bar1=bar1-1 RETURNING after.bar1, before.bar1*2;
! ERROR: no relation entry for relid 3
UPDATE foo SET bar1=bar1+1, bar2=bar2 || 'z' RETURNING before.*, after.*;
! ERROR: no relation entry for relid 2
-- check single after
UPDATE foo SET bar1=bar1+1, bar2=bar2 || 'a' RETURNING after.*;
! ERROR: no relation entry for relid 3
-- check single before
UPDATE foo SET bar1=bar1+1, bar2=bar2 || 'b' RETURNING before.*;
! ERROR: no relation entry for relid 2
-- it should fail
UPDATE foo SET bar1=bar1+before.bar1 RETURNING before.*;
ERROR: missing FROM-clause entry for table "before"
***************
*** 53,90 ****
^
-- test before/after aliases
UPDATE foo AS before SET bar1=bar1+1 RETURNING before.*,after.*;
! bar1 | bar2 | bar1 | bar2
! ------+------+------+------
! 5 | xzab | 5 | xzab
! 6 | yzab | 6 | yzab
! (2 rows)
!
UPDATE foo AS after SET bar1=bar1-1 RETURNING before.*,after.*;
! bar1 | bar2 | bar1 | bar2
! ------+------+------+------
! 5 | xzab | 4 | xzab
! 6 | yzab | 5 | yzab
! (2 rows)
!
-- test inheritance
CREATE TABLE foo2 (bar INTEGER) INHERITS(foo);
INSERT INTO foo2 VALUES (1,'b',5);
UPDATE foo2 SET bar1=bar1*2, bar=bar1+5, bar2=bar1::text || bar::text
RETURNING before.*, after.*, *;
! bar1 | bar2 | bar | bar1 | bar2 | bar | bar1 | bar2 | bar
! ------+------+-----+------+------+-----+------+------+-----
! 1 | b | 5 | 2 | 15 | 6 | 2 | 15 | 6
! (1 row)
!
-- check views
CREATE VIEW view_foo AS SELECT * FROM foo;
UPDATE foo SET bar1=bar1+1 RETURNING before.*, bar1, bar2;
! bar1 | bar2 | bar1 | bar2
! ------+------+------+------
! 4 | xzab | 5 | xzab
! 5 | yzab | 6 | yzab
! 2 | 15 | 3 | 15
! (3 rows)
!
CREATE TABLE foo3 (bar1 INTEGER, bar4 FLOAT);
INSERT INTO foo2 VALUES (2, 'asdf', 33);
INSERT INTO foo3 VALUES (2, 7.77);
--- 28,45 ----
^
-- test before/after aliases
UPDATE foo AS before SET bar1=bar1+1 RETURNING before.*,after.*;
! ERROR: no relation entry for relid 2
UPDATE foo AS after SET bar1=bar1-1 RETURNING before.*,after.*;
! ERROR: no relation entry for relid 2
-- test inheritance
CREATE TABLE foo2 (bar INTEGER) INHERITS(foo);
INSERT INTO foo2 VALUES (1,'b',5);
UPDATE foo2 SET bar1=bar1*2, bar=bar1+5, bar2=bar1::text || bar::text
RETURNING before.*, after.*, *;
! ERROR: no relation entry for relid 2
-- check views
CREATE VIEW view_foo AS SELECT * FROM foo;
UPDATE foo SET bar1=bar1+1 RETURNING before.*, bar1, bar2;
! ERROR: no relation entry for relid 2
CREATE TABLE foo3 (bar1 INTEGER, bar4 FLOAT);
INSERT INTO foo2 VALUES (2, 'asdf', 33);
INSERT INTO foo3 VALUES (2, 7.77);
***************
*** 104,115 ****
NOTICE: drop cascades to view view_join
CREATE TRIGGER bef_foo BEFORE UPDATE ON foo FOR EACH ROW EXECUTE PROCEDURE
returning_trig();
UPDATE foo SET bar1=bar1+1, bar2=bar2 || 'z' RETURNING before.*, after.*, *;
! bar1 | bar2 | bar1 | bar2 | bar1 | bar2
! ------+------+------+-------+------+-------
! 5 | xzab | 6 | xzabz | 36 | xzabz
! 6 | yzab | 7 | yzabz | 49 | yzabz
! (2 rows)
!
DROP TABLE foo CASCADE;
NOTICE: drop cascades to view view_foo
DROP TABLE foo3 CASCADE;
--- 59,65 ----
NOTICE: drop cascades to view view_join
CREATE TRIGGER bef_foo BEFORE UPDATE ON foo FOR EACH ROW EXECUTE PROCEDURE
returning_trig();
UPDATE foo SET bar1=bar1+1, bar2=bar2 || 'z' RETURNING before.*, after.*, *;
! ERROR: no relation entry for relid 2
DROP TABLE foo CASCADE;
NOTICE: drop cascades to view view_foo
DROP TABLE foo3 CASCADE;
***************
*** 119,144 ****
INSERT INTO t1 VALUES (DEFAULT,4,5,6);
-- check WITH statement
WITH foo AS (UPDATE t1 SET x=x*2, y=y+1, z=x+y+z RETURNING BEFORE.x,
BEFORE.y, AFTER.z) INSERT INTO t2 (x,y,z) SELECT x, y, z FROM foo RETURNING *;
! id | x | y | z
! ----+---+---+----
! 1 | 1 | 2 | 6
! 2 | 4 | 5 | 15
! (2 rows)
!
-- check UPDATE ... FROM statement
UPDATE t2 SET x = t1.x+2 FROM t1 WHERE t2.id=t1.id RETURNING after.x,
before.x;
! x | x
! ----+---
! 4 | 1
! 10 | 4
! (2 rows)
!
UPDATE t2 SET x = t1.x*2 FROM t1 WHERE t2.id=t1.id RETURNING after.*,
before.*;
! id | x | y | z | id | x | y | z
! ----+----+---+----+----+----+---+----
! 1 | 4 | 2 | 6 | 1 | 4 | 2 | 6
! 2 | 16 | 5 | 15 | 2 | 10 | 5 | 15
! (2 rows)
!
DROP TABLE t1;
DROP TABLE t2;
--- 69,79 ----
INSERT INTO t1 VALUES (DEFAULT,4,5,6);
-- check WITH statement
WITH foo AS (UPDATE t1 SET x=x*2, y=y+1, z=x+y+z RETURNING BEFORE.x,
BEFORE.y, AFTER.z) INSERT INTO t2 (x,y,z) SELECT x, y, z FROM foo RETURNING *;
! ERROR: no relation entry for relid 2
-- check UPDATE ... FROM statement
UPDATE t2 SET x = t1.x+2 FROM t1 WHERE t2.id=t1.id RETURNING after.x,
before.x;
! ERROR: no relation entry for relid 4
UPDATE t2 SET x = t1.x*2 FROM t1 WHERE t2.id=t1.id RETURNING after.*,
before.*;
! ERROR: no relation entry for relid 4
DROP TABLE t1;
DROP TABLE t2;
======================================================================
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
