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.

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 4c05c80b272115175167bb361f9cf972bd23fa01..8198882b252ed3559d2ac0cae1eb6de393765057 100644 (file)
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -2372,6 +2372,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 */
@@ -2526,12 +2527,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 or streaming
         * replication is in use. We can skip the FSM in any case.
@@ -2552,8 +2555,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);
 
@@ -2564,7 +2570,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);
        }
 }
 

diff --git a/src/test/regress/expected/select_into.out b/src/test/regress/expected/select_into.out
index 503efe04fc224305c4659456bbd3affc5a0637e9..a71abf5b48d613aad4187f997ab4ea1acb0a63a2 100644 (file)
--- a/src/test/regress/expected/select_into.out
+++ b/src/test/regress/expected/select_into.out
@@ -11,3 +11,28 @@ SELECT *
    FROM onek2
    WHERE onek2.unique1 < 2;
 DROP TABLE tmp1;
+--
+-- CREATE TABLE AS/SELECT INTO as last command in a SQL function
+-- have been known to cause problems
+--
+CREATE FUNCTION make_table() RETURNS VOID
+AS $$
+  CREATE TABLE created_table AS SELECT * FROM int8_tbl;
+$$ LANGUAGE SQL;
+SELECT make_table();
+ make_table 
+------------
+ 
+(1 row)
+
+SELECT * FROM created_table;
+        q1        |        q2         
+------------------+-------------------
+              123 |               456
+              123 |  4567890123456789
+ 4567890123456789 |               123
+ 4567890123456789 |  4567890123456789
+ 4567890123456789 | -4567890123456789
+(5 rows)
+
+DROP TABLE created_table;

diff --git a/src/test/regress/sql/select_into.sql b/src/test/regress/sql/select_into.sql
index 2fa3d1e08ecd07a250203baa3aa14f3b993721d2..5763c5e1842219f7bd38d5489bd535a5db411d1d 100644 (file)
--- a/src/test/regress/sql/select_into.sql
+++ b/src/test/regress/sql/select_into.sql
@@ -15,3 +15,18 @@ SELECT *
    WHERE onek2.unique1 < 2;
 
 DROP TABLE tmp1;
+
+--
+-- CREATE TABLE AS/SELECT INTO as last command in a SQL function
+-- have been known to cause problems
+--
+CREATE FUNCTION make_table() RETURNS VOID
+AS $$
+  CREATE TABLE created_table AS SELECT * FROM int8_tbl;
+$$ LANGUAGE SQL;
+
+SELECT make_table();
+
+SELECT * FROM created_table;
+
+DROP TABLE created_table;