Re: [HACKERS] [BUGS] BUG #6379: SQL Function Causes Back-end Crash

2012-01-04 Thread Paul Ramsey
Further notes, from Andrew (RhodiumToad) on IRC about the cause of this crasher:

[12:03pm] RhodiumToad: what happens is this
[12:04pm] RhodiumToad: postquel_start know this statement doesn't
return the result, so it supplies None_Receiver as the dest-receiver
for the query
[12:04pm] RhodiumToad: however, it knows it's a plannedStmt, so it
fires up the full executor to run it
[12:05pm] RhodiumToad: and the executor allocates a new destreceiver
in its own memory context, replaces es-qd-dest with it,
[12:05pm] RhodiumToad: (the new destreceiver is the one that writes
tuples to the created table)
[12:06pm] RhodiumToad: then at executorEnd (called from postquel_end),
executor shutdown closes the new rel, _and then frees the executor's
memory context, including the destreceiver it created
[12:07pm] RhodiumToad: postquel_end doesn't know that its setting of
-dest was clobbered, so it goes to try and destroy it again, and gets
garbage (if assertions are on)
[12:07pm] RhodiumToad: if assertions weren't on, then the rDestroy
call is harmless
[12:07pm] RhodiumToad: well, mostly harmless
[12:07pm] RhodiumToad: sneaky one, that
[12:09pm] RhodiumToad: you can confirm it by tracing through that
second call to postquel_end and confirming that it's the call to
ExecutorEnd that stomps the content of qd-dest
[12:12pm] pramsey: confirmed, the pass through ExecutorEnd has
clobbered the value so there's garbage when it arrives at line 638
[12:14pm] RhodiumToad: if you trace through ExecutorEnd itself, it
should be the FreeExecutorState that does it
[12:15pm] RhodiumToad: wonder how far back this bug goes
[12:16pm] RhodiumToad: actually not very far
[12:17pm] RhodiumToad: older versions just figured that qd-dest was
always None_Receiver and therefore did not need an rDestroy call
[12:17pm] RhodiumToad: (which is a no-op for None_Receiver)
[12:17pm] pramsey: kills my 8.4
[12:17pm] RhodiumToad: so this is broken in 8.4+
[12:17pm] pramsey: ah
[12:18pm] RhodiumToad: 8.4 introduced the lazy-eval of selects in sql functions
[12:19pm] RhodiumToad: prior to that they were always run immediately
to completion
[12:19pm] RhodiumToad: that requires juggling the destreceiver a bit,
hence the bug
[12:20pm] RhodiumToad: btw, the first statement of the function
shouldn't be needed
[12:21pm] RhodiumToad: just  ... as $f$ create table foo as select 1
as x; $f$;  should be enough to break it
[12:31pm] RhodiumToad: there's no trivial fix


On Wed, Jan 4, 2012 at 11:32 AM, Paul Ramsey pram...@cleverelephant.ca wrote:
 One extra detail, my PostgreSQL is compiled with --enable-cassert.
 This is required to set off the killer function.

 On Wed, Jan 04, 2012 at 07:17:17PM +, pram...@cleverelephant.ca wrote:
 The following bug has been logged on the website:

 Bug reference:      6379
 Logged by:          Paul Ramsey
 Email address:      pram...@cleverelephant.ca
 PostgreSQL version: 9.1.2
 Operating system:   OSX 10.6.8
 Description:

 CREATE OR REPLACE FUNCTION kill_backend()
 RETURNS VOID
 AS $$
   DROP TABLE if EXISTS foo;
   CREATE TABLE foo AS SELECT * FROM pg_class LIMIT 1;
 $$ LANGUAGE 'sql';

 SELECT kill_backend();


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [BUGS] BUG #6379: SQL Function Causes Back-end Crash

2012-01-04 Thread Tom Lane
Paul Ramsey pram...@cleverelephant.ca writes:
 Further notes, from Andrew (RhodiumToad) on IRC about the cause of this 
 crasher:
 [12:31pm] RhodiumToad: there's no trivial fix

IMO the main bug here is that functions.c isn't expecting qd-dest to be
overwritten,  so we could work around it by keeping a separate private
copy of the dest pointer.  However, it would also be fair to ask whether
there's not a cleaner solution.  Perhaps the intoRel stuff should be
saving/restoring the original destreceiver instead of just blindly
overwriting it.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [BUGS] BUG #6379: SQL Function Causes Back-end Crash

2012-01-04 Thread Tom Lane
I wrote:
 Perhaps the intoRel stuff should be
 saving/restoring the original destreceiver instead of just blindly
 overwriting it.

I concluded that was the best fix, and have committed it.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers