Thread: TODO-Item: TRUNCATE ... CASCADE

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.


Joachim

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 Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

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                                              joe@mcknight.de
C/ Usandizaga 12 1°B                                           ICQ: 37225940
20002 Donostia / San Sebastian (Spain)                     GPG key available

Joachim Wieland <joe@mcknight.de> 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

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

Joachim Wieland <joe@mcknight.de> 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