Thread: about truncate
Hi, just out of curiosity, why TRUNCATE doesn't support ONLY? audit=# TRUNCATE only postgres_log; ERROR: syntax error at or near "only" LINE 1: TRUNCATE only postgres_log; -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157
On Sun, Dec 21, 2008 at 10:09:54PM -0500, Jaime Casanova wrote: > Hi, > > just out of curiosity, why TRUNCATE doesn't support ONLY? > > audit=# TRUNCATE only postgres_log; > ERROR: syntax error at or near "only" > LINE 1: TRUNCATE only postgres_log; Given that the main (and only sane, IMHO) use for table inheritance is in table partitioning, can we see about deprecating ONLY (in the table inheritance sense) for the next couple of development cycles and then removing it? Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
David Fetter <david@fetter.org> writes:
> Given that the main (and only sane, IMHO) use for table inheritance is
> in table partitioning, can we see about deprecating ONLY (in the table
> inheritance sense) for the next couple of development cycles and then
> removing it?
No.
1. It's required by SQL standard.
2. Just because you don't have a use for it doesn't mean no one does.
regards, tom lane
On Sun, Dec 21, 2008 at 11:06:09PM -0500, Tom Lane wrote: > David Fetter <david@fetter.org> writes: > > Given that the main (and only sane, IMHO) use for table inheritance is > > in table partitioning, can we see about deprecating ONLY (in the table > > inheritance sense) for the next couple of development cycles and then > > removing it? > > No. > > 1. It's required by SQL standard. Well blow me down! I had no idea the SQL standard had this wart in it. > 2. Just because you don't have a use for it doesn't mean no one does. Clearly the SQL standards committee does, and their usage controls ;) Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
On Monday 22 December 2008 05:09:54 Jaime Casanova wrote: > just out of curiosity, why TRUNCATE doesn't support ONLY? It was probably just an omission. Note that TRUNCATE currently does not act on inheriting tables. In other words, the behavior is already like ONLY. FWIW, the SQL standard says that TRUNCATE should support ONLY, just like DELETE. Something should probably be fixed or at least documented about this.
Peter Eisentraut wrote: > On Monday 22 December 2008 05:09:54 Jaime Casanova wrote: >> just out of curiosity, why TRUNCATE doesn't support ONLY? > > It was probably just an omission. > > Note that TRUNCATE currently does not act on inheriting tables. In other > words, the behavior is already like ONLY. > > FWIW, the SQL standard says that TRUNCATE should support ONLY, just like > DELETE. > > Something should probably be fixed or at least documented about this. Before I or someone goes to write code for this, note that a proper fix would introduce a backward incompatibility when TRUNCATE is used on inheritance hierarchies. Currently, TRUNCATE only acts on the named table itself, not on any children. The behavior required by the SQL standard (and by consistency with pretty much all other commands in PostgreSQL) is that TRUNCATE operate on all child tables, unless ONLY is specified. Note that there is currently no way to get a TRUNCATE not-ONLY without writing manual loops, which is a significant gap of functionality. Considering that TRUNCATE is a pretty dangerous operation, how can we make adjustments to the behavior without upsetting lots of users?
Peter Eisentraut wrote: > Peter Eisentraut wrote: > > On Monday 22 December 2008 05:09:54 Jaime Casanova wrote: > >> just out of curiosity, why TRUNCATE doesn't support ONLY? > > > > It was probably just an omission. > > > > Note that TRUNCATE currently does not act on inheriting tables. In other > > words, the behavior is already like ONLY. > > > > FWIW, the SQL standard says that TRUNCATE should support ONLY, just like > > DELETE. > > > > Something should probably be fixed or at least documented about this. > > Before I or someone goes to write code for this, note that a proper fix > would introduce a backward incompatibility when TRUNCATE is used on > inheritance hierarchies. > > Currently, TRUNCATE only acts on the named table itself, not on any > children. > > The behavior required by the SQL standard (and by consistency with > pretty much all other commands in PostgreSQL) is that TRUNCATE operate > on all child tables, unless ONLY is specified. > > Note that there is currently no way to get a TRUNCATE not-ONLY without > writing manual loops, which is a significant gap of functionality. > > Considering that TRUNCATE is a pretty dangerous operation, how can we > make adjustments to the behavior without upsetting lots of users? Well, it is one of those, "Either we fix it or live with the inconsistency forever". Historically we have opted to fix it with a clear warning in the major release notes. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> Peter Eisentraut wrote:
>> Considering that TRUNCATE is a pretty dangerous operation, how can we
>> make adjustments to the behavior without upsetting lots of users?
> Well, it is one of those, "Either we fix it or live with the
> inconsistency forever". Historically we have opted to fix it with a
> clear warning in the major release notes.
The only alternatives I can see are
(1) go ahead and change it.
(2) invent a separate "truncate_inheritance" GUC that is just like
"sql_inheritance" except it applies only for TRUNCATE.
Ugly as (2) is, I think it just puts off the pain. Sooner or later
we'd want to flip the factory default from false to true, and the
release that does that is *still* going to burn anyone who's not
paying attention to the release notes.
My vote is to just go ahead and change it. I don't really see much
of a use-case for truncating only the parent of an inheritance
hierarchy anyway, so I doubt that many people would be affected.
I note though that we have a lot of other non-recursive maintenance
operations (CLUSTER, some variants of ALTER TABLE, etc) ... are we
going to try to make them all recursive?
regards, tom lane
Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Peter Eisentraut wrote: > >> Considering that TRUNCATE is a pretty dangerous operation, how can we > >> make adjustments to the behavior without upsetting lots of users? > > > Well, it is one of those, "Either we fix it or live with the > > inconsistency forever". Historically we have opted to fix it with a > > clear warning in the major release notes. > > The only alternatives I can see are > > (1) go ahead and change it. > > (2) invent a separate "truncate_inheritance" GUC that is just like > "sql_inheritance" except it applies only for TRUNCATE. > > Ugly as (2) is, I think it just puts off the pain. Sooner or later > we'd want to flip the factory default from false to true, and the > release that does that is *still* going to burn anyone who's not > paying attention to the release notes. The only way I think #2 works is if we say the GUC will disappear in the next major release, but it hardly seems worth adding the GUC because few people have even noticed the current behavior is a problem, meaning they are probably not using it for parent truncation often. > My vote is to just go ahead and change it. I don't really see much > of a use-case for truncating only the parent of an inheritance > hierarchy anyway, so I doubt that many people would be affected. > > I note though that we have a lot of other non-recursive maintenance > operations (CLUSTER, some variants of ALTER TABLE, etc) ... are we > going to try to make them all recursive? Uh, good question. ;-) I think fixing TRUNCATE makes sense because it is similar to DELETE (it operates on the data), but I see ALTER TABLE and CLUSTER as per-table operations that people would not expect to ever recurse, i.e. TRUNCATE is like DELETE without a WHERE clause, but CLUSTER or ALTER TABLE have no DML equivalents. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes: > Tom Lane wrote: >> My vote is to just go ahead and change it. I don't really see much >> of a use-case for truncating only the parent of an inheritance >> hierarchy anyway, so I doubt that many people would be affected. agreed. >> I note though that we have a lot of other non-recursive maintenance >> operations (CLUSTER, some variants of ALTER TABLE, etc) ... are we >> going to try to make them all recursive? > > Uh, good question. ;-) I think fixing TRUNCATE makes sense because it > is similar to DELETE (it operates on the data), but I see ALTER TABLE > and CLUSTER as per-table operations that people would not expect to ever > recurse, i.e. TRUNCATE is like DELETE without a WHERE clause, but > CLUSTER or ALTER TABLE have no DML equivalents. What does the standard say about ALTER TABLE and inheritance? It seems like it would be hard to make ALTER TABLE recursive since, while some operations might make sense, others will depend on the current state of the table and that might be very different for different children. Likewise CLUSTER ON/USING doesn't make much sense to be recursive since the index names will be different. It might be handy to have a recursive version of the command to recluster an already clustered table though. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Get trained by Bruce Momjian - ask me about EnterpriseDB'sPostgreSQL training!
On Tue, Dec 30, 2008 at 11:50:06AM -0500, Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > Peter Eisentraut wrote: > >> Considering that TRUNCATE is a pretty dangerous operation, how can we > >> make adjustments to the behavior without upsetting lots of users? > > > Well, it is one of those, "Either we fix it or live with the > > inconsistency forever". Historically we have opted to fix it with a > > clear warning in the major release notes. > > The only alternatives I can see are > > (1) go ahead and change it. > > (2) invent a separate "truncate_inheritance" GUC that is just like > "sql_inheritance" except it applies only for TRUNCATE. > > Ugly as (2) is, I think it just puts off the pain. Sooner or later > we'd want to flip the factory default from false to true, and the > release that does that is *still* going to burn anyone who's not > paying attention to the release notes. > > My vote is to just go ahead and change it. I don't really see much > of a use-case for truncating only the parent of an inheritance > hierarchy anyway, so I doubt that many people would be affected. Here's one such use-case. Let's say a table has gotten large and you've decided to partition it. You add child tables, add one or more triggers to the parent table to make sure it never gets a row, populate the child tables from the parent table, then you want to remove all the rows from the parent table. TRUNCATE ONLY handles this case just fine, so long as there's a clear message in the release notes. :) > I note though that we have a lot of other non-recursive maintenance > operations (CLUSTER, some variants of ALTER TABLE, etc) ... are we > going to try to make them all recursive? We probably should. Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
David Fetter wrote: > > My vote is to just go ahead and change it. I don't really see much > > of a use-case for truncating only the parent of an inheritance > > hierarchy anyway, so I doubt that many people would be affected. > > Here's one such use-case. Let's say a table has gotten large and > you've decided to partition it. You add child tables, add one or more > triggers to the parent table to make sure it never gets a row, > populate the child tables from the parent table, then you want to > remove all the rows from the parent table. > > TRUNCATE ONLY handles this case just fine, so long as there's a clear > message in the release notes. :) Agreed. The good thing is that I don't imagine what you have described above would be scripted so someone would be typing that and hopefully know the current behavior. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
On Tue, Dec 30, 2008 at 2:00 PM, David Fetter <david@fetter.org> wrote: > > Here's one such use-case. Let's say a table has gotten large and > you've decided to partition it. You add child tables, add one or more > triggers to the parent table to make sure it never gets a row, > populate the child tables from the parent table, then you want to > remove all the rows from the parent table. > you're spying me? exactly that happen to me... ;) my first attempt was to execute TRUNCATE ONLY... and gives me an error and the thread begun... -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157
On Tue, Dec 30, 2008 at 04:07:33PM -0500, Jaime Casanova wrote: > On Tue, Dec 30, 2008 at 2:00 PM, David Fetter <david@fetter.org> wrote: > > Here's one such use-case. Let's say a table has gotten large and > > you've decided to partition it. You add child tables, add one or > > more triggers to the parent table to make sure it never gets a > > row, populate the child tables from the parent table, then you > > want to remove all the rows from the parent table. > > > > you're spying me? D'oh! You've found out. Now that you know... ;) Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Tom Lane wrote: > I note though that we have a lot of other non-recursive maintenance > operations (CLUSTER, some variants of ALTER TABLE, etc) ... are we > going to try to make them all recursive? Here is the current line-up: command supports ONLY ALTER TABLE all other actions yes ALTER TABLE RENAME COLUMN yes ALTER TABLE RENAME no ALTER TABLE SET SCHEMA documented no, but accepted and ignored ANALYZE no CLUSTER no COMMENT no COPY no CREATE INDEX no DELETE yes DROP TABLE no GRANT no INSERT no LOCK no REINDEX no REVOKE no SELECT yes TRUNCATE no UPDATE yes VACUUM no Obviously, there is no practical sense in making them all behave the same, because ALTER TABLE RENAME not-ONLY for example would be nonsense. So there are always going to be two kinds of commands:"logical" ones that operate try to give the illusion that inheriting tables are included in the parent table, and "physical" ones that operate on a in single table only. About the current situation: Most people seemed to agree that TRUNCATE should support ONLY, to behave like DELETE. ALTER TABLE SET SCHEMA appears to be an omission. There could be some rare use cases for recursive versions of ANALYZE, CLUSTER, REINDEX, and VACUUM, but those would only be for convenience and would have no logical effect. A recursive version of CREATE INDEX could be quite useful, but that might belong into the whole inheritance vs. indexes bag of a mess. LOCK got me thinking. If you have a situation where an explicit lock is necessary because serializable transaction isolation does not give you the necessary guarantees, you would really want LOCK to be recursive. If you happen to write your application properly following one of the few obscure practical examples about explicit locking, and then the DBA partitions the table under you, you lose quite badly.
Peter Eisentraut <peter_e@gmx.net> writes:
> [ good summary ]
+1 for making TRUNCATE and LOCK support ONLY. I don't care much about
ALTER TABLE SET SCHEMA, but perhaps there's a use-case for recursion
on that. We should stay away from recursive CREATE INDEX for the
moment --- for one thing, you'd have to invent names for the
additional indexes.
I wonder whether GRANT/REVOKE shouldn't be made to support recursion
too. We have a standard warning "don't forget to grant rights on the
child tables" ...
regards, tom lane
On Wed, Jan 07, 2009 at 11:17:46AM -0500, Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > > [ good summary ] > > +1 for making TRUNCATE and LOCK support ONLY. I don't care much > about ALTER TABLE SET SCHEMA, but perhaps there's a use-case for > recursion on that. We should stay away from recursive CREATE INDEX > for the moment --- for one thing, you'd have to invent names for the > additional indexes. > > I wonder whether GRANT/REVOKE shouldn't be made to support recursion > too. We have a standard warning "don't forget to grant rights on > the child tables" ... +1 for adding recursion to GRANT/REVOKE :) Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
David Fetter wrote: > +1 for adding recursion to GRANT/REVOKE :) This area is under SQL standard control, so we can't really invent our own behavior. Consider the following: CREATE TABLE persons (name, email); CREATE TABLE employees (grade, salary) INHERITS (persons); GRANT SELECT ON persons TO allstaff; -- ??? GRANT SELECT ON employees TO managers; What you want in practice is that allstaff can read only those columns of employees that come from the persons table. Both recursive and nonrecursive GRANT do the wrong thing here.
Tom Lane wrote:
> +1 for making TRUNCATE and LOCK support ONLY.
Patch attached.
> I don't care much about
> ALTER TABLE SET SCHEMA, but perhaps there's a use-case for recursion
> on that.
I have added this to the Todo list for later reconsideration.
Index: doc/src/sgml/ref/lock.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/ref/lock.sgml,v
retrieving revision 1.51
diff -u -3 -p -c -r1.51 lock.sgml
*** doc/src/sgml/ref/lock.sgml 14 Nov 2008 10:22:47 -0000 1.51
--- doc/src/sgml/ref/lock.sgml 8 Jan 2009 13:27:47 -0000
*************** PostgreSQL documentation
*** 21,27 ****
<refsynopsisdiv>
<synopsis>
! LOCK [ TABLE ] <replaceable class="PARAMETER">name</replaceable> [, ...] [ IN <replaceable
class="PARAMETER">lockmode</replaceable>MODE ] [ NOWAIT ]
where <replaceable class="PARAMETER">lockmode</replaceable> is one of:
--- 21,27 ----
<refsynopsisdiv>
<synopsis>
! LOCK [ TABLE ] [ ONLY ] <replaceable class="PARAMETER">name</replaceable> [, ...] [ IN <replaceable
class="PARAMETER">lockmode</replaceable>MODE ] [ NOWAIT ]
where <replaceable class="PARAMETER">lockmode</replaceable> is one of:
*************** where <replaceable class="PARAMETER">loc
*** 109,115 ****
<listitem>
<para>
The name (optionally schema-qualified) of an existing table to
! lock.
</para>
<para>
--- 109,117 ----
<listitem>
<para>
The name (optionally schema-qualified) of an existing table to
! lock. If <literal>ONLY</> is specified, only that table is
! locked. If <literal>ONLY</> is not specified, the table and all
! its descendant tables (if any) are locked.
</para>
<para>
Index: doc/src/sgml/ref/truncate.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/ref/truncate.sgml,v
retrieving revision 1.31
diff -u -3 -p -c -r1.31 truncate.sgml
*** doc/src/sgml/ref/truncate.sgml 18 Dec 2008 10:45:00 -0000 1.31
--- doc/src/sgml/ref/truncate.sgml 8 Jan 2009 13:27:47 -0000
*************** PostgreSQL documentation
*** 21,27 ****
<refsynopsisdiv>
<synopsis>
! TRUNCATE [ TABLE ] <replaceable class="PARAMETER">name</replaceable> [, ... ]
[ RESTART IDENTITY | CONTINUE IDENTITY ] [ CASCADE | RESTRICT ]
</synopsis>
</refsynopsisdiv>
--- 21,27 ----
<refsynopsisdiv>
<synopsis>
! TRUNCATE [ TABLE ] [ ONLY ] <replaceable class="PARAMETER">name</replaceable> [, ... ]
[ RESTART IDENTITY | CONTINUE IDENTITY ] [ CASCADE | RESTRICT ]
</synopsis>
</refsynopsisdiv>
*************** TRUNCATE [ TABLE ] <replaceable class="P
*** 47,53 ****
<term><replaceable class="PARAMETER">name</replaceable></term>
<listitem>
<para>
! The name (optionally schema-qualified) of a table to be truncated.
</para>
</listitem>
</varlistentry>
--- 47,56 ----
<term><replaceable class="PARAMETER">name</replaceable></term>
<listitem>
<para>
! The name (optionally schema-qualified) of a table to be
! truncated. If <literal>ONLY</> is specified, only that table is
! truncated. If <literal>ONLY</> is not specified, the table and
! all its descendant tables (if any) are truncated.
</para>
</listitem>
</varlistentry>
Index: src/backend/commands/lockcmds.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/lockcmds.c,v
retrieving revision 1.20
diff -u -3 -p -c -r1.20 lockcmds.c
*** src/backend/commands/lockcmds.c 1 Jan 2009 17:23:38 -0000 1.20
--- src/backend/commands/lockcmds.c 8 Jan 2009 13:27:47 -0000
***************
*** 18,23 ****
--- 18,25 ----
#include "catalog/namespace.h"
#include "commands/lockcmds.h"
#include "miscadmin.h"
+ #include "optimizer/prep.h"
+ #include "parser/parse_clause.h"
#include "utils/acl.h"
#include "utils/lsyscache.h"
#include "utils/rel.h"
*************** LockTableCommand(LockStmt *lockstmt)
*** 40,77 ****
{
RangeVar *relation = lfirst(p);
Oid reloid;
! AclResult aclresult;
! Relation rel;
- /*
- * We don't want to open the relation until we've checked privilege.
- * So, manually get the relation OID.
- */
reloid = RangeVarGetRelid(relation, false);
! if (lockstmt->mode == AccessShareLock)
! aclresult = pg_class_aclcheck(reloid, GetUserId(),
! ACL_SELECT);
else
! aclresult = pg_class_aclcheck(reloid, GetUserId(),
! ACL_UPDATE | ACL_DELETE | ACL_TRUNCATE);
! if (aclresult != ACLCHECK_OK)
! aclcheck_error(aclresult, ACL_KIND_CLASS,
! get_rel_name(reloid));
! if (lockstmt->nowait)
! rel = relation_open_nowait(reloid, lockstmt->mode);
! else
! rel = relation_open(reloid, lockstmt->mode);
!
! /* Currently, we only allow plain tables to be locked */
! if (rel->rd_rel->relkind != RELKIND_RELATION)
! ereport(ERROR,
! (errcode(ERRCODE_WRONG_OBJECT_TYPE),
! errmsg("\"%s\" is not a table",
! relation->relname)));
!
! relation_close(rel, NoLock); /* close rel, keep lock */
}
}
--- 42,89 ----
{
RangeVar *relation = lfirst(p);
Oid reloid;
! bool recurse = interpretInhOption(relation->inhOpt);
! List *children_and_self;
! ListCell *child;
reloid = RangeVarGetRelid(relation, false);
! if (recurse)
! children_and_self = find_all_inheritors(reloid);
else
! children_and_self = list_make1_oid(reloid);
! foreach(child, children_and_self)
! {
! Oid childreloid = lfirst_oid(child);
! Relation rel;
! AclResult aclresult;
!
! /* We don't want to open the relation until we've checked privilege. */
! if (lockstmt->mode == AccessShareLock)
! aclresult = pg_class_aclcheck(childreloid, GetUserId(),
! ACL_SELECT);
! else
! aclresult = pg_class_aclcheck(childreloid, GetUserId(),
! ACL_UPDATE | ACL_DELETE | ACL_TRUNCATE);
!
! if (aclresult != ACLCHECK_OK)
! aclcheck_error(aclresult, ACL_KIND_CLASS,
! get_rel_name(childreloid));
!
! if (lockstmt->nowait)
! rel = relation_open_nowait(childreloid, lockstmt->mode);
! else
! rel = relation_open(childreloid, lockstmt->mode);
!
! /* Currently, we only allow plain tables to be locked */
! if (rel->rd_rel->relkind != RELKIND_RELATION)
! ereport(ERROR,
! (errcode(ERRCODE_WRONG_OBJECT_TYPE),
! errmsg("\"%s\" is not a table",
! get_rel_name(childreloid))));
! relation_close(rel, NoLock); /* close rel, keep lock */
! }
}
}
Index: src/backend/commands/tablecmds.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.276
diff -u -3 -p -c -r1.276 tablecmds.c
*** src/backend/commands/tablecmds.c 1 Jan 2009 17:23:39 -0000 1.276
--- src/backend/commands/tablecmds.c 8 Jan 2009 13:27:47 -0000
*************** ExecuteTruncate(TruncateStmt *stmt)
*** 772,788 ****
{
RangeVar *rv = lfirst(cell);
Relation rel;
rel = heap_openrv(rv, AccessExclusiveLock);
/* don't throw error for "TRUNCATE foo, foo" */
! if (list_member_oid(relids, RelationGetRelid(rel)))
{
heap_close(rel, AccessExclusiveLock);
continue;
}
truncate_check_rel(rel);
rels = lappend(rels, rel);
! relids = lappend_oid(relids, RelationGetRelid(rel));
}
/*
--- 772,812 ----
{
RangeVar *rv = lfirst(cell);
Relation rel;
+ bool recurse = interpretInhOption(rv->inhOpt);
+ Oid myrelid;
rel = heap_openrv(rv, AccessExclusiveLock);
+ myrelid = RelationGetRelid(rel);
/* don't throw error for "TRUNCATE foo, foo" */
! if (list_member_oid(relids, myrelid))
{
heap_close(rel, AccessExclusiveLock);
continue;
}
truncate_check_rel(rel);
rels = lappend(rels, rel);
! relids = lappend_oid(relids, myrelid);
!
! if (recurse)
! {
! ListCell *child;
! List *children;
!
! children = find_all_inheritors(myrelid);
!
! foreach(child, children)
! {
! Oid childrelid = lfirst_oid(child);
!
! if (list_member_oid(relids, childrelid))
! continue;
!
! rel = heap_open(childrelid, AccessExclusiveLock);
! truncate_check_rel(rel);
! rels = lappend(rels, rel);
! relids = lappend_oid(relids, childrelid);
! }
! }
}
/*
Index: src/backend/parser/gram.y
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.652
diff -u -3 -p -c -r2.652 gram.y
*** src/backend/parser/gram.y 7 Jan 2009 22:54:45 -0000 2.652
--- src/backend/parser/gram.y 8 Jan 2009 13:27:47 -0000
*************** static TypeName *TableFuncTypeName(List
*** 284,289 ****
--- 284,290 ----
execute_param_clause using_clause returning_clause
enum_val_list table_func_column_list
create_generic_options alter_generic_options
+ relation_expr_list
%type <range> OptTempTableName
%type <into> into_clause create_as_target
*************** attrs: '.' attr_name
*** 3794,3800 ****
*****************************************************************************/
TruncateStmt:
! TRUNCATE opt_table qualified_name_list opt_restart_seqs opt_drop_behavior
{
TruncateStmt *n = makeNode(TruncateStmt);
n->relations = $3;
--- 3795,3801 ----
*****************************************************************************/
TruncateStmt:
! TRUNCATE opt_table relation_expr_list opt_restart_seqs opt_drop_behavior
{
TruncateStmt *n = makeNode(TruncateStmt);
n->relations = $3;
*************** using_clause:
*** 6558,6564 ****
| /*EMPTY*/ { $$ = NIL; }
;
! LockStmt: LOCK_P opt_table qualified_name_list opt_lock opt_nowait
{
LockStmt *n = makeNode(LockStmt);
--- 6559,6573 ----
| /*EMPTY*/ { $$ = NIL; }
;
!
! /*****************************************************************************
! *
! * QUERY:
! * LOCK TABLE
! *
! *****************************************************************************/
!
! LockStmt: LOCK_P opt_table relation_expr_list opt_lock opt_nowait
{
LockStmt *n = makeNode(LockStmt);
*************** relation_expr:
*** 7487,7492 ****
--- 7496,7507 ----
;
+ relation_expr_list:
+ relation_expr { $$ = list_make1($1); }
+ | relation_expr_list ',' relation_expr { $$ = lappend($1, $3); }
+ ;
+
+
/*
* Given "UPDATE foo set set ...", we have to decide without looking any
* further ahead whether the first "set" is an alias or the UPDATE's SET
Index: src/test/regress/expected/truncate.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/truncate.out,v
retrieving revision 1.18
diff -u -3 -p -c -r1.18 truncate.out
*** src/test/regress/expected/truncate.out 1 Sep 2008 20:42:46 -0000 1.18
--- src/test/regress/expected/truncate.out 8 Jan 2009 13:27:48 -0000
*************** SELECT * FROM trunc_e;
*** 141,146 ****
--- 141,290 ----
(0 rows)
DROP TABLE truncate_a,trunc_c,trunc_b,trunc_d,trunc_e CASCADE;
+ -- Test TRUNCATE with inheritance
+ CREATE TABLE trunc_f (col1 integer primary key);
+ NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "trunc_f_pkey" for table "trunc_f"
+ INSERT INTO trunc_f VALUES (1);
+ INSERT INTO trunc_f VALUES (2);
+ CREATE TABLE trunc_fa (col2a text) INHERITS (trunc_f);
+ INSERT INTO trunc_fa VALUES (3, 'three');
+ CREATE TABLE trunc_fb (col2b int) INHERITS (trunc_f);
+ INSERT INTO trunc_fb VALUES (4, 444);
+ CREATE TABLE trunc_faa (col3 text) INHERITS (trunc_fa);
+ INSERT INTO trunc_faa VALUES (5, 'five', 'FIVE');
+ BEGIN;
+ SELECT * FROM trunc_f;
+ col1
+ ------
+ 1
+ 2
+ 3
+ 4
+ 5
+ (5 rows)
+
+ TRUNCATE trunc_f;
+ SELECT * FROM trunc_f;
+ col1
+ ------
+ (0 rows)
+
+ ROLLBACK;
+ BEGIN;
+ SELECT * FROM trunc_f;
+ col1
+ ------
+ 1
+ 2
+ 3
+ 4
+ 5
+ (5 rows)
+
+ TRUNCATE ONLY trunc_f;
+ SELECT * FROM trunc_f;
+ col1
+ ------
+ 3
+ 4
+ 5
+ (3 rows)
+
+ ROLLBACK;
+ BEGIN;
+ SELECT * FROM trunc_f;
+ col1
+ ------
+ 1
+ 2
+ 3
+ 4
+ 5
+ (5 rows)
+
+ SELECT * FROM trunc_fa;
+ col1 | col2a
+ ------+-------
+ 3 | three
+ 5 | five
+ (2 rows)
+
+ SELECT * FROM trunc_faa;
+ col1 | col2a | col3
+ ------+-------+------
+ 5 | five | FIVE
+ (1 row)
+
+ TRUNCATE ONLY trunc_fb, ONLY trunc_fa;
+ SELECT * FROM trunc_f;
+ col1
+ ------
+ 1
+ 2
+ 5
+ (3 rows)
+
+ SELECT * FROM trunc_fa;
+ col1 | col2a
+ ------+-------
+ 5 | five
+ (1 row)
+
+ SELECT * FROM trunc_faa;
+ col1 | col2a | col3
+ ------+-------+------
+ 5 | five | FIVE
+ (1 row)
+
+ ROLLBACK;
+ BEGIN;
+ SELECT * FROM trunc_f;
+ col1
+ ------
+ 1
+ 2
+ 3
+ 4
+ 5
+ (5 rows)
+
+ SELECT * FROM trunc_fa;
+ col1 | col2a
+ ------+-------
+ 3 | three
+ 5 | five
+ (2 rows)
+
+ SELECT * FROM trunc_faa;
+ col1 | col2a | col3
+ ------+-------+------
+ 5 | five | FIVE
+ (1 row)
+
+ TRUNCATE ONLY trunc_fb, trunc_fa;
+ SELECT * FROM trunc_f;
+ col1
+ ------
+ 1
+ 2
+ (2 rows)
+
+ SELECT * FROM trunc_fa;
+ col1 | col2a
+ ------+-------
+ (0 rows)
+
+ SELECT * FROM trunc_faa;
+ col1 | col2a | col3
+ ------+-------+------
+ (0 rows)
+
+ ROLLBACK;
+ DROP TABLE trunc_f CASCADE;
+ NOTICE: drop cascades to 3 other objects
+ DETAIL: drop cascades to table trunc_fa
+ drop cascades to table trunc_faa
+ drop cascades to table trunc_fb
-- Test ON TRUNCATE triggers
CREATE TABLE trunc_trigger_test (f1 int, f2 text, f3 text);
CREATE TABLE trunc_trigger_log (tgop text, tglevel text, tgwhen text,
Index: src/test/regress/sql/truncate.sql
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/sql/truncate.sql,v
retrieving revision 1.7
diff -u -3 -p -c -r1.7 truncate.sql
*** src/test/regress/sql/truncate.sql 16 May 2008 23:36:05 -0000 1.7
--- src/test/regress/sql/truncate.sql 8 Jan 2009 13:27:48 -0000
*************** SELECT * FROM trunc_e;
*** 78,83 ****
--- 78,132 ----
DROP TABLE truncate_a,trunc_c,trunc_b,trunc_d,trunc_e CASCADE;
+ -- Test TRUNCATE with inheritance
+
+ CREATE TABLE trunc_f (col1 integer primary key);
+ INSERT INTO trunc_f VALUES (1);
+ INSERT INTO trunc_f VALUES (2);
+
+ CREATE TABLE trunc_fa (col2a text) INHERITS (trunc_f);
+ INSERT INTO trunc_fa VALUES (3, 'three');
+
+ CREATE TABLE trunc_fb (col2b int) INHERITS (trunc_f);
+ INSERT INTO trunc_fb VALUES (4, 444);
+
+ CREATE TABLE trunc_faa (col3 text) INHERITS (trunc_fa);
+ INSERT INTO trunc_faa VALUES (5, 'five', 'FIVE');
+
+ BEGIN;
+ SELECT * FROM trunc_f;
+ TRUNCATE trunc_f;
+ SELECT * FROM trunc_f;
+ ROLLBACK;
+
+ BEGIN;
+ SELECT * FROM trunc_f;
+ TRUNCATE ONLY trunc_f;
+ SELECT * FROM trunc_f;
+ ROLLBACK;
+
+ BEGIN;
+ SELECT * FROM trunc_f;
+ SELECT * FROM trunc_fa;
+ SELECT * FROM trunc_faa;
+ TRUNCATE ONLY trunc_fb, ONLY trunc_fa;
+ SELECT * FROM trunc_f;
+ SELECT * FROM trunc_fa;
+ SELECT * FROM trunc_faa;
+ ROLLBACK;
+
+ BEGIN;
+ SELECT * FROM trunc_f;
+ SELECT * FROM trunc_fa;
+ SELECT * FROM trunc_faa;
+ TRUNCATE ONLY trunc_fb, trunc_fa;
+ SELECT * FROM trunc_f;
+ SELECT * FROM trunc_fa;
+ SELECT * FROM trunc_faa;
+ ROLLBACK;
+
+ DROP TABLE trunc_f CASCADE;
+
-- Test ON TRUNCATE triggers
CREATE TABLE trunc_trigger_test (f1 int, f2 text, f3 text);
On Thu, Jan 08, 2009 at 02:39:52PM +0200, Peter Eisentraut wrote: > David Fetter wrote: >> +1 for adding recursion to GRANT/REVOKE :) > > This area is under SQL standard control, so we can't really invent our > own behavior. > > Consider the following: > > CREATE TABLE persons (name, email); > CREATE TABLE employees (grade, salary) INHERITS (persons); > > GRANT SELECT ON persons TO allstaff; -- ??? > GRANT SELECT ON employees TO managers; > > What you want in practice is that allstaff can read only those columns > of employees that come from the persons table. Both recursive and > nonrecursive GRANT do the wrong thing here. What *would* do the right thing here, or would anything? Cheers, David (not getting into the design decisions implicit in the above tables, which IMHO is not right) -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
David Fetter wrote: > On Thu, Jan 08, 2009 at 02:39:52PM +0200, Peter Eisentraut wrote: >> David Fetter wrote: >>> +1 for adding recursion to GRANT/REVOKE :) >> This area is under SQL standard control, so we can't really invent our >> own behavior. >> >> Consider the following: >> >> CREATE TABLE persons (name, email); >> CREATE TABLE employees (grade, salary) INHERITS (persons); >> >> GRANT SELECT ON persons TO allstaff; -- ??? >> GRANT SELECT ON employees TO managers; >> >> What you want in practice is that allstaff can read only those columns >> of employees that come from the persons table. Both recursive and >> nonrecursive GRANT do the wrong thing here. > > What *would* do the right thing here, or would anything? I think we don't need GRANT to be recursive, but instead the permission checks at runtime should allow SELECT * FROM persons; to succeed even if there are no permissions on "employees". But only on the columns of "persons" and only if actually queried through "persons". Needs a more detailed analysis, but that is how I imagine it ought to work.
Peter Eisentraut <peter_e@gmx.net> writes:
>>> This area is under SQL standard control, so we can't really invent our
>>> own behavior.
>> What *would* do the right thing here, or would anything?
> I think we don't need GRANT to be recursive, but instead the permission
> checks at runtime should allow
> SELECT * FROM persons;
> to succeed even if there are no permissions on "employees".
Hmm, if we are supposing that the spec should control this, then
surely we can find chapter and verse spelling out what should happen.
regards, tom lane
Peter Eisentraut wrote: > Tom Lane wrote: >> +1 for making TRUNCATE and LOCK support ONLY. > > Patch attached. This was committed.
I wrote: > Here is the current line-up: > > command supports ONLY > > ALTER TABLE all other actions yes > ALTER TABLE RENAME COLUMN yes > ALTER TABLE RENAME no > ALTER TABLE SET SCHEMA documented no, but accepted and ignored This is actually a bit worse: All variants of ALTER TABLE accept ONLY, but only about half of them are potentially recursive and about half of them never recurse, and this is not documented in an obvious place (or anywhere). I have added a Todo list item to sort this out.
Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: >>>> This area is under SQL standard control, so we can't really invent our >>>> own behavior. > >>> What *would* do the right thing here, or would anything? > >> I think we don't need GRANT to be recursive, but instead the permission >> checks at runtime should allow >> SELECT * FROM persons; >> to succeed even if there are no permissions on "employees". > > Hmm, if we are supposing that the spec should control this, then > surely we can find chapter and verse spelling out what should happen. The SQL standard uses a recursive-by-default language. For example, the rules for the DELETE command state: """ 6) Case: a) If <target table> contains ONLY, then the rows for which the result of the <search condition> is True and for which there is no subrow in a proper subtable of T are identified for deletion from T. b) Otherwise, the rows for which the result of the <search condition> is True are identified for deletion from T. """ So when the SQL standard says, privileges are granted on this table, or $action is done on that table, it means, in PostgreSQL terms, the table and its children.
On Mon, 2009-01-12 at 11:43 +0200, Peter Eisentraut wrote: > Peter Eisentraut wrote: > > Tom Lane wrote: > >> +1 for making TRUNCATE and LOCK support ONLY. > > > > Patch attached. > > This was committed. Please could we put in a GUC to allow that to be toggled in this release and warning issued for non-optional behaviour change in following release? This seems like a dangerous behaviour change for some apps and may be a blocker to upgrade, as the changes in casting behaviour have proved. -- Simon Riggs www.2ndQuadrant.comPostgreSQL Training, Services and Support
Simon Riggs <simon@2ndQuadrant.com> writes: > On Mon, 2009-01-12 at 11:43 +0200, Peter Eisentraut wrote: >> Peter Eisentraut wrote: >> > Tom Lane wrote: >> >> +1 for making TRUNCATE and LOCK support ONLY. >> > >> > Patch attached. >> >> This was committed. > > Please could we put in a GUC to allow that to be toggled in this release That seems like it would just be putting off the pain. It doesn't make it any easier to migrate in the end. > and warning issued for non-optional behaviour change in following > release? We do print INFO messages when drops cascade. We could print similar messages when DDL applies recursively by default. (We can't do DML since it would fill logs quickly). That seems reasonable to me. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services!
Gregory Stark <stark@enterprisedb.com> writes:
> Simon Riggs <simon@2ndQuadrant.com> writes:
>> Please could we put in a GUC to allow that to be toggled in this release
> That seems like it would just be putting off the pain.
Yes, we already had exactly this discussion and concluded that a GUC
wasn't going to improve matters.
regards, tom lane
Peter Eisentraut <peter_e@gmx.net> writes:
> The SQL standard uses a recursive-by-default language. For example, the
> rules for the DELETE command state:
Actually, I'm not convinced. Take a look at the SELECT WITH HIERARCHY
OPTION stuff in SQL99 and later, in particular this from SQL99
12.2 <grant privilege statement>:
7) Let SWH be the set of privilege descriptors in CPD whose action is SELECT WITH HIERARCHY OPTION,
andlet ST be the set of subtables of O, then for every grantee G in SWH and for every table T in
ST,the following <grant statement> is effectively executed without further Access Rule checking:
GRANT SELECT ON T TO G GRANTED BY A
It's difficult to read that any other way than that privileges are *not*
auto-recursive, and they have chosen to spell "*" in GRANT as "WITH
HIERARCHY OPTION" (gackk).
On the other hand, it's hard to square that reading with the lack of any
UPDATE or DELETE WITH HIERARCHY OPTION syntax. What am I missing here?
regards, tom lane
Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > >> The SQL standard uses a recursive-by-default language. For example, the >> rules for the DELETE command state: >> > > Actually, I'm not convinced. Take a look at the SELECT WITH HIERARCHY > OPTION stuff in SQL99 and later, in particular this from SQL99 > 12.2 <grant privilege statement>: > > 7) Let SWH be the set of privilege descriptors in CPD whose action > is SELECT WITH HIERARCHY OPTION, and let ST be the set of > subtables of O, then for every grantee G in SWH and for every > table T in ST, the following <grant statement> is effectively > executed without further Access Rule checking: > > GRANT SELECT ON T TO G GRANTED BY A > > It's difficult to read that any other way than that privileges are *not* > auto-recursive, and they have chosen to spell "*" in GRANT as "WITH > HIERARCHY OPTION" (gackk). > > On the other hand, it's hard to square that reading with the lack of any > UPDATE or DELETE WITH HIERARCHY OPTION syntax. What am I missing here? > > > It's just occurred to me that if TRUNCATE no longer means TRUNCATE ONLY, parallel restore will need to detect which server version is being used so that for version > 8.3 it issues TRUNCATE ONLY. Otherwise there would be a danger of a collision between a table and its children. The only alternative would be to create a dependency between the data of a table and the data of its children, which would be undesirable as well as more complicated - in general the data should only depend on the table creation (at most). cheers andrew
Andrew Dunstan wrote: > It's just occurred to me that if TRUNCATE no longer means TRUNCATE ONLY, > parallel restore will need to detect which server version is being used > so that for version > 8.3 it issues TRUNCATE ONLY. The pg_dump output was never backward compatible. (The input is.) So the output of parallel restore need not be backward compatible either. (Unless this mandate has changed dramatically while I was not looking?) So always issue TRUNCATE ONLY, if that is what thelogic requires. The additional benefit is that this will fail safely on older versions.
Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: >> The SQL standard uses a recursive-by-default language. For example, the >> rules for the DELETE command state: > > Actually, I'm not convinced. Take a look at the SELECT WITH HIERARCHY > OPTION stuff in SQL99 and later, in particular this from SQL99 > 12.2 <grant privilege statement>: Ah, the mysterious HIERARCHY OPTION comes into play. That appears to be the ticket. > 7) Let SWH be the set of privilege descriptors in CPD whose action > is SELECT WITH HIERARCHY OPTION, and let ST be the set of > subtables of O, then for every grantee G in SWH and for every > table T in ST, the following <grant statement> is effectively > executed without further Access Rule checking: > > GRANT SELECT ON T TO G GRANTED BY A > > It's difficult to read that any other way than that privileges are *not* > auto-recursive, and they have chosen to spell "*" in GRANT as "WITH > HIERARCHY OPTION" (gackk). Er, well, I see this piece from SQL:2008 on <table reference>: """ 1) Case: [...] B) [...], the current privileges shall include SELECT on at least one column of T. 2) If TP simply contains <only spec> and TN identifies a typed table, then Case: [...] B) [...], the current privileges shall include SELECT WITH HIERARCHY OPTION on at least one supertable of T. """ (The omitted phrases deal with SECURITY INVOKER situations.) I read that as that privileges are auto-recursive, and that you need the hierarchy option to be permitted to use ONLY. (So the hierarchy option is an additional privilege on top of SELECT that allows you to break the encapsulation of the inheritance setup.) > On the other hand, it's hard to square that reading with the lack of any > UPDATE or DELETE WITH HIERARCHY OPTION syntax. What am I missing here? You need SELECT with or without HIERARCHY, as the case may be, to locate the row. Once you have located it, you can UPDATE or DELETE it depending on privilege, but then it doesn't matter anymore how you got it.
Peter Eisentraut wrote: > Andrew Dunstan wrote: >> It's just occurred to me that if TRUNCATE no longer means TRUNCATE ONLY, >> parallel restore will need to detect which server version is being >> used so that for version > 8.3 it issues TRUNCATE ONLY. > > The pg_dump output was never backward compatible. (The input is.) So > the output of parallel restore need not be backward compatible either. > (Unless this mandate has changed dramatically while I was not > looking?) So always issue TRUNCATE ONLY, if that is what the logic > requires. The additional benefit is that this will fail safely on > older versions. > No it won't fail safely on older versions, because the truncate is part of a transaction, and thus the data member(s) will all fail. I'd like to be able to use 8.4 pg_restore to run parallel restores on older servers, and the fix for this is utterly trivial. I'll be posting a new patch with it in today. (If we can't or don't want to make it work with older servers, I will create an out-of-tree patch for 8.3 that does, and put it on pgFoundry. But that would be a pity.) cheers andrew
Andrew Dunstan wrote: >> The pg_dump output was never backward compatible. (The input is.) So >> the output of parallel restore need not be backward compatible either. >> (Unless this mandate has changed dramatically while I was not >> looking?) So always issue TRUNCATE ONLY, if that is what the logic >> requires. The additional benefit is that this will fail safely on >> older versions. > No it won't fail safely on older versions, because the truncate is part > of a transaction, and thus the data member(s) will all fail. I meant "safe" as in, it won't randomly delete more data than you intended. I didn't mean in as in do-what-I-mean. :-) > I'd like to > be able to use 8.4 pg_restore to run parallel restores on older servers, > and the fix for this is utterly trivial. I'll be posting a new patch > with it in today. Works for me.