Re: [PATCHES] TODO-Item: TRUNCATE ... CASCADE

2006-03-02 Thread Tom Lane
Joachim Wieland [EMAIL PROTECTED] writes:
 Ok, the attached patch now does it correctly as suggested by Alvaro.

Applied with minor cleanup --- I thought the code for scanning for
dependent relations was unreasonably complicated and created
unpredictable locking order, so I simplified it.

regards, tom lane

---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [PATCHES] TODO-Item: TRUNCATE ... CASCADE

2006-02-05 Thread Joachim Wieland
On Fri, Feb 03, 2006 at 10:27:30AM -0500, Tom Lane wrote:
 Basically: it's the user's fault if he says TRUNCATE t2 in a situation
 where the referent of t2 might be changing concurrently.  But once
 you've identified t2, it's your fault if you don't track the
 dependencies of t2 correctly, even if someone else is busy renaming them.

Ok, the attached patch now does it correctly as suggested by Alvaro.

For code simplicity I changed the locking order of the tables in the
RESTRICT-case as well.
Before they got locked and then checked one by one. To simplify integration
of the CASCADE-case however, I changed it to lock all - check all.

So it is now:

CASCADE:
lock direct - add and lock cascaded tables - check all - truncate all

RESTRICT:
lock direct (= all) - check all - truncate all.


Joachim
diff -ur cvs/pgsql/doc/src/sgml/ref/truncate.sgml 
cvs.build/pgsql/doc/src/sgml/ref/truncate.sgml
--- cvs/pgsql/doc/src/sgml/ref/truncate.sgml2005-02-22 20:06:18.0 
+0100
+++ cvs.build/pgsql/doc/src/sgml/ref/truncate.sgml  2006-02-05 
01:52:56.0 +0100
@@ -20,7 +20,7 @@
 
  refsynopsisdiv
 synopsis
-TRUNCATE [ TABLE ] replaceable class=PARAMETERname/replaceable [, ...]
+TRUNCATE [ TABLE ] replaceable class=PARAMETERname/replaceable [, ...] [ 
CASCADE | RESTRICT ]
 /synopsis
  /refsynopsisdiv
 
@@ -59,9 +59,10 @@
 
   para
commandTRUNCATE/ cannot be used on a table that has foreign-key
-   references 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.
+   references from other tables, unless either all such tables are also
+   truncated in the same command or the literalCASCADE/ keyword is
+   specified. Checking validity in such cases would require table scans,
+   and the whole point is not to do one.
   /para
 
   para
@@ -80,8 +81,17 @@
 TRUNCATE TABLE bigtable, fattable;
 /programlisting
   /para
+
+  para
+   Truncate the table literalothertable/literal and cascade to tables that
+   are referencing literalothertable/literal via foreign-key constraints:
+
+programlisting
+TRUNCATE othertable CASCADE;
+/programlisting
+  /para
  /refsect1
- 
+
  refsect1
   titleCompatibility/title
 
diff -ur cvs/pgsql/src/backend/catalog/heap.c 
cvs.build/pgsql/src/backend/catalog/heap.c
--- cvs/pgsql/src/backend/catalog/heap.c2005-11-22 19:17:08.0 
+0100
+++ cvs.build/pgsql/src/backend/catalog/heap.c  2006-02-05 01:52:56.0 
+0100
@@ -2066,7 +2066,7 @@
   
get_rel_name(con-conrelid),
   
get_rel_name(con-confrelid),
   
NameStr(con-conname)),
-errhint(Truncate table \%s\ 
at the same time.,
+errhint(Truncate table \%s\ 
at the same time or use TRUNCATE ... CASCADE.,
 
get_rel_name(con-conrelid;
}
}
diff -ur cvs/pgsql/src/backend/commands/tablecmds.c 
cvs.build/pgsql/src/backend/commands/tablecmds.c
--- cvs/pgsql/src/backend/commands/tablecmds.c  2006-01-30 22:52:35.0 
+0100
+++ cvs.build/pgsql/src/backend/commands/tablecmds.c2006-02-05 
16:39:48.0 +0100
@@ -523,30 +523,163 @@
performDeletion(object, behavior);
 }
 
+/* This function is essentially copied from heap_truncate_check_FKs.
+ *
+ * We look here for relations referencing one of the relations in the
+ * oids list. We also pass the list of relations we have already found as
+ * found_earlier.
+ *
+ * What gets found in one run will appear in oids in the next call until
+ * no new relations are found.
+ */
+static List *
+BuildReferencingRelationList(List *oids, List *found_earlier)
+{
+   List   *referencingRelids = NIL;
+   List   *referencingRels = NIL;
+   RelationfkeyRel;
+   SysScanDesc fkeyScan;
+   HeapTuple   tuple;
+
+   /* oids is a subset of found_earlier */
+   Assert(list_length(list_difference_oid(oids, found_earlier)) == 0);
+
+   /*
+* Right now, it is a seqscan because there is no available index on
+* confrelid (cf. heap_truncate_check_FKs()).
+*/
+   fkeyRel = heap_open(ConstraintRelationId, AccessShareLock);
+   fkeyScan = systable_beginscan(fkeyRel, InvalidOid, false,
+ SnapshotNow, 
0, NULL);
+
+   while (HeapTupleIsValid(tuple = systable_getnext(fkeyScan)))
+   {
+   Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(tuple);
+
+   /* Not a foreign key */
+   if (con-contype != CONSTRAINT_FOREIGN)
+   continue;
+
+   /*
+* 

Re: [PATCHES] TODO-Item: TRUNCATE ... CASCADE

2006-02-03 Thread Joachim Wieland
On Thu, Feb 02, 2006 at 12:34:28PM -0300, Alvaro Herrera wrote:
  The patch also adds a function makeRangeVarFromRelId() to namespace.c
  that I thought would be useful. I hope I didn't overlook something
  similar that exists already.

 That's the wrong way to go about it -- better refactor the code so that
 a function gets a list of Oids instead of RangeVars, and truncates them.
 ExecuteTruncate should build the list and pass it down.

Ok. But are RangeVars deprecated in general? Is there more information
around about when to use what?


 Also I think all the involved relations should be opened and locked
 before any of them is touched (so maybe instead of passing Oids you
 should be passing Relations).

There is already the possibility to do TRUNCATE t1, t2, t3. The patch just
adds all referencing relations as if the user had specified all of them and
then starts truncating the same way it is doing it right now. If all
relations have to be opened and locked before truncating multiple relations,
then this problem also exists in the current code.

Or are you talking about the fact that a RangeVar contains the actual name
of the relation that the user wants to truncate whereas an Oid could
possibly be changed by another backend while our backend is constructing the
list and therefore when using Oids I have to get a lock on the table earlier?


Joachim

-- 
Joachim Wieland  [EMAIL PROTECTED]
C/ Usandizaga 12 1°B   ICQ: 37225940
20002 Donostia / San Sebastian (Spain) GPG key available

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match


Re: [PATCHES] TODO-Item: TRUNCATE ... CASCADE

2006-02-03 Thread Tom Lane
Joachim Wieland [EMAIL PROTECTED] writes:
 On Thu, Feb 02, 2006 at 12:34:28PM -0300, Alvaro Herrera wrote:
 That's the wrong way to go about it -- better refactor the code so that
 a function gets a list of Oids instead of RangeVars, and truncates them.
 ExecuteTruncate should build the list and pass it down.

 Ok. But are RangeVars deprecated in general?

No, but they're a parse-time representation.  Once you've resolved a
relation down to its OID, you should stick with the OID.  In this
particular case, you would for instance be exposing yourself to needless
race conditions against ALTER TABLE RENAME if you generate a RangeVar
from the OID and then pass that to somewhere to be looked up.

 Also I think all the involved relations should be opened and locked
 before any of them is touched (so maybe instead of passing Oids you
 should be passing Relations).

 There is already the possibility to do TRUNCATE t1, t2, t3. The patch just
 adds all referencing relations as if the user had specified all of them and
 then starts truncating the same way it is doing it right now. If all
 relations have to be opened and locked before truncating multiple relations,
 then this problem also exists in the current code.

I think the point here is that you need to acquire lock on a relation
before you start inspecting its dependencies.  The patch might already
do that OK, I haven't looked at it yet.

Basically: it's the user's fault if he says TRUNCATE t2 in a situation
where the referent of t2 might be changing concurrently.  But once
you've identified t2, it's your fault if you don't track the
dependencies of t2 correctly, even if someone else is busy renaming them.

regards, tom lane

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match


Re: [PATCHES] TODO-Item: TRUNCATE ... CASCADE

2006-02-02 Thread Alvaro Herrera
Joachim Wieland wrote:
 The proposed patch implements TRUNCATE ... CASCADE:
 
 * %Allow TRUNCATE ... CASCADE/RESTRICT
   This is like DELETE CASCADE, but truncates.
 
 The patch also adds a function makeRangeVarFromRelId() to namespace.c that I
 thought would be useful. I hope I didn't overlook something similar that
 exists already.

That's the wrong way to go about it -- better refactor the code so that
a function gets a list of Oids instead of RangeVars, and truncates them.
ExecuteTruncate should build the list and pass it down.

Also I think all the involved relations should be opened and locked
before any of them is touched (so maybe instead of passing Oids you
should be passing Relations).


 + static
 + List* BuildReferencingRelationList(List* oids, List* found_earlier)

Minor stylistic gripe: this should be

static List *
BuildReferencingRelationList(List *oids, List *found_earlier)

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster