From d23c38d46c78ddccd927cc4fef85e647df9c136b Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 4 Jan 2012 18:31:14 -0500 Subject: Make executor's SELECT INTO code save and restore original tuple receiver. As previously coded, the QueryDesc's dest pointer was left dangling (pointing at an already-freed receiver object) after ExecutorEnd. It's a bit astonishing that it took us this long to notice, and I'm not sure that the known problem case with SQL functions is the only one. Fix it by saving and restoring the original receiver pointer, which seems the most bulletproof way of ensuring any related bugs are also covered. Per bug #6379 from Paul Ramsey. Back-patch to 8.4 where the current handling of SELECT INTO was introduced. --- src/backend/executor/execMain.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) (limited to 'src/backend/executor/execMain.c') diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index d69ca83187b..aa64ae7a670 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -2842,6 +2842,7 @@ typedef struct { DestReceiver pub; /* publicly-known function pointers */ EState *estate; /* EState we are working with */ + DestReceiver *origdest; /* QueryDesc's original receiver */ Relation rel; /* Relation to write to */ int hi_options; /* heap_insert performance options */ BulkInsertState bistate; /* bulk insert state */ @@ -2999,12 +3000,14 @@ OpenIntoRel(QueryDesc *queryDesc) /* * Now replace the query's DestReceiver with one for SELECT INTO */ - queryDesc->dest = CreateDestReceiver(DestIntoRel); - myState = (DR_intorel *) queryDesc->dest; + myState = (DR_intorel *) CreateDestReceiver(DestIntoRel); Assert(myState->pub.mydest == DestIntoRel); myState->estate = estate; + myState->origdest = queryDesc->dest; myState->rel = intoRelationDesc; + queryDesc->dest = (DestReceiver *) myState; + /* * We can skip WAL-logging the insertions, unless PITR is in use. We can * skip the FSM in any case. @@ -3025,8 +3028,11 @@ CloseIntoRel(QueryDesc *queryDesc) { DR_intorel *myState = (DR_intorel *) queryDesc->dest; - /* OpenIntoRel might never have gotten called */ - if (myState && myState->pub.mydest == DestIntoRel && myState->rel) + /* + * OpenIntoRel might never have gotten called, and we also want to guard + * against double destruction. + */ + if (myState && myState->pub.mydest == DestIntoRel) { FreeBulkInsertState(myState->bistate); @@ -3037,7 +3043,11 @@ CloseIntoRel(QueryDesc *queryDesc) /* close rel, but keep lock until commit */ heap_close(myState->rel, NoLock); - myState->rel = NULL; + /* restore the receiver belonging to executor's caller */ + queryDesc->dest = myState->origdest; + + /* might as well invoke my destructor */ + intorel_destroy((DestReceiver *) myState); } } -- cgit v1.2.3