Thread: ON COMMIT temp table handling

ON COMMIT temp table handling

From
Bruce Momjian
Date:
Gavin, I have applied your patch for ON COMMIT temp table handling;
attached. I had a few problems:

o  The code has drifted since last August, so I had to apply
some of it by hand.

o  I did some reformatting of the brackets for consistency.

o  Your code in cluster.c called heap_create_with_catalog() using:

    isTempNamespace(RelationGetNamespace(OldHeap)),

which returns a boolean, and passes it as the value for 'ateoxact',
which is not a boolean.  This call is used to create the new heap file
for the cluster operation.  I changed it to ATEOXACTNOOP.

o  Your handling of the temprels List was incorrect.  You can't use
foreach() to traverse a List when you are removing entries from the
list.  (The loop variable pointer is removed and causes havoc.) I
changed the loop to while() and added the necessary loop control
statements.  I thought you used my temprel.c code for this.  It had the
proper control statements.  Did you remove them by mistake:

  http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/backend/utils/cache/Attic/temprel.c

Regression tests and initdb pass.

---------------------------------------------------------------------------

pgman wrote:
>
> OK, I will save this for 7.4.  Sorry, Gavin.  I missed this one for 7.3.
>
> ---------------------------------------------------------------------------
>
> pgman wrote:
> > Tom Lane wrote:
> > > Mike Mascari <mascarm@mascari.com> writes:
> > > > Bruce wrote:
> > > > "Yes, someone from India has a project to test LRU-K and MRU for
> > > > large table scans and report back the results.  He will
> > > > implement whichever is best."
> > > > Did this make it into 7.3?
> > >
> > > No, we never heard back from that guy.  It is still a live topic though.
> > > One of the Red Hat people was looking at it over the summer, and I think
> > > Neil Conway is experimenting with LRU-2 code right now.
> > >
> > > > 2. Gavin Sherry had worked up a patch so that temporary
> > > > relations could be dropped automatically upon transaction
> > > > commit. Did any of those patches it make it?
> > >
> > > No they didn't; I forget whether there was any objection to his last try
> > > or it was just too late to get reviewed before feature freeze.
> >
> > I see it going into the patch queue.  Here is the full thread:
> >
> >
http://groups.google.com/groups?hl=en&lr=&ie=UTF-8&threadm=200208272124.g7RLO1L20172%40candle.pha.pa.us&rnum=1&prev=/groups%3Fq%3Dcreate%2Btemp%2Btable%2Bon%2Bcommit%2Bgroup:comp.databases.postgresql.*%26hl%3Den%26lr%3D%26ie%3DUTF-8%26scoring%3Dd%26selm%3D200208272124.g7RLO1L20172%2540candle.pha.pa.us%26rnum%3D1
> >
> > I don't see why it wasn't applied.
> >
> > --
> >   Bruce Momjian                        |  http://candle.pha.pa.us
> >   pgman@candle.pha.pa.us               |  (610) 359-1001
> >   +  If your life is a hard drive,     |  13 Roberts Road
> >   +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
>
> --
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (610) 359-1001
>   +  If your life is a hard drive,     |  13 Roberts Road
>   +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: doc/src/sgml/ref/create_table.sgml
===================================================================
RCS file: /cvsroot/pgsql-server/doc/src/sgml/ref/create_table.sgml,v
retrieving revision 1.56
diff -c -c -r1.56 create_table.sgml
*** doc/src/sgml/ref/create_table.sgml    2 Sep 2002 06:20:53 -0000    1.56
--- doc/src/sgml/ref/create_table.sgml    9 Nov 2002 23:32:07 -0000
***************
*** 21,27 ****
      | <replaceable>table_constraint</replaceable> }  [, ... ]
  )
  [ INHERITS ( <replaceable>parent_table</replaceable> [, ... ] ) ]
! [ WITH OIDS | WITHOUT OIDS ]

  where <replaceable class="PARAMETER">column_constraint</replaceable> is:

--- 21,27 ----
      | <replaceable>table_constraint</replaceable> }  [, ... ]
  )
  [ INHERITS ( <replaceable>parent_table</replaceable> [, ... ] ) ]
! [ WITH OIDS | WITHOUT OIDS ] [ON COMMIT { PRESERVE ROWS | DELETE ROWS | DROP } ]

  where <replaceable class="PARAMETER">column_constraint</replaceable> is:

***************
*** 107,116 ****
       <para>
        If specified, the table is created as a temporary table.
        Temporary tables are automatically dropped at the end of a
!       session.  Existing permanent tables with the same name are not
!       visible to the current session while the temporary table exists,
!       unless they are referenced with schema-qualified names.
!       Any indexes created on a temporary table are automatically
        temporary as well.
       </para>

--- 107,117 ----
       <para>
        If specified, the table is created as a temporary table.
        Temporary tables are automatically dropped at the end of a
!       session or optionally at the end of the current transaction
!       (See ON COMMIT below).  Existing permanent tables with the same
!       name are not visible to the current session while the temporary
!       table exists, unless they are referenced with schema-qualified
!       names. Any indexes created on a temporary table are automatically
        temporary as well.
       </para>

***************
*** 487,495 ****
       </para>
      </listitem>
     </varlistentry>
-   </variablelist>


   </refsect1>


--- 488,541 ----
       </para>
      </listitem>
     </varlistentry>

+    <varlistentry>
+     <term><literal>ON COMMIT</literal></term>
+     <listitem>
+      <para>
+       The behaviour of temporary tables at the end of a transaction
+       block can be controlled using <literal>ON COMMIT</literal>.
+       The table will exhibit the same behavior at the end of
+       transaction blocks for the duration of the session unless
+       ON COMMIT DROP is specified or the temporary table is dropped.
+      </para>
+      <para>
+       The three parameters to ON COMMIT are:
+
+       <variablelist>
+        <varlistentry>
+         <term><literal>PRESERVE ROWS</literal></term>
+         <listitem>
+          <para>
+           The rows in the temporary table will persist after the
+           transaction block.
+          </para>
+          </listitem>
+        </varlistentry>
+
+        <varlistentry>
+         <term><literal>DELETE ROWS</literal></term>
+         <listitem>
+          <para>
+           All rows in the temporary table will be deleted at the
+           end of the transaction block.
+          </para>
+         </listitem>
+        </varlistentry>

+        <varlistentry>
+         <term><literal>DROP</literal></term>
+         <listitem>
+          <para>
+           The temporary table will be dropped at the end of the transaction.
+          </para>
+         </listitem>
+        </varlistentry>
+       </variablelist>
+      </para>
+     </listitem>
+    </varlistentry>
+   </variablelist>
   </refsect1>


Index: src/backend/access/transam/xact.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/access/transam/xact.c,v
retrieving revision 1.135
diff -c -c -r1.135 xact.c
*** src/backend/access/transam/xact.c    22 Oct 2002 22:44:36 -0000    1.135
--- src/backend/access/transam/xact.c    9 Nov 2002 23:32:13 -0000
***************
*** 166,171 ****
--- 166,172 ----
  #include "catalog/index.h"
  #include "catalog/namespace.h"
  #include "commands/async.h"
+ #include "commands/tablecmds.h"
  #include "commands/trigger.h"
  #include "commands/user.h"
  #include "executor/spi.h"
***************
*** 1026,1031 ****
--- 1027,1033 ----
      AtEOXact_hash();
      AtEOXact_nbtree();
      AtEOXact_rtree();
+     AtEOXact_temp_relations(true,s->blockState);
      AtEOXact_Namespace(true);
      AtEOXact_CatCache(true);
      AtEOXact_Files();
***************
*** 1136,1141 ****
--- 1138,1144 ----
      AtEOXact_hash();
      AtEOXact_nbtree();
      AtEOXact_rtree();
+     AtEOXact_temp_relations(false,s->blockState);
      AtEOXact_Namespace(false);
      AtEOXact_CatCache(false);
      AtEOXact_Files();
Index: src/backend/bootstrap/bootparse.y
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/bootstrap/bootparse.y,v
retrieving revision 1.53
diff -c -c -r1.53 bootparse.y
*** src/backend/bootstrap/bootparse.y    1 Nov 2002 22:52:33 -0000    1.53
--- src/backend/bootstrap/bootparse.y    9 Nov 2002 23:32:14 -0000
***************
*** 34,39 ****
--- 34,40 ----
  #include "catalog/pg_class.h"
  #include "catalog/pg_namespace.h"
  #include "commands/defrem.h"
+ #include "commands/tablecmds.h"
  #include "miscadmin.h"
  #include "nodes/makefuncs.h"
  #include "nodes/nodes.h"
***************
*** 197,202 ****
--- 198,204 ----
                                                        tupdesc,
                                                        RELKIND_RELATION,
                                                        $3,
+                                                       ATEOXACTNOOP,
                                                        true);
                          elog(DEBUG3, "relation created with oid %u", id);
                      }
Index: src/backend/catalog/heap.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/catalog/heap.c,v
retrieving revision 1.232
diff -c -c -r1.232 heap.c
*** src/backend/catalog/heap.c    21 Oct 2002 22:06:18 -0000    1.232
--- src/backend/catalog/heap.c    9 Nov 2002 23:32:20 -0000
***************
*** 31,47 ****
--- 31,50 ----

  #include "access/heapam.h"
  #include "access/genam.h"
+ #include "access/xact.h"
  #include "catalog/catalog.h"
  #include "catalog/catname.h"
  #include "catalog/dependency.h"
  #include "catalog/heap.h"
  #include "catalog/index.h"
  #include "catalog/indexing.h"
+ #include "catalog/namespace.h"
  #include "catalog/pg_attrdef.h"
  #include "catalog/pg_constraint.h"
  #include "catalog/pg_inherits.h"
  #include "catalog/pg_statistic.h"
  #include "catalog/pg_type.h"
+ #include "commands/tablecmds.h"
  #include "commands/trigger.h"
  #include "miscadmin.h"
  #include "nodes/makefuncs.h"
***************
*** 670,681 ****
--- 673,686 ----
   *        creates a new cataloged relation.  see comments above.
   * --------------------------------
   */
+
  Oid
  heap_create_with_catalog(const char *relname,
                           Oid relnamespace,
                           TupleDesc tupdesc,
                           char relkind,
                           bool shared_relation,
+                          char ateoxact,  /* Only used for temp relations */
                           bool allow_system_table_mods)
  {
      Relation    pg_class_desc;
***************
*** 717,722 ****
--- 722,746 ----
      /* Assign an OID for the relation's tuple type */
      new_type_oid = newoid();

+
+     /*
+      *  Add to temprels if we are a temp relation now that we have oid
+      */
+
+     if(isTempNamespace(relnamespace)) {
+         TempTable    *t;
+         MemoryContext oldcxt;
+
+         oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
+         t = (TempTable *) palloc(sizeof(TempTable));
+         t->relid = new_rel_oid;
+         t->ateoxact = ateoxact;
+         t->tid = GetCurrentTransactionId();
+         t->dead = false;
+         reg_temp_rel(t);
+         MemoryContextSwitchTo(oldcxt);
+     }
+
      /*
       * now create an entry in pg_class for the relation.
       *
***************
*** 1146,1151 ****
--- 1170,1183 ----
      if (rel->rd_rel->relkind != RELKIND_VIEW &&
          rel->rd_rel->relkind != RELKIND_COMPOSITE_TYPE)
          smgrunlink(DEFAULT_SMGR, rel);
+
+     /*
+      * Keep temprels up to date so that we don't have ON COMMIT execution
+      * problems at the end of the next transaction block
+      */
+
+     if(isTempNamespace(RelationGetNamespace(rel)))
+         rm_temp_rel(rid);

      /*
       * Close relcache entry, but *keep* AccessExclusiveLock on the
Index: src/backend/catalog/namespace.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/catalog/namespace.c,v
retrieving revision 1.38
diff -c -c -r1.38 namespace.c
*** src/backend/catalog/namespace.c    2 Nov 2002 18:41:21 -0000    1.38
--- src/backend/catalog/namespace.c    9 Nov 2002 23:32:25 -0000
***************
*** 34,39 ****
--- 34,40 ----
  #include "catalog/pg_proc.h"
  #include "catalog/pg_shadow.h"
  #include "catalog/pg_type.h"
+ #include "commands/tablecmds.h"
  #include "lib/stringinfo.h"
  #include "miscadmin.h"
  #include "nodes/makefuncs.h"
***************
*** 1670,1675 ****
--- 1671,1677 ----

          CommitTransactionCommand(true);
      }
+     free_temp_rels();
  }


Index: src/backend/commands/cluster.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/commands/cluster.c,v
retrieving revision 1.91
diff -c -c -r1.91 cluster.c
*** src/backend/commands/cluster.c    2 Nov 2002 21:20:40 -0000    1.91
--- src/backend/commands/cluster.c    9 Nov 2002 23:32:28 -0000
***************
*** 25,30 ****
--- 25,31 ----
  #include "catalog/index.h"
  #include "catalog/indexing.h"
  #include "catalog/catname.h"
+ #include "catalog/namespace.h"
  #include "commands/cluster.h"
  #include "commands/tablecmds.h"
  #include "miscadmin.h"
***************
*** 203,208 ****
--- 204,210 ----
                                            tupdesc,
                                            OldHeap->rd_rel->relkind,
                                            OldHeap->rd_rel->relisshared,
+                                           ATEOXACTNOOP,
                                            allowSystemTableMods);

      /*
Index: src/backend/commands/tablecmds.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/commands/tablecmds.c,v
retrieving revision 1.51
diff -c -c -r1.51 tablecmds.c
*** src/backend/commands/tablecmds.c    2 Nov 2002 22:02:08 -0000    1.51
--- src/backend/commands/tablecmds.c    9 Nov 2002 23:32:59 -0000
***************
*** 14,19 ****
--- 14,20 ----
   */
  #include "postgres.h"

+ #include "access/xact.h"
  #include "access/genam.h"
  #include "access/tuptoaster.h"
  #include "catalog/catalog.h"
***************
*** 48,56 ****
  #include "utils/builtins.h"
  #include "utils/fmgroids.h"
  #include "utils/lsyscache.h"
- #include "utils/syscache.h"
  #include "utils/relcache.h"


  static List *MergeAttributes(List *schema, List *supers, bool istemp,
                  List **supOids, List **supconstr, bool *supHasOids);
--- 49,58 ----
  #include "utils/builtins.h"
  #include "utils/fmgroids.h"
  #include "utils/lsyscache.h"
  #include "utils/relcache.h"
+ #include "utils/syscache.h"

+ static List *temprels = NIL;

  static List *MergeAttributes(List *schema, List *supers, bool istemp,
                  List **supOids, List **supconstr, bool *supHasOids);
***************
*** 116,121 ****
--- 118,124 ----
      int            i;
      AttrNumber    attnum;

+
      /*
       * Truncate relname to appropriate length (probably a waste of time,
       * as parser should have done this already).
***************
*** 222,227 ****
--- 225,231 ----
                                            descriptor,
                                            relkind,
                                            false,
+                                           stmt->ateoxact,
                                            allowSystemTableMods);

      StoreCatalogInheritance(relationId, inheritOids);
***************
*** 3783,3793 ****
--- 3787,3804 ----
       * when its master is, so there's no need to handle the toast rel as
       * temp.
       */
+
+     /*
+      * Pass ATEOXACTNOOP for ateoxact since we want heap_drop_with_catalog()
+      * to remove TOAST tables for temp tables, not AtEOXact_temp_relations()
+      */
+
      toast_relid = heap_create_with_catalog(toast_relname,
                                             PG_TOAST_NAMESPACE,
                                             tupdesc,
                                             RELKIND_TOASTVALUE,
                                             shared_relation,
+                                            ATEOXACTNOOP,
                                             true);

      /* make the toast relation visible, else index creation will fail */
***************
*** 3922,3924 ****
--- 3933,4138 ----
          MAXALIGN(data_length);
      return (tuple_length > TOAST_TUPLE_THRESHOLD);
  }
+
+ /*
+  * To handle ON COMMIT { DROP | PRESERVE ROWS | DELETE ROWS }
+  */
+ void
+ AtEOXact_temp_relations(bool iscommit, int bstate)
+ {
+     List       *l,
+                *prev;
+     MemoryContext oldctx;
+
+     if (temprels == NIL)
+         return;
+
+     /*
+      *    These loops are tricky because we are removing items from the List
+      *    while we are traversing it.
+      */
+
+
+     /* Remove 'dead' entries on commit and clear 'dead' status on abort */
+     l = temprels;
+     prev = NIL;
+     while (l != NIL)
+     {
+         TempTable  *t = lfirst(l);
+
+         if (t->dead)
+         {
+             if (iscommit)
+             {
+                 /* Remove from temprels, since the user has DROP'd */
+                 oldctx = MemoryContextSwitchTo(CacheMemoryContext);
+                 if (prev == NIL)
+                 {
+                     pfree(t);
+                     temprels = lnext(l);
+                     pfree(l);
+                     l = temprels;
+                 }
+                 else
+                 {
+                     pfree(t);
+                     lnext(prev) = lnext(l);
+                     pfree(l);
+                     l = lnext(prev);
+                 }
+                 MemoryContextSwitchTo(oldctx);
+                 continue;
+             }
+             else
+                 /* user dropped but now we're aborted */
+                 t->dead = false;
+         }
+         prev = l;
+         l = lnext(l);
+     }
+
+     if ((iscommit && bstate != TBLOCK_END) ||
+         (!iscommit && bstate != TBLOCK_ABORT))
+         return;
+
+     /* Perform per-xact actions */
+     l = temprels;
+     prev = NIL;
+
+     if (iscommit)
+     {
+         while (l != NIL)
+         {
+             TempTable  *t = lfirst(l);
+
+             if (t->ateoxact == ATEOXACTDROP)
+             {
+                 ObjectAddress object;
+
+                 object.classId = RelOid_pg_class;
+                 object.objectId = t->relid;
+                 object.objectSubId = 0;
+
+                 performDeletion(&object, DROP_CASCADE);
+                 oldctx = MemoryContextSwitchTo(CacheMemoryContext);
+
+                 if (prev == NIL)
+                 {
+                     pfree(t);
+                     temprels = lnext(l);
+                     pfree(l);
+                     l = temprels;
+                 }
+                 else
+                 {
+                     pfree(t);
+                     lnext(prev) = lnext(l);
+                     pfree(l);
+                     l = lnext(prev);
+                 }
+
+                 MemoryContextSwitchTo(oldctx);
+                 CommandCounterIncrement();
+                 continue;
+             }
+             else if (t->ateoxact == ATEOXACTDELETE)
+             {
+                 heap_truncate(t->relid);
+                 CommandCounterIncrement();
+             }
+             prev = l;
+             l = lnext(l);
+         }
+     }
+     else
+     {
+         /* Abort --- remove entries added by this xact */
+         TransactionId curtid = GetCurrentTransactionId();
+
+         oldctx = MemoryContextSwitchTo(CacheMemoryContext);
+
+         while (l != NIL)
+         {
+             TempTable  *t = lfirst(l);
+
+             if (t->tid == curtid)
+             {
+                 if (prev == NIL)
+                 {
+                     pfree(t);
+                     temprels = lnext(l);
+                     pfree(l);
+                     l = temprels;
+                 }
+                 else
+                 {
+                     pfree(t);
+                     lnext(prev) = lnext(l);
+                     pfree(l);
+                     l = lnext(prev);
+                 }
+                 continue;
+             }
+             prev = l;
+             l = lnext(l);
+         }
+         MemoryContextSwitchTo(oldctx);
+     }
+ }
+
+ /*
+  * Register a temp rel in temprels
+  */
+
+ void
+ reg_temp_rel(TempTable * t)
+ {
+     temprels = lcons(t, temprels);
+ }
+
+ /*
+  * return the ON COMMIT/ateoxact value for a given temp rel
+  */
+
+ void
+ free_temp_rels(void)
+ {
+     MemoryContext oldctx;
+
+     oldctx = MemoryContextSwitchTo(CacheMemoryContext);
+     while (temprels != NIL)
+     {
+         List       *l = temprels;
+
+         temprels = lnext(temprels);
+         pfree(lfirst(l));
+         pfree(l);
+     }
+     MemoryContextSwitchTo(oldctx);
+ }
+
+ /*
+  * Remove (actually just mark for deletion, in case we abort)
+  * Relid from the temprels list
+  */
+
+ void
+ rm_temp_rel(Oid relid)
+ {
+     List       *l;
+
+     foreach(l, temprels)
+     {
+         TempTable  *t = lfirst(l);
+
+         if (t->relid == relid)
+         {
+             t->dead = true;
+             return;
+         }
+     }
+
+     /* If we get here, we're in trouble */
+     Assert(1==1);
+ }
+
Index: src/backend/executor/execMain.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/executor/execMain.c,v
retrieving revision 1.180
diff -c -c -r1.180 execMain.c
*** src/backend/executor/execMain.c    14 Oct 2002 16:51:30 -0000    1.180
--- src/backend/executor/execMain.c    9 Nov 2002 23:33:05 -0000
***************
*** 732,737 ****
--- 732,738 ----
                                               tupdesc,
                                               RELKIND_RELATION,
                                               false,
+                                              ATEOXACTNOOP,
                                               allowSystemTableMods);

                  FreeTupleDesc(tupdesc);
Index: src/backend/parser/gram.y
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/parser/gram.y,v
retrieving revision 2.373
diff -c -c -r2.373 gram.y
*** src/backend/parser/gram.y    2 Nov 2002 18:41:21 -0000    2.373
--- src/backend/parser/gram.y    9 Nov 2002 23:33:26 -0000
***************
*** 54,59 ****
--- 54,60 ----
  #include "catalog/index.h"
  #include "catalog/namespace.h"
  #include "catalog/pg_type.h"
+ #include "commands/tablecmds.h"
  #include "nodes/makefuncs.h"
  #include "nodes/params.h"
  #include "nodes/parsenodes.h"
***************
*** 224,229 ****
--- 225,231 ----
  %type <typnam>    func_arg func_return func_type aggr_argtype

  %type <boolean> opt_arg TriggerForType OptTemp OptWithOids
+ %type <chr>        OptEOXact

  %type <list>    for_update_clause opt_for_update_clause update_list
  %type <boolean>    opt_all
***************
*** 370,379 ****
      ORDER OUT_P OUTER_P OVERLAPS OVERLAY OWNER

      PARTIAL PASSWORD PATH_P PENDANT PLACING POSITION
!     PRECISION PREPARE PRIMARY PRIOR PRIVILEGES PROCEDURAL PROCEDURE

      READ REAL RECHECK REFERENCES REINDEX RELATIVE RENAME REPLACE
!     RESET RESTRICT RETURNS REVOKE RIGHT ROLLBACK ROW
      RULE

      SCHEMA SCROLL SECOND_P SECURITY SELECT SEQUENCE
--- 372,382 ----
      ORDER OUT_P OUTER_P OVERLAPS OVERLAY OWNER

      PARTIAL PASSWORD PATH_P PENDANT PLACING POSITION
!     PRECISION PRESERVE PREPARE PRIMARY PRIOR PRIVILEGES PROCEDURAL
!     PROCEDURE

      READ REAL RECHECK REFERENCES REINDEX RELATIVE RENAME REPLACE
!     RESET RESTRICT RETURNS REVOKE RIGHT ROLLBACK ROW ROWS
      RULE

      SCHEMA SCROLL SECOND_P SECURITY SELECT SEQUENCE
***************
*** 1372,1386 ****
   *****************************************************************************/

  CreateStmt:    CREATE OptTemp TABLE qualified_name '(' OptTableElementList ')'
!             OptInherit OptWithOids
                  {
                      CreateStmt *n = makeNode(CreateStmt);
                      $4->istemp = $2;
                      n->relation = $4;
                      n->tableElts = $6;
                      n->inhRelations = $8;
                      n->constraints = NIL;
                      n->hasoids = $9;
                      $$ = (Node *)n;
                  }
          | CREATE OptTemp TABLE qualified_name OF qualified_name
--- 1375,1394 ----
   *****************************************************************************/

  CreateStmt:    CREATE OptTemp TABLE qualified_name '(' OptTableElementList ')'
!             OptInherit OptWithOids OptEOXact
                  {
                      CreateStmt *n = makeNode(CreateStmt);
+
+                     if($2 == FALSE && $10 != ATEOXACTNOOP)
+                         elog(ERROR,"ON COMMIT can only be used on TEMP tables");
+
                      $4->istemp = $2;
                      n->relation = $4;
                      n->tableElts = $6;
                      n->inhRelations = $8;
                      n->constraints = NIL;
                      n->hasoids = $9;
+                     n->ateoxact = $10;
                      $$ = (Node *)n;
                  }
          | CREATE OptTemp TABLE qualified_name OF qualified_name
***************
*** 1799,1805 ****
              | /*EMPTY*/                                { $$ = TRUE; }
          ;

!
  /*
   * Note: CREATE TABLE ... AS SELECT ... is just another spelling for
   * SELECT ... INTO.
--- 1807,1817 ----
              | /*EMPTY*/                                { $$ = TRUE; }
          ;

! OptEOXact:    ON COMMIT DROP                   { $$ = ATEOXACTDROP; }
!             | ON COMMIT DELETE_P ROWS         { $$ = ATEOXACTDELETE; }
!             | ON COMMIT PRESERVE ROWS        { $$ = ATEOXACTPRESERVE; }
!             | /*EMPTY*/         { $$ = ATEOXACTNOOP; }
!        ;
  /*
   * Note: CREATE TABLE ... AS SELECT ... is just another spelling for
   * SELECT ... INTO.
***************
*** 7074,7079 ****
--- 7086,7092 ----
              | PENDANT
              | PRECISION
              | PREPARE
+             | PRESERVE
              | PRIOR
              | PRIVILEGES
              | PROCEDURAL
***************
*** 7089,7094 ****
--- 7102,7108 ----
              | RETURNS
              | REVOKE
              | ROLLBACK
+             | ROWS
              | RULE
              | SCHEMA
              | SCROLL
Index: src/backend/parser/keywords.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/parser/keywords.c,v
retrieving revision 1.127
diff -c -c -r1.127 keywords.c
*** src/backend/parser/keywords.c    18 Sep 2002 21:35:22 -0000    1.127
--- src/backend/parser/keywords.c    9 Nov 2002 23:33:28 -0000
***************
*** 232,237 ****
--- 232,238 ----
      {"position", POSITION},
      {"precision", PRECISION},
      {"prepare", PREPARE},
+     {"preserve", PRESERVE},
      {"primary", PRIMARY},
      {"prior", PRIOR},
      {"privileges", PRIVILEGES},
***************
*** 252,257 ****
--- 253,259 ----
      {"right", RIGHT},
      {"rollback", ROLLBACK},
      {"row", ROW},
+     {"rows",ROWS},
      {"rule", RULE},
      {"schema", SCHEMA},
      {"scroll", SCROLL},
Index: src/include/catalog/heap.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/catalog/heap.h,v
retrieving revision 1.57
diff -c -c -r1.57 heap.h
*** src/include/catalog/heap.h    4 Sep 2002 20:31:37 -0000    1.57
--- src/include/catalog/heap.h    9 Nov 2002 23:33:31 -0000
***************
*** 41,46 ****
--- 41,47 ----
                           TupleDesc tupdesc,
                           char relkind,
                           bool shared_relation,
+                          char ateoxact,
                           bool allow_system_table_mods);

  extern void heap_drop_with_catalog(Oid rid);
Index: src/include/commands/tablecmds.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/commands/tablecmds.h,v
retrieving revision 1.8
diff -c -c -r1.8 tablecmds.h
*** src/include/commands/tablecmds.h    21 Oct 2002 20:31:52 -0000    1.8
--- src/include/commands/tablecmds.h    9 Nov 2002 23:33:31 -0000
***************
*** 14,19 ****
--- 14,20 ----
  #ifndef TABLECMDS_H
  #define TABLECMDS_H

+ #include "access/htup.h"
  #include "nodes/parsenodes.h"

  extern void AlterTableAddColumn(Oid myrelid, bool recurse, ColumnDef *colDef);
***************
*** 61,65 ****
--- 62,91 ----

  extern void renamerel(Oid myrelid,
            const char *newrelname);
+
+ /*
+  *  Temp rel stuff
+  */
+ typedef struct TempTable
+ {
+     Oid                relid;            /* relid of temp relation */
+     char             ateoxact;        /* what to do at end of xact */
+     TransactionId    tid;            /* trans id where in rel was created */
+     bool            dead;            /* table was dropped in the current xact */
+ } TempTable;
+
+ extern void AtEOXact_temp_relations(bool iscommit, int bstate);
+ extern void reg_temp_rel(TempTable *t);
+ extern void free_temp_rels(void);
+ extern void rm_temp_rel(Oid relid);
+
+ /*
+  *  What to do at commit time for temporary relations
+  */
+
+ #define ATEOXACTNOOP        0         /* no operation at commit */
+ #define ATEOXACTPRESERVE    1        /* preserve rows */
+ #define ATEOXACTDELETE        2        /* delete rows */
+ #define ATEOXACTDROP        3        /* drop temp table */

  #endif   /* TABLECMDS_H */
Index: src/include/nodes/parsenodes.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/nodes/parsenodes.h,v
retrieving revision 1.210
diff -c -c -r1.210 parsenodes.h
*** src/include/nodes/parsenodes.h    6 Nov 2002 00:00:44 -0000    1.210
--- src/include/nodes/parsenodes.h    9 Nov 2002 23:33:38 -0000
***************
*** 919,924 ****
--- 919,925 ----
      List       *inhRelations;    /* relations to inherit from */
      List       *constraints;    /* constraints (list of Constraint nodes) */
      bool        hasoids;        /* should it have OIDs? */
+     char        ateoxact;        /* what do we do at COMMIT for TEMP ? */
  } CreateStmt;

  /* ----------

Re: ON COMMIT temp table handling

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> +     if ((iscommit && bstate != TBLOCK_END) ||
> +         (!iscommit && bstate != TBLOCK_ABORT))
> +         return;

Why is temp table handling in need of looking into xact.c's private
state?  There is no other AtEOXact routine anywhere that does this.
ISTM either the above code is wrong, or every other AtEOXact routine
is wrong.

There are some other things I don't like about the patch, but I can
fix them myself.  This one I thought I'd better ask about.

            regards, tom lane

Re: ON COMMIT temp table handling

From
Bruce Momjian
Date:
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > +     if ((iscommit && bstate != TBLOCK_END) ||
> > +         (!iscommit && bstate != TBLOCK_ABORT))
> > +         return;
>
> Why is temp table handling in need of looking into xact.c's private
> state?  There is no other AtEOXact routine anywhere that does this.
> ISTM either the above code is wrong, or every other AtEOXact routine
> is wrong.
>
> There are some other things I don't like about the patch, but I can
> fix them myself.  This one I thought I'd better ask about.

I looked at that.  The basic issue is that Gavin wants special handling
for ON COMMIT, meaning he only wants to activate the ON COMMIT code when
the transaction is committed and it is an end block, or it is not a
commit but it is an abort.

What I don't understand is how he would get into this function unless
either is true.  The two call locations would seem to be in transaction
commit/abort routines.  I have to say I am confused by the various
TBLOCK values and their progression.  I can't find any comments on them
and the code seems contorted.

Tom, can you guess on why they are there?  I hear Gavin is traveling in
Europe for the next week or two.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: ON COMMIT temp table handling

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> Why is temp table handling in need of looking into xact.c's private
>> state?  There is no other AtEOXact routine anywhere that does this.
>> ISTM either the above code is wrong, or every other AtEOXact routine
>> is wrong.

> I looked at that.  The basic issue is that Gavin wants special handling
> for ON COMMIT, meaning he only wants to activate the ON COMMIT code when
> the transaction is committed and it is an end block, or it is not a
> commit but it is an abort.

I eventually realized that the problem is that his AtEOXact routine
needs to be split in two pieces.  The actual ON COMMIT DROP or DELETE
ROWS action has to happen *before* we record transaction commit.
(Imagine what happens if we get an error while doing that part.)
But the cleanup of the list of on-commit objects should happen
afterwards.  I think if you look at the version I committed this
afternoon, you'll be much happier.

> I have to say I am confused by the various
> TBLOCK values and their progression.  I can't find any comments on them
> and the code seems contorted.

I think xact.c has more generality than it actually needs --- some of
the TBLOCK states could probably be eliminated.  I'm not really excited
about rewriting it though; it works and has worked for years.

            regards, tom lane

Re: ON COMMIT temp table handling

From
Bruce Momjian
Date:
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Tom Lane wrote:
> >> Why is temp table handling in need of looking into xact.c's private
> >> state?  There is no other AtEOXact routine anywhere that does this.
> >> ISTM either the above code is wrong, or every other AtEOXact routine
> >> is wrong.
>
> > I looked at that.  The basic issue is that Gavin wants special handling
> > for ON COMMIT, meaning he only wants to activate the ON COMMIT code when
> > the transaction is committed and it is an end block, or it is not a
> > commit but it is an abort.
>
> I eventually realized that the problem is that his AtEOXact routine
> needs to be split in two pieces.  The actual ON COMMIT DROP or DELETE
> ROWS action has to happen *before* we record transaction commit.
> (Imagine what happens if we get an error while doing that part.)
> But the cleanup of the list of on-commit objects should happen
> afterwards.  I think if you look at the version I committed this
> afternoon, you'll be much happier.

Now, that makes sense.

> > I have to say I am confused by the various
> > TBLOCK values and their progression.  I can't find any comments on them
> > and the code seems contorted.
>
> I think xact.c has more generality than it actually needs --- some of
> the TBLOCK states could probably be eliminated.  I'm not really excited
> about rewriting it though; it works and has worked for years.

Yep, over generalized is the word for it.  Let me see if I can add some
comments.


--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: ON COMMIT temp table handling

From
Gavin Sherry
Date:
Thanks Bruce,

I've been in France/Germany and haven't had internet access :-(. Hence
slow response, below.

On Sat, 9 Nov 2002, Bruce Momjian wrote:

> o  The code has drifted since last August, so I had to apply
> some of it by hand.

Thanks.

> o  Your code in cluster.c called heap_create_with_catalog() using:
>
>     isTempNamespace(RelationGetNamespace(OldHeap)),
>
> which returns a boolean, and passes it as the value for 'ateoxact',
> which is not a boolean.  This call is used to create the new heap file
> for the cluster operation.  I changed it to ATEOXACTNOOP.

I haven't looked at the patch because its a bit chaotic here after getting
back, but it occurs to me that if the cluster is on a temp table we want
to tell heap_create_with_catalog().

>
> o  Your handling of the temprels List was incorrect.  You can't use
> foreach() to traverse a List when you are removing entries from the
> list.  (The loop variable pointer is removed and causes havoc.) I
> changed the loop to while() and added the necessary loop control
> statements.  I thought you used my temprel.c code for this.  It had the
> proper control statements.  Did you remove them by mistake:
>
>   http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/backend/utils/cache/Attic/temprel.c

I must have. Oops.

Gavin


Re: ON COMMIT temp table handling

From
Bruce Momjian
Date:
Gavin Sherry wrote:
> > o  Your code in cluster.c called heap_create_with_catalog() using:
> >
> >     isTempNamespace(RelationGetNamespace(OldHeap)),
> >
> > which returns a boolean, and passes it as the value for 'ateoxact',
> > which is not a boolean.  This call is used to create the new heap file
> > for the cluster operation.  I changed it to ATEOXACTNOOP.
>
> I haven't looked at the patch because its a bit chaotic here after getting
> back, but it occurs to me that if the cluster is on a temp table we want
> to tell heap_create_with_catalog().

The flag we being passed for the creation of the new heap, which was
going to be deleted anyway when the transaction finished.  Why bother
passing anything special for it?

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: ON COMMIT temp table handling

From
Gavin Sherry
Date:
> The flag we being passed for the creation of the new heap, which was
> going to be deleted anyway when the transaction finished.  Why bother
> passing anything special for it?

I see. I should have looked at the code itself. Sorry.

Gavin


Re: ON COMMIT temp table handling

From
Bruce Momjian
Date:
Gavin Sherry wrote:
> > The flag we being passed for the creation of the new heap, which was
> > going to be deleted anyway when the transaction finished.  Why bother
> > passing anything special for it?
>
> I see. I should have looked at the code itself. Sorry.

The problem is that the flag to heap_create_with_catalog now has so many
possible values that it isn't easy to find the value that exists for
the table being clustered, and because the temp heap goes away anyway
after the command, it isn't worth trying to figure out what it is.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073