Обсуждение: ON COMMIT and foreign keys

Поиск
Список
Период
Сортировка

ON COMMIT and foreign keys

От
Alvaro Herrera
Дата:
Hackers,

There's a bug with temporary tables signalled ON COMMIT DELETE ROWS,
when they contain foreign key references.  An example:

alvherre=# begin;
BEGIN
alvherre=# CREATE TEMP TABLE foo (a int PRIMARY KEY) ON COMMIT DELETE ROWS;
NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index "foo_pkey" for table "foo"
CREATE TABLE
alvherre=# CREATE TEMP TABLE bar (a int REFERENCES foo) ON COMMIT DELETE ROWS;
CREATE TABLE
alvherre=# COMMIT;
ERROR:  cannot truncate a table referenced in a foreign key constraint
DETAIL:  Table "bar" references "foo" via foreign key constraint "bar_a_fkey".

Say again?  Certainly this shouldn't happen, because both tables are
supposed to lose rows on transaction commit.  But this isn't working.

The attached patch fixes this bug.  (In all likelyhood, not a lot of
people uses referential integrity on temp tables, and that's why this
hasn't been reported.  But it's a bug anyway.)


Incidentally ("collateral damage"), the patch modifies the TRUNCATE
command so that it can work on multiple tables.  In particular, if
foreign key references are all internal to the group that's being
truncated, the command is allowed.

There's one thing that bothers me on this patch: the fact that
pg_constraint has to be scanned multiple times, and they are all
seqscans.  Not sure what to do about that.  Maybe there's a way to do
better?

Also, observe that when the TRUNCATE operation is aborted because of a
foreign key, a HINT is emitted as well telling the user to truncate the
referencing table too.  This is IMHO a good hint, but it may be
misleading when the truncation has taken the ON COMMIT DELETE ROWS path.
Not sure if it's worth fixing (maybe the hint should suggest to add ON
COMMIT DELETE ROWS to the referencing table as well?).

Please have a look.  The patch is not as intrusive as it looks; there's
a lot of whitespace change.

--
Alvaro Herrera (<alvherre[a]dcc.uchile.cl>)
"Ellos andaban todos desnudos como su madre los parió, y también las mujeres,
aunque no vi más que una, harto moza, y todos los que yo vi eran todos
mancebos, que ninguno vi de edad de más de XXX años" (Cristóbal Colón)

Вложения

Re: ON COMMIT and foreign keys

От
Tom Lane
Дата:
Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
> Incidentally ("collateral damage"), the patch modifies the TRUNCATE
> command so that it can work on multiple tables.  In particular, if
> foreign key references are all internal to the group that's being
> truncated, the command is allowed.

I think this is a feature addition that you've cleverly managed to
present as a bug fix ;-).  While I like the feature, I've got very
mixed emotions about applying it this late in beta.  The "bug" exists
in 7.4 but has yet to be reported in the field, which says to me that
we could leave it unfixed in 8.0 as well.  Conservatism would suggest
holding the change for 8.1.  OTOH, multiple TRUNCATE would be a useful
feature --- it came up just the other day.

Comments anyone?

            regards, tom lane

Re: ON COMMIT and foreign keys

От
Bruce Momjian
Дата:
Tom Lane wrote:
> Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
> > Incidentally ("collateral damage"), the patch modifies the TRUNCATE
> > command so that it can work on multiple tables.  In particular, if
> > foreign key references are all internal to the group that's being
> > truncated, the command is allowed.
>
> I think this is a feature addition that you've cleverly managed to
> present as a bug fix ;-).  While I like the feature, I've got very
> mixed emotions about applying it this late in beta.  The "bug" exists
> in 7.4 but has yet to be reported in the field, which says to me that
> we could leave it unfixed in 8.0 as well.  Conservatism would suggest
> holding the change for 8.1.  OTOH, multiple TRUNCATE would be a useful
> feature --- it came up just the other day.
>
> Comments anyone?

Alvaro, would you post the patch skipping whitespace changes.  As posted
the patch does look very large, as you suggested.

--
  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 and foreign keys

От
Bruce Momjian
Дата:
Bruce Momjian wrote:
> Tom Lane wrote:
> > Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
> > > Incidentally ("collateral damage"), the patch modifies the TRUNCATE
> > > command so that it can work on multiple tables.  In particular, if
> > > foreign key references are all internal to the group that's being
> > > truncated, the command is allowed.
> >
> > I think this is a feature addition that you've cleverly managed to
> > present as a bug fix ;-).  While I like the feature, I've got very
> > mixed emotions about applying it this late in beta.  The "bug" exists
> > in 7.4 but has yet to be reported in the field, which says to me that
> > we could leave it unfixed in 8.0 as well.  Conservatism would suggest
> > holding the change for 8.1.  OTOH, multiple TRUNCATE would be a useful
> > feature --- it came up just the other day.
> >
> > Comments anyone?
>
> Alvaro, would you post the patch skipping whitespace changes.  As posted
> the patch does look very large, as you suggested.

I generated the diff myself, attached.

--
  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
? config.log
? GNUmakefile
? config.status
? src/Makefile.custom
? src/log
? src/Makefile.global
? src/bin/psql/psql
? src/bin/psql/po/cs.mo
? src/bin/psql/po/de.mo
? src/bin/psql/po/es.mo
? src/bin/psql/po/fa.mo
? src/bin/psql/po/fr.mo
? src/bin/psql/po/hu.mo
? src/bin/psql/po/it.mo
? src/bin/psql/po/nb.mo
? src/bin/psql/po/pt_BR.mo
? src/bin/psql/po/ru.mo
? src/bin/psql/po/sk.mo
? src/bin/psql/po/sl.mo
? src/bin/psql/po/sv.mo
? src/bin/psql/po/tr.mo
? src/bin/psql/po/zh_CN.mo
? src/bin/psql/po/zh_TW.mo
? src/include/pg_config.h
? src/include/stamp-h
? src/interfaces/libpq/libpq.so.3.2
? src/interfaces/libpq/po/af.mo
? src/interfaces/libpq/po/cs.mo
? src/interfaces/libpq/po/de.mo
? src/interfaces/libpq/po/es.mo
? src/interfaces/libpq/po/fr.mo
? src/interfaces/libpq/po/hr.mo
? src/interfaces/libpq/po/it.mo
? src/interfaces/libpq/po/nb.mo
? src/interfaces/libpq/po/pt_BR.mo
? src/interfaces/libpq/po/ru.mo
? src/interfaces/libpq/po/sk.mo
? src/interfaces/libpq/po/sl.mo
? src/interfaces/libpq/po/sv.mo
? src/interfaces/libpq/po/tr.mo
? src/interfaces/libpq/po/zh_CN.mo
? src/interfaces/libpq/po/zh_TW.mo
? src/port/pg_config_paths.h
Index: doc/src/sgml/ref/truncate.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/ref/truncate.sgml,v
retrieving revision 1.17
diff -c -c -b -r1.17 truncate.sgml
*** doc/src/sgml/ref/truncate.sgml    23 Mar 2004 13:21:41 -0000    1.17
--- doc/src/sgml/ref/truncate.sgml    6 Nov 2004 05:56:52 -0000
***************
*** 11,17 ****

   <refnamediv>
    <refname>TRUNCATE</refname>
!   <refpurpose>empty a table</refpurpose>
   </refnamediv>

   <indexterm zone="sql-truncate">
--- 11,17 ----

   <refnamediv>
    <refname>TRUNCATE</refname>
!   <refpurpose>empty a set of tables</refpurpose>
   </refnamediv>

   <indexterm zone="sql-truncate">
***************
*** 20,26 ****

   <refsynopsisdiv>
  <synopsis>
! TRUNCATE [ TABLE ] <replaceable class="PARAMETER">name</replaceable>
  </synopsis>
   </refsynopsisdiv>

--- 20,26 ----

   <refsynopsisdiv>
  <synopsis>
! TRUNCATE [ TABLE ] <replaceable class="PARAMETER">name</replaceable> [, ...]
  </synopsis>
   </refsynopsisdiv>

***************
*** 28,37 ****
    <title>Description</title>

    <para>
!    <command>TRUNCATE</command> quickly removes all rows from a
!    table. It has the same effect as an unqualified
!    <command>DELETE</command> but since it does not actually scan the
!    table it is faster. This is most useful on large tables.
    </para>
   </refsect1>

--- 28,37 ----
    <title>Description</title>

    <para>
!    <command>TRUNCATE</command> quickly removes all rows from a set of
!    tables. It has the same effect as an unqualified
!    <command>DELETE</command> on each of them, but since it does not actually
!    scan the tables it is faster. This is most useful on large tables.
    </para>
   </refsect1>

***************
*** 55,67 ****

    <para>
     <command>TRUNCATE</> cannot be used if there are foreign-key references
!    to the table from other tables.  Checking validity in such cases would
!    require table scans, and the whole point is not to do one.
    </para>

    <para>
     <command>TRUNCATE</> will not run any user-defined <literal>ON
!    DELETE</literal> triggers that might exist for the table.
    </para>
   </refsect1>

--- 55,68 ----

    <para>
     <command>TRUNCATE</> cannot be used if there are foreign-key references
!    to the table from other tables, unless all such tables are also truncated
!    in the same command.  Checking validity in such cases would require table
!    scans, and the whole point is not to do one.
    </para>

    <para>
     <command>TRUNCATE</> will not run any user-defined <literal>ON
!    DELETE</literal> triggers that might exist for the tables.
    </para>
   </refsect1>

***************
*** 69,78 ****
    <title>Examples</title>

    <para>
!    Truncate the table <literal>bigtable</literal>:

  <programlisting>
! TRUNCATE TABLE bigtable;
  </programlisting>
    </para>
   </refsect1>
--- 70,79 ----
    <title>Examples</title>

    <para>
!    Truncate the tables <literal>bigtable</literal> and <literal>fattable</literal>:

  <programlisting>
! TRUNCATE TABLE bigtable, fattable;
  </programlisting>
    </para>
   </refsect1>
Index: src/backend/catalog/heap.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/catalog/heap.c,v
retrieving revision 1.276
diff -c -c -b -r1.276 heap.c
*** src/backend/catalog/heap.c    31 Aug 2004 17:10:36 -0000    1.276
--- src/backend/catalog/heap.c    6 Nov 2004 05:56:56 -0000
***************
*** 2002,2024 ****
  /*
   *     heap_truncate
   *
!  *     This routine deletes all data within the specified relation.
   *
   * This is not transaction-safe!  There is another, transaction-safe
   * implementation in commands/cluster.c.  We now use this only for
   * ON COMMIT truncation of temporary tables, where it doesn't matter.
   */
  void
! heap_truncate(Oid rid)
  {
      Relation    rel;
-     Oid            toastrelid;

      /* Open relation for processing, and grab exclusive access on it. */
      rel = heap_open(rid, AccessExclusiveLock);

      /* Don't allow truncate on tables that are referenced by foreign keys */
!     heap_truncate_check_FKs(rel);

      /*
       * Release any buffers associated with this relation.  If they're
--- 2002,2044 ----
  /*
   *     heap_truncate
   *
!  *     This routine deletes all data within all the specified relations.
   *
   * This is not transaction-safe!  There is another, transaction-safe
   * implementation in commands/cluster.c.  We now use this only for
   * ON COMMIT truncation of temporary tables, where it doesn't matter.
   */
  void
! heap_truncate(List *relids)
  {
+     List       *relations = NIL;
+     List       *toasttables = NIL;
+     ListCell   *cell;
+
+     /*
+      * Fast path when there's nothing to truncate
+      */
+     if (list_length(relids) == 0)
+         return;
+
+     foreach (cell, relids)
+     {
+         Oid            rid = lfirst_oid(cell);
          Relation    rel;

          /* Open relation for processing, and grab exclusive access on it. */
          rel = heap_open(rid, AccessExclusiveLock);

+         relations = lappend(relations, rel);
+     }
+
      /* Don't allow truncate on tables that are referenced by foreign keys */
!     heap_truncate_check_FKs(relations);
!
!     foreach (cell, relations)
!     {
!         Relation    rel = lfirst(cell);
!         Oid            toastrelid;

          /*
           * Release any buffers associated with this relation.  If they're
***************
*** 2030,2080 ****
      RelationTruncate(rel, 0);

      /* If this relation has indexes, truncate the indexes too */
!     RelationTruncateIndexes(rid);

!     /* If it has a toast table, recursively truncate that too */
      toastrelid = rel->rd_rel->reltoastrelid;
      if (OidIsValid(toastrelid))
!         heap_truncate(toastrelid);

      /*
       * Close the relation, but keep exclusive lock on it until commit.
       */
      heap_close(rel, NoLock);
  }

  /*
   * heap_truncate_check_FKs
!  *        Check for foreign keys referencing a relation that's to be truncated
   *
   * We disallow such FKs (except self-referential ones) since the whole point
   * of TRUNCATE is to not scan the individual rows to be thrown away.
   *
   * This is split out so it can be shared by both implementations of truncate.
!  * Caller should already hold a suitable lock on the relation.
   */
  void
! heap_truncate_check_FKs(Relation rel)
  {
!     Oid            relid = RelationGetRelid(rel);
!     ScanKeyData key;
      Relation    fkeyRel;
-     SysScanDesc fkeyScan;
-     HeapTuple    tuple;

      /*
!      * Fast path: if the relation has no triggers, it surely has no FKs
!      * either.
       */
      if (rel->rd_rel->reltriggers == 0)
          return;

      /*
!      * Otherwise, must scan pg_constraint.    Right now, this is a seqscan
       * because there is no available index on confrelid.
       */
      fkeyRel = heap_openr(ConstraintRelationName, AccessShareLock);

      ScanKeyInit(&key,
                  Anum_pg_constraint_confrelid,
                  BTEqualStrategyNumber, F_OIDEQ,
--- 2050,2137 ----
          RelationTruncate(rel, 0);

          /* If this relation has indexes, truncate the indexes too */
!         RelationTruncateIndexes(RelationGetRelid(rel));

!         /*
!          * If it has a toast table, schedule it for later truncation.
!          * Note that we cannot just append it to the list being processed,
!          * because it's not open nor locked.
!          */
          toastrelid = rel->rd_rel->reltoastrelid;
          if (OidIsValid(toastrelid))
!             toasttables = lappend_oid(toasttables, toastrelid);

          /*
           * Close the relation, but keep exclusive lock on it until commit.
           */
          heap_close(rel, NoLock);
+     }
+
+     /* now truncate TOAST tables */
+     if (list_length(toasttables) > 0)
+         heap_truncate(toasttables);
  }

  /*
   * heap_truncate_check_FKs
!  *        Check for foreign keys referencing a list of relations that
!  *        have to be truncated
   *
   * We disallow such FKs (except self-referential ones) since the whole point
   * of TRUNCATE is to not scan the individual rows to be thrown away.
   *
   * This is split out so it can be shared by both implementations of truncate.
!  * Caller should already hold a suitable lock on the relations.
   */
  void
! heap_truncate_check_FKs(List *relations)
  {
!     List       *rels = NIL;
!     List       *oids = NIL;
!     ListCell   *cell;
      Relation    fkeyRel;

      /*
!      * Get the list of involved relations and their Oids.
!      */
!     foreach(cell, relations)
!     {
!         Relation    rel = lfirst(cell);
!
!         /*
!          * If a relation has no triggers, then it can't neither
!          * have FKs nor be referenced by a FK from another table,
!          * so skip it.
           */
          if (rel->rd_rel->reltriggers == 0)
+             continue;
+
+         rels = lappend(rels, rel);
+         oids = lappend_oid(oids, RelationGetRelid(rel));
+     }
+
+     /*
+      * Fast path: if no relation has triggers, none has FKs either.
+      */
+     if (list_length(rels) == 0)
          return;

      /*
!      * Otherwise, must scan pg_constraint.    Right now, these are seqscans
       * because there is no available index on confrelid.
+      *
+      * XXX -- is there a way to do it in one sweep?
       */
      fkeyRel = heap_openr(ConstraintRelationName, AccessShareLock);

+     foreach (cell, rels)
+     {
+         Relation    rel = lfirst(cell);
+         Oid            relid = RelationGetRelid(rel);
+         SysScanDesc fkeyScan;
+         HeapTuple    tuple;
+         ScanKeyData key;
+
          ScanKeyInit(&key,
                      Anum_pg_constraint_confrelid,
                      BTEqualStrategyNumber, F_OIDEQ,
***************
*** 2087,2102 ****
      {
          Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(tuple);

!         if (con->contype == CONSTRAINT_FOREIGN && con->conrelid != relid)
              ereport(ERROR,
                      (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                       errmsg("cannot truncate a table referenced in a foreign key constraint"),
                       errdetail("Table \"%s\" references \"%s\" via foreign key constraint \"%s\".",
                                 get_rel_name(con->conrelid),
                                 RelationGetRelationName(rel),
!                                NameStr(con->conname))));
      }
-
      systable_endscan(fkeyScan);
      heap_close(fkeyRel, AccessShareLock);
  }
--- 2144,2163 ----
          {
              Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(tuple);

!             if (con->contype == CONSTRAINT_FOREIGN &&
!                     ! list_member_oid(oids, con->conrelid))
                  ereport(ERROR,
                          (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                           errmsg("cannot truncate a table referenced in a foreign key constraint"),
                           errdetail("Table \"%s\" references \"%s\" via foreign key constraint \"%s\".",
                               get_rel_name(con->conrelid),
                               RelationGetRelationName(rel),
!                              NameStr(con->conname)),
!                          errhint("Truncate table \"%s\" at the same time",
!                              get_rel_name(con->conrelid))));
          }
          systable_endscan(fkeyScan);
+     }
+
      heap_close(fkeyRel, AccessShareLock);
  }
Index: src/backend/commands/tablecmds.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.139
diff -c -c -b -r1.139 tablecmds.c
*** src/backend/commands/tablecmds.c    5 Nov 2004 19:15:57 -0000    1.139
--- src/backend/commands/tablecmds.c    6 Nov 2004 05:57:08 -0000
***************
*** 524,541 ****
  }

  /*
!  * TruncateRelation
!  *        Removes all the rows from a relation.
   */
  void
! TruncateRelation(const RangeVar *relation)
  {
      Relation    rel;
-     Oid            heap_relid;
-     Oid            toast_relid;

      /* Grab exclusive lock in preparation for truncate */
!     rel = heap_openrv(relation, AccessExclusiveLock);

      /* Only allow truncate on regular tables */
      if (rel->rd_rel->relkind != RELKIND_RELATION)
--- 524,552 ----
  }

  /*
!  * ExecuteTruncate
!  *         Executes a TRUNCATE command.
!  *
!  * This is a multi-relation truncate.  It first opens and grabs exclusive
!  * locks on all relations involved, checking permissions and otherwise
!  * verifying that the relation is OK for truncation.  When they are all
!  * open, it checks foreign key references on them, namely that FK references
!  * are all internal to the group that's being truncated.  Finally all
!  * relations are truncated and reindexed.
   */
  void
! ExecuteTruncate(List *relations)
  {
+     List        *rels = NIL;
+     ListCell    *cell;
+
+     foreach (cell, relations)
+     {
+         RangeVar   *rv = lfirst(cell);
          Relation    rel;

          /* Grab exclusive lock in preparation for truncate */
!         rel = heap_openrv(rv, AccessExclusiveLock);

          /* Only allow truncate on regular tables */
          if (rel->rd_rel->relkind != RELKIND_RELATION)
***************
*** 575,589 ****
                  (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
            errmsg("cannot truncate temporary tables of other sessions")));

      /*
!      * Don't allow truncate on tables which are referenced by foreign keys
       */
!     heap_truncate_check_FKs(rel);

      /*
!      * Okay, here we go: create a new empty storage file for the relation,
!      * and assign it as the relfilenode value.    The old storage file is
!      * scheduled for deletion at commit.
       */
      setNewRelfilenode(rel);

--- 586,610 ----
                      (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                       errmsg("cannot truncate temporary tables of other sessions")));

+         /* Save it into the list of rels to truncate */
+         rels = lappend(rels, rel);
+     }
+
      /*
!      * Check foreign key references.
       */
!     heap_truncate_check_FKs(rels);
!
!     foreach (cell, rels)
!     {
!         Relation    rel = lfirst(cell);
!         Oid            heap_relid;
!         Oid            toast_relid;

          /*
!          * Create a new empty storage file for the relation, and assign it as
!          * the relfilenode value.    The old storage file is scheduled for
!          * deletion at commit.
           */
          setNewRelfilenode(rel);

***************
*** 606,611 ****
--- 627,633 ----
           * Reconstruct the indexes to match, and we're done.
           */
          reindex_relation(heap_relid, true);
+     }
  }

  /*----------
***************
*** 5959,5964 ****
--- 5981,5987 ----
  PreCommit_on_commit_actions(void)
  {
      ListCell   *l;
+     List       *oids_to_truncate = NIL;

      foreach(l, on_commits)
      {
***************
*** 5975,5982 ****
                  /* Do nothing (there shouldn't be such entries, actually) */
                  break;
              case ONCOMMIT_DELETE_ROWS:
!                 heap_truncate(oc->relid);
!                 CommandCounterIncrement();        /* XXX needed? */
                  break;
              case ONCOMMIT_DROP:
                  {
--- 5998,6004 ----
                  /* Do nothing (there shouldn't be such entries, actually) */
                  break;
              case ONCOMMIT_DELETE_ROWS:
!                 oids_to_truncate = lappend_oid(oids_to_truncate, oc->relid);
                  break;
              case ONCOMMIT_DROP:
                  {
***************
*** 5997,6002 ****
--- 6019,6027 ----
                  }
          }
      }
+     if (list_length(oids_to_truncate) > 0)
+         heap_truncate(oids_to_truncate);
+     CommandCounterIncrement();                /* XXX needed? */
  }

  /*
Index: src/backend/nodes/copyfuncs.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v
retrieving revision 1.293
diff -c -c -b -r1.293 copyfuncs.c
*** src/backend/nodes/copyfuncs.c    5 Nov 2004 19:15:59 -0000    1.293
--- src/backend/nodes/copyfuncs.c    6 Nov 2004 05:57:12 -0000
***************
*** 1800,1806 ****
  {
      TruncateStmt *newnode = makeNode(TruncateStmt);

!     COPY_NODE_FIELD(relation);

      return newnode;
  }
--- 1800,1806 ----
  {
      TruncateStmt *newnode = makeNode(TruncateStmt);

!     COPY_NODE_FIELD(relations);

      return newnode;
  }
Index: src/backend/nodes/equalfuncs.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/nodes/equalfuncs.c,v
retrieving revision 1.232
diff -c -c -b -r1.232 equalfuncs.c
*** src/backend/nodes/equalfuncs.c    5 Nov 2004 19:15:59 -0000    1.232
--- src/backend/nodes/equalfuncs.c    6 Nov 2004 05:57:15 -0000
***************
*** 873,879 ****
  static bool
  _equalTruncateStmt(TruncateStmt *a, TruncateStmt *b)
  {
!     COMPARE_NODE_FIELD(relation);

      return true;
  }
--- 873,879 ----
  static bool
  _equalTruncateStmt(TruncateStmt *a, TruncateStmt *b)
  {
!     COMPARE_NODE_FIELD(relations);

      return true;
  }
Index: src/backend/parser/gram.y
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.479
diff -c -c -b -r2.479 gram.y
*** src/backend/parser/gram.y    5 Nov 2004 19:16:02 -0000    2.479
--- src/backend/parser/gram.y    6 Nov 2004 05:57:29 -0000
***************
*** 2684,2698 ****
  /*****************************************************************************
   *
   *        QUERY:
!  *                truncate table relname
   *
   *****************************************************************************/

  TruncateStmt:
!             TRUNCATE opt_table qualified_name
                  {
                      TruncateStmt *n = makeNode(TruncateStmt);
!                     n->relation = $3;
                      $$ = (Node *)n;
                  }
          ;
--- 2684,2698 ----
  /*****************************************************************************
   *
   *        QUERY:
!  *                truncate table relname1, relname2, ...
   *
   *****************************************************************************/

  TruncateStmt:
!             TRUNCATE opt_table qualified_name_list
                  {
                      TruncateStmt *n = makeNode(TruncateStmt);
!                     n->relations = $3;
                      $$ = (Node *)n;
                  }
          ;
Index: src/backend/tcop/utility.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/tcop/utility.c,v
retrieving revision 1.230
diff -c -c -b -r1.230 utility.c
*** src/backend/tcop/utility.c    13 Sep 2004 20:07:06 -0000    1.230
--- src/backend/tcop/utility.c    6 Nov 2004 05:57:31 -0000
***************
*** 575,581 ****
              {
                  TruncateStmt *stmt = (TruncateStmt *) parsetree;

!                 TruncateRelation(stmt->relation);
              }
              break;

--- 575,581 ----
              {
                  TruncateStmt *stmt = (TruncateStmt *) parsetree;

!                 ExecuteTruncate(stmt->relations);
              }
              break;

Index: src/include/catalog/heap.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/catalog/heap.h,v
retrieving revision 1.71
diff -c -c -b -r1.71 heap.h
*** src/include/catalog/heap.h    31 Aug 2004 17:10:36 -0000    1.71
--- src/include/catalog/heap.h    6 Nov 2004 05:57:34 -0000
***************
*** 56,64 ****

  extern void heap_drop_with_catalog(Oid relid);

! extern void heap_truncate(Oid rid);

! extern void heap_truncate_check_FKs(Relation rel);

  extern List *AddRelationRawConstraints(Relation rel,
                            List *rawColDefaults,
--- 56,64 ----

  extern void heap_drop_with_catalog(Oid relid);

! extern void heap_truncate(List *relids);

! extern void heap_truncate_check_FKs(List *relations);

  extern List *AddRelationRawConstraints(Relation rel,
                            List *rawColDefaults,
Index: src/include/commands/tablecmds.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/commands/tablecmds.h,v
retrieving revision 1.20
diff -c -c -b -r1.20 tablecmds.h
*** src/include/commands/tablecmds.h    16 Sep 2004 16:58:39 -0000    1.20
--- src/include/commands/tablecmds.h    6 Nov 2004 05:57:34 -0000
***************
*** 27,33 ****

  extern void AlterTableCreateToastTable(Oid relOid, bool silent);

! extern void TruncateRelation(const RangeVar *relation);

  extern void renameatt(Oid myrelid,
            const char *oldattname,
--- 27,33 ----

  extern void AlterTableCreateToastTable(Oid relOid, bool silent);

! extern void ExecuteTruncate(List *relations);

  extern void renameatt(Oid myrelid,
            const char *oldattname,
Index: src/include/nodes/parsenodes.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/nodes/parsenodes.h,v
retrieving revision 1.270
diff -c -c -b -r1.270 parsenodes.h
*** src/include/nodes/parsenodes.h    5 Nov 2004 19:16:38 -0000    1.270
--- src/include/nodes/parsenodes.h    6 Nov 2004 05:57:37 -0000
***************
*** 1283,1289 ****
  typedef struct TruncateStmt
  {
      NodeTag        type;
!     RangeVar   *relation;        /* relation to be truncated */
  } TruncateStmt;

  /* ----------------------
--- 1283,1289 ----
  typedef struct TruncateStmt
  {
      NodeTag        type;
!     List       *relations;        /* relations (RangeVars) to be truncated */
  } TruncateStmt;

  /* ----------------------
Index: src/test/regress/expected/temp.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/temp.out,v
retrieving revision 1.9
diff -c -c -b -r1.9 temp.out
*** src/test/regress/expected/temp.out    25 Sep 2003 06:58:06 -0000    1.9
--- src/test/regress/expected/temp.out    6 Nov 2004 05:57:40 -0000
***************
*** 82,84 ****
--- 82,103 ----
  -- ON COMMIT is only allowed for TEMP
  CREATE TABLE temptest(col int) ON COMMIT DELETE ROWS;
  ERROR:  ON COMMIT can only be used on temporary tables
+ -- Test foreign keys
+ BEGIN;
+ CREATE TEMP TABLE temptest1(col int PRIMARY KEY) ON COMMIT DELETE ROWS;
+ NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index "temptest1_pkey" for table "temptest1"
+ CREATE TEMP TABLE temptest2(col int REFERENCES temptest1)
+   ON COMMIT DELETE ROWS;
+ INSERT INTO temptest1 VALUES (1);
+ INSERT INTO temptest2 VALUES (1);
+ COMMIT;
+ SELECT * FROM temptest1;
+  col
+ -----
+ (0 rows)
+
+ SELECT * FROM temptest2;
+  col
+ -----
+ (0 rows)
+
Index: src/test/regress/expected/truncate.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/truncate.out,v
retrieving revision 1.9
diff -c -c -b -r1.9 truncate.out
*** src/test/regress/expected/truncate.out    10 Jun 2004 17:56:01 -0000    1.9
--- src/test/regress/expected/truncate.out    6 Nov 2004 05:57:41 -0000
***************
*** 30,52 ****
  ------
  (0 rows)

! -- Test foreign constraint check
! CREATE TABLE truncate_b(col1 integer references truncate_a);
  INSERT INTO truncate_a VALUES (1);
! SELECT * FROM truncate_a;
!  col1
! ------
!     1
! (1 row)
!
! TRUNCATE truncate_a;
  ERROR:  cannot truncate a table referenced in a foreign key constraint
! DETAIL:  Table "truncate_b" references "truncate_a" via foreign key constraint "truncate_b_col1_fkey".
! SELECT * FROM truncate_a;
   col1
  ------
!     1
! (1 row)

! DROP TABLE truncate_b;
! DROP TABLE truncate_a;
--- 30,113 ----
  ------
  (0 rows)

! -- Test foreign-key checks
! CREATE TABLE trunc_b (a int REFERENCES truncate_a);
! CREATE TABLE trunc_c (a serial PRIMARY KEY);
! NOTICE:  CREATE TABLE will create implicit sequence "trunc_c_a_seq" for serial column "trunc_c.a"
! NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index "trunc_c_pkey" for table "trunc_c"
! CREATE TABLE trunc_d (a int REFERENCES trunc_c);
! CREATE TABLE trunc_e (a int REFERENCES truncate_a, b int REFERENCES trunc_c);
! TRUNCATE TABLE truncate_a;        -- fail
! ERROR:  cannot truncate a table referenced in a foreign key constraint
! DETAIL:  Table "trunc_b" references "truncate_a" via foreign key constraint "trunc_b_a_fkey".
! HINT:  Truncate table "trunc_b" at the same time
! TRUNCATE TABLE truncate_a,trunc_b;        -- fail
! ERROR:  cannot truncate a table referenced in a foreign key constraint
! DETAIL:  Table "trunc_e" references "truncate_a" via foreign key constraint "trunc_e_a_fkey".
! HINT:  Truncate table "trunc_e" at the same time
! TRUNCATE TABLE truncate_a,trunc_b,trunc_e;    -- ok
! TRUNCATE TABLE truncate_a,trunc_e;        -- fail
! ERROR:  cannot truncate a table referenced in a foreign key constraint
! DETAIL:  Table "trunc_b" references "truncate_a" via foreign key constraint "trunc_b_a_fkey".
! HINT:  Truncate table "trunc_b" at the same time
! TRUNCATE TABLE trunc_c;        -- fail
! ERROR:  cannot truncate a table referenced in a foreign key constraint
! DETAIL:  Table "trunc_d" references "trunc_c" via foreign key constraint "trunc_d_a_fkey".
! HINT:  Truncate table "trunc_d" at the same time
! TRUNCATE TABLE trunc_c,trunc_d;        -- fail
! ERROR:  cannot truncate a table referenced in a foreign key constraint
! DETAIL:  Table "trunc_e" references "trunc_c" via foreign key constraint "trunc_e_b_fkey".
! HINT:  Truncate table "trunc_e" at the same time
! TRUNCATE TABLE trunc_c,trunc_d,trunc_e;    -- ok
! TRUNCATE TABLE trunc_c,trunc_d,trunc_e,truncate_a;    -- fail
! ERROR:  cannot truncate a table referenced in a foreign key constraint
! DETAIL:  Table "trunc_b" references "truncate_a" via foreign key constraint "trunc_b_a_fkey".
! HINT:  Truncate table "trunc_b" at the same time
! TRUNCATE TABLE trunc_c,trunc_d,trunc_e,truncate_a,trunc_b;    -- ok
! -- circular references
! ALTER TABLE truncate_a ADD FOREIGN KEY (col1) REFERENCES trunc_c;
! -- Add some data to verify that truncating actually works ...
! INSERT INTO trunc_c VALUES (1);
  INSERT INTO truncate_a VALUES (1);
! INSERT INTO trunc_b VALUES (1);
! INSERT INTO trunc_d VALUES (1);
! INSERT INTO trunc_e VALUES (1,1);
! TRUNCATE TABLE trunc_c;
! ERROR:  cannot truncate a table referenced in a foreign key constraint
! DETAIL:  Table "trunc_d" references "trunc_c" via foreign key constraint "trunc_d_a_fkey".
! HINT:  Truncate table "trunc_d" at the same time
! TRUNCATE TABLE trunc_c,trunc_d;
  ERROR:  cannot truncate a table referenced in a foreign key constraint
! DETAIL:  Table "trunc_e" references "trunc_c" via foreign key constraint "trunc_e_b_fkey".
! HINT:  Truncate table "trunc_e" at the same time
! TRUNCATE TABLE trunc_c,trunc_d,trunc_e;
! ERROR:  cannot truncate a table referenced in a foreign key constraint
! DETAIL:  Table "truncate_a" references "trunc_c" via foreign key constraint "truncate_a_col1_fkey".
! HINT:  Truncate table "truncate_a" at the same time
! TRUNCATE TABLE trunc_c,trunc_d,trunc_e,truncate_a;
! ERROR:  cannot truncate a table referenced in a foreign key constraint
! DETAIL:  Table "trunc_b" references "truncate_a" via foreign key constraint "trunc_b_a_fkey".
! HINT:  Truncate table "trunc_b" at the same time
! TRUNCATE TABLE trunc_c,trunc_d,trunc_e,truncate_a,trunc_b;
! -- Verify that truncating did actually work
! SELECT * FROM truncate_a
!    UNION ALL
!  SELECT * FROM trunc_c
!    UNION ALL
!  SELECT * FROM trunc_b
!    UNION ALL
!  SELECT * FROM trunc_d;
   col1
  ------
! (0 rows)
!
! SELECT * FROM trunc_e;
!  a | b
! ---+---
! (0 rows)

! DROP TABLE truncate_a,trunc_c,trunc_b,trunc_d,trunc_e CASCADE;
! NOTICE:  drop cascades to constraint trunc_e_a_fkey on table trunc_e
! NOTICE:  drop cascades to constraint trunc_b_a_fkey on table trunc_b
! NOTICE:  drop cascades to constraint trunc_e_b_fkey on table trunc_e
! NOTICE:  drop cascades to constraint trunc_d_a_fkey on table trunc_d
Index: src/test/regress/sql/temp.sql
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/sql/temp.sql,v
retrieving revision 1.5
diff -c -c -b -r1.5 temp.sql
*** src/test/regress/sql/temp.sql    14 May 2003 03:26:03 -0000    1.5
--- src/test/regress/sql/temp.sql    6 Nov 2004 05:57:41 -0000
***************
*** 83,85 ****
--- 83,97 ----
  -- ON COMMIT is only allowed for TEMP

  CREATE TABLE temptest(col int) ON COMMIT DELETE ROWS;
+
+ -- Test foreign keys
+ BEGIN;
+
+ CREATE TEMP TABLE temptest1(col int PRIMARY KEY) ON COMMIT DELETE ROWS;
+ CREATE TEMP TABLE temptest2(col int REFERENCES temptest1)
+   ON COMMIT DELETE ROWS;
+ INSERT INTO temptest1 VALUES (1);
+ INSERT INTO temptest2 VALUES (1);
+ COMMIT;
+ SELECT * FROM temptest1;
+ SELECT * FROM temptest2;
Index: src/test/regress/sql/truncate.sql
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/sql/truncate.sql,v
retrieving revision 1.2
diff -c -c -b -r1.2 truncate.sql
*** src/test/regress/sql/truncate.sql    23 Nov 2002 04:05:52 -0000    1.2
--- src/test/regress/sql/truncate.sql    6 Nov 2004 05:57:42 -0000
***************
*** 14,25 ****
  COMMIT;
  SELECT * FROM truncate_a;

! -- Test foreign constraint check
! CREATE TABLE truncate_b(col1 integer references truncate_a);
  INSERT INTO truncate_a VALUES (1);
! SELECT * FROM truncate_a;
! TRUNCATE truncate_a;
! SELECT * FROM truncate_a;

! DROP TABLE truncate_b;
! DROP TABLE truncate_a;
--- 14,58 ----
  COMMIT;
  SELECT * FROM truncate_a;

! -- Test foreign-key checks
! CREATE TABLE trunc_b (a int REFERENCES truncate_a);
! CREATE TABLE trunc_c (a serial PRIMARY KEY);
! CREATE TABLE trunc_d (a int REFERENCES trunc_c);
! CREATE TABLE trunc_e (a int REFERENCES truncate_a, b int REFERENCES trunc_c);
!
! TRUNCATE TABLE truncate_a;        -- fail
! TRUNCATE TABLE truncate_a,trunc_b;        -- fail
! TRUNCATE TABLE truncate_a,trunc_b,trunc_e;    -- ok
! TRUNCATE TABLE truncate_a,trunc_e;        -- fail
! TRUNCATE TABLE trunc_c;        -- fail
! TRUNCATE TABLE trunc_c,trunc_d;        -- fail
! TRUNCATE TABLE trunc_c,trunc_d,trunc_e;    -- ok
! TRUNCATE TABLE trunc_c,trunc_d,trunc_e,truncate_a;    -- fail
! TRUNCATE TABLE trunc_c,trunc_d,trunc_e,truncate_a,trunc_b;    -- ok
!
! -- circular references
! ALTER TABLE truncate_a ADD FOREIGN KEY (col1) REFERENCES trunc_c;
!
! -- Add some data to verify that truncating actually works ...
! INSERT INTO trunc_c VALUES (1);
  INSERT INTO truncate_a VALUES (1);
! INSERT INTO trunc_b VALUES (1);
! INSERT INTO trunc_d VALUES (1);
! INSERT INTO trunc_e VALUES (1,1);
! TRUNCATE TABLE trunc_c;
! TRUNCATE TABLE trunc_c,trunc_d;
! TRUNCATE TABLE trunc_c,trunc_d,trunc_e;
! TRUNCATE TABLE trunc_c,trunc_d,trunc_e,truncate_a;
! TRUNCATE TABLE trunc_c,trunc_d,trunc_e,truncate_a,trunc_b;
!
! -- Verify that truncating did actually work
! SELECT * FROM truncate_a
!    UNION ALL
!  SELECT * FROM trunc_c
!    UNION ALL
!  SELECT * FROM trunc_b
!    UNION ALL
!  SELECT * FROM trunc_d;
! SELECT * FROM trunc_e;

! DROP TABLE truncate_a,trunc_c,trunc_b,trunc_d,trunc_e CASCADE;

Re: ON COMMIT and foreign keys

От
Alvaro Herrera
Дата:
> Tom Lane wrote:
> > Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
> > > Incidentally ("collateral damage"), the patch modifies the TRUNCATE
> > > command so that it can work on multiple tables.  In particular, if
> > > foreign key references are all internal to the group that's being
> > > truncated, the command is allowed.
> >
> > I think this is a feature addition that you've cleverly managed to
> > present as a bug fix ;-).  While I like the feature, I've got very
> > mixed emotions about applying it this late in beta.

Ok, you caught me :-)  Actually, I developed the patch thinking only on
the multi-relation TRUNCATE, with the goal of having it for 8.1.  Only
after I prepared a first patch I noticed the temp-table-FKs problem.

So I don't really care if it has to wait for 8.1 because that was the
original intention anyway.

> > OTOH, multiple TRUNCATE would be a useful feature --- it came up
> > just the other day.

Right, that's why I did it in the first place.


On Sat, Nov 06, 2004 at 12:56:24AM -0500, Bruce Momjian wrote:

> Alvaro, would you post the patch skipping whitespace changes.  As posted
> the patch does look very large, as you suggested.

Ok, turns out it wasn't that much smaller anyway :-)  But really it's
only stashing a couple of foreach loops in two places, which is a fairly
trivial change; and the other important part is the modification of
heap_truncate_check_FKs and it's fairly localized anyway.

--
Alvaro Herrera (<alvherre[a]dcc.uchile.cl>)
"Aprender sin pensar es inútil; pensar sin aprender, peligroso" (Confucio)


Re: ON COMMIT and foreign keys

От
Tom Lane
Дата:
Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
>> Tom Lane wrote:
> I think this is a feature addition that you've cleverly managed to
> present as a bug fix ;-).  While I like the feature, I've got very
> mixed emotions about applying it this late in beta.

> Ok, you caught me :-)  Actually, I developed the patch thinking only on
> the multi-relation TRUNCATE, with the goal of having it for 8.1.  Only
> after I prepared a first patch I noticed the temp-table-FKs problem.

Well, like I said, I have mixed emotions.  Anyone else have an opinion
whether to apply now or hold for 8.1?

            regards, tom lane

Re: ON COMMIT and foreign keys

От
Bruce Momjian
Дата:
Tom Lane wrote:
> Alvaro Herrera <alvherre@dcc.uchile.cl> writes:
> >> Tom Lane wrote:
> > I think this is a feature addition that you've cleverly managed to
> > present as a bug fix ;-).  While I like the feature, I've got very
> > mixed emotions about applying it this late in beta.
>
> > Ok, you caught me :-)  Actually, I developed the patch thinking only on
> > the multi-relation TRUNCATE, with the goal of having it for 8.1.  Only
> > after I prepared a first patch I noticed the temp-table-FKs problem.
>
> Well, like I said, I have mixed emotions.  Anyone else have an opinion
> whether to apply now or hold for 8.1?

My initial reaction is to hold it all for 8.1.  The patch is large and
the bug is rare.

--
  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 and foreign keys

От
Bruce Momjian
Дата:
FYI, this was already applied:

    2005-01-26 22:17  tgl

            * doc/src/sgml/ref/truncate.sgml, src/backend/catalog/heap.c,
            src/backend/commands/tablecmds.c, src/backend/nodes/copyfuncs.c,
            src/backend/nodes/equalfuncs.c, src/backend/parser/gram.y,
            src/backend/tcop/utility.c, src/include/catalog/heap.h,
            src/include/commands/tablecmds.h, src/include/nodes/parsenodes.h,
            src/test/regress/expected/temp.out,
            src/test/regress/expected/truncate.out,
            src/test/regress/sql/temp.sql, src/test/regress/sql/truncate.sql:
            Generalize TRUNCATE to support truncating multiple tables in one
            command.  This is useful because we can allow truncation of tables
            referenced by foreign keys, so long as the referencing table is
            truncated in the same command.

            Alvaro Herrera


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

Alvaro Herrera wrote:
> Hackers,
>
> There's a bug with temporary tables signalled ON COMMIT DELETE ROWS,
> when they contain foreign key references.  An example:
>
> alvherre=# begin;
> BEGIN
> alvherre=# CREATE TEMP TABLE foo (a int PRIMARY KEY) ON COMMIT DELETE ROWS;
> NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index "foo_pkey" for table "foo"
> CREATE TABLE
> alvherre=# CREATE TEMP TABLE bar (a int REFERENCES foo) ON COMMIT DELETE ROWS;
> CREATE TABLE
> alvherre=# COMMIT;
> ERROR:  cannot truncate a table referenced in a foreign key constraint
> DETAIL:  Table "bar" references "foo" via foreign key constraint "bar_a_fkey".
>
> Say again?  Certainly this shouldn't happen, because both tables are
> supposed to lose rows on transaction commit.  But this isn't working.
>
> The attached patch fixes this bug.  (In all likelyhood, not a lot of
> people uses referential integrity on temp tables, and that's why this
> hasn't been reported.  But it's a bug anyway.)
>
>
> Incidentally ("collateral damage"), the patch modifies the TRUNCATE
> command so that it can work on multiple tables.  In particular, if
> foreign key references are all internal to the group that's being
> truncated, the command is allowed.
>
> There's one thing that bothers me on this patch: the fact that
> pg_constraint has to be scanned multiple times, and they are all
> seqscans.  Not sure what to do about that.  Maybe there's a way to do
> better?
>
> Also, observe that when the TRUNCATE operation is aborted because of a
> foreign key, a HINT is emitted as well telling the user to truncate the
> referencing table too.  This is IMHO a good hint, but it may be
> misleading when the truncation has taken the ON COMMIT DELETE ROWS path.
> Not sure if it's worth fixing (maybe the hint should suggest to add ON
> COMMIT DELETE ROWS to the referencing table as well?).
>
> Please have a look.  The patch is not as intrusive as it looks; there's
> a lot of whitespace change.
>
> --
> Alvaro Herrera (<alvherre[a]dcc.uchile.cl>)
> "Ellos andaban todos desnudos como su madre los pari?, y tambi?n las mujeres,
> aunque no vi m?s que una, harto moza, y todos los que yo vi eran todos
> mancebos, que ninguno vi de edad de m?s de XXX a?os" (Crist?bal Col?n)

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
>     (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)

--
  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