TRUNCATE, VACUUM, ANALYZE privileges

Greetings,

The following patch implements individual privileges for TRUNCATE,
VACUUM and ANALYZE. Includes documentation and regression test
updates. Resolves TODO item 'Add a separate TRUNCATE permission'.
Created off of current (2005/01/03) CVS TIP.

At least the 'no one interested has written a patch' argument is gone
now, fire away with other comments/concerns. :)

Thanks,

Stephen

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> The following patch implements individual privileges for TRUNCATE,
> VACUUM and ANALYZE. Includes documentation and regression test
> updates. Resolves TODO item 'Add a separate TRUNCATE permission'.

> At least the 'no one interested has written a patch' argument is gone
> now, fire away with other comments/concerns. :)

I have a very serious problem with the idea of inventing individual
privilege bits for every maintenance command in sight. That does not
scale. How will you handle "GRANT ADD COLUMN", or "GRANT ADD COLUMN
as-long-as-its-not-SERIAL-because-I-dont-want-you-creating-sequences",
or "GRANT ALTER TABLE RELIABILITY" as soon as someone writes that patch,
or a dozen other cases that I could name without stopping for breath?

The proposed patch eats three of the five available privilege bits (that
is, available without accepting the distributed cost of enlarging ACL
bitmasks), and you've made no case at all why we should spend that
limited resource in this particular fashion.

regards, tom lane

On Tue, Jan 03, 2006 at 11:32:01PM -0500, Tom Lane wrote:
> Stephen Frost <sfrost(at)snowman(dot)net> writes:
> > The following patch implements individual privileges for TRUNCATE,
> > VACUUM and ANALYZE. Includes documentation and regression test
> > updates. Resolves TODO item 'Add a separate TRUNCATE permission'.
>
> > At least the 'no one interested has written a patch' argument is gone
> > now, fire away with other comments/concerns. :)
>
> I have a very serious problem with the idea of inventing individual
> privilege bits for every maintenance command in sight. That does not
> scale. How will you handle "GRANT ADD COLUMN", or "GRANT ADD COLUMN
> as-long-as-its-not-SERIAL-because-I-dont-want-you-creating-sequences",
> or "GRANT ALTER TABLE RELIABILITY" as soon as someone writes that patch,
> or a dozen other cases that I could name without stopping for breath?
>
> The proposed patch eats three of the five available privilege bits (that
> is, available without accepting the distributed cost of enlarging ACL
> bitmasks), and you've made no case at all why we should spend that
> limited resource in this particular fashion.

We rely heavily on truncate as delete for large numbers of rows is very
costly. An example, we copy_in batches of rows from several sources through
the day to a "pending work" table, with another process periodically
processing the rows and sweeping them into a history table. The sweep
leaves an empty "pending work" table. Truncate is very efficient for this
pattern.

However it means that all our jobs have to run with more permissions than
they really should have as there is no way to grant "truncate". If giving
truncate its very own permission is too wasteful of permission bits, perhaps
having truncate be the same as "delete" for permissions purposes would work.

Alternatively a separate "whole table operations" permision might cover
truncate and some of the alter type things too. Of course table owner does
this, but that is what I don't want everyone to be require to have.

-dg

--
David Gould daveg(at)sonic(dot)net
If simplicity worked, the world would be overrun with insects.

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Stephen Frost <sfrost(at)snowman(dot)net> writes:
> > The following patch implements individual privileges for TRUNCATE,
> > VACUUM and ANALYZE. Includes documentation and regression test
> > updates. Resolves TODO item 'Add a separate TRUNCATE permission'.
>
> > At least the 'no one interested has written a patch' argument is gone
> > now, fire away with other comments/concerns. :)
>
> I have a very serious problem with the idea of inventing individual
> privilege bits for every maintenance command in sight. That does not
> scale. How will you handle "GRANT ADD COLUMN", or "GRANT ADD COLUMN
> as-long-as-its-not-SERIAL-because-I-dont-want-you-creating-sequences",
> or "GRANT ALTER TABLE RELIABILITY" as soon as someone writes that patch,
> or a dozen other cases that I could name without stopping for breath?

GRANT ADD COLUMN, etc, aren't maintenance commands, they're DDL
statements and as such should be the purview of the owner. TRUNCATE,
VACUUM and ANALYZE are DML commands and are commands a user of
the table would use through the normal course of inserting, updating or
deleteing data in the table.

> The proposed patch eats three of the five available privilege bits (that
> is, available without accepting the distributed cost of enlarging ACL
> bitmasks), and you've made no case at all why we should spend that
> limited resource in this particular fashion.

I've shown a specific use-case for this. It's been asked for before by
others. I've shown why these particular ones make sense (while 'ADD
COLUMN', etc, don't). If we come up with more Postgres-specific DML
statements which aren't covered by other grants (which doesn't seem
terribly likely at this point) then we should add those. I could see
making VACUUM and ANALYZE use the same bit (since one implies the other)
but I'm not really a big fan of that and I don't see any other need for
these bits coming down the line anytime soon.

Thanks,

Stephen

* daveg (daveg(at)sonic(dot)net) wrote:
> We rely heavily on truncate as delete for large numbers of rows is very
> costly. An example, we copy_in batches of rows from several sources through
> the day to a "pending work" table, with another process periodically
> processing the rows and sweeping them into a history table. The sweep
> leaves an empty "pending work" table. Truncate is very efficient for this
> pattern.
>
> However it means that all our jobs have to run with more permissions than
> they really should have as there is no way to grant "truncate". If giving
> truncate its very own permission is too wasteful of permission bits, perhaps
> having truncate be the same as "delete" for permissions purposes would work.

Sounds very similar to my use-case, except my users just have to suffer
with delete because I don't want to grant them additional permissions.
Having truncate act off of delete isn't actually an option
unfortunately. This is because truncate skips triggers (probably not an
issue for you, certainly not one for me, but a problem with doing it in
the general case).

I'm not sure about you, but I know that I'd like to be able to do:
TRUNCATE, insert/copy data, ANALYZE without having to give all the other
permissions associated with ownership.

> Alternatively a separate "whole table operations" permision might cover
> truncate and some of the alter type things too. Of course table owner does
> this, but that is what I don't want everyone to be require to have.

I'm not entirely sure if that'd be better or not.. It would involve
changing the structure of the ACLs to have two sets for each relation
and you'd have to sometimes look at one, sometimes at the other, and
possible both in some cases...

Thanks,

Stephen

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> GRANT ADD COLUMN, etc, aren't maintenance commands, they're DDL
> statements and as such should be the purview of the owner. TRUNCATE,
> VACUUM and ANALYZE are DML commands and are commands a user of
> the table would use through the normal course of inserting, updating or
> deleteing data in the table.

I find this reasoning fairly dubious. In particular, it's hard to argue
that there is no DDL component to TRUNCATE when it effectively does an
implicit disable-triggers operation. Another thing setting TRUNCATE
apart from run-of-the-mill DDL operations is that it inherently violates
MVCC rules (by deleting rows that should still be visible to concurrent
transactions).

But my real problem with the approach is that I don't see where it
stops. If you're allowed to do ANALYZE, why not ALTER TABLE SET
STATISTICS? If you're allowed to do TRUNCATE, why not the
recently-discussed ALTER TABLE SET RELIABILITY? And how about CLUSTER?
All of these could be pretty useful for some applications not too far
removed from yours. And there will be someone wanting a bit for
DISABLE/ENABLE TRIGGER coming along right afterwards. Must we implement
a separate nonstandard privilege bit for every operation that someone
comes up and wants a bit for, if they have the necessary cut-and-paste
skill to submit a patch for it?

I'd feel happier about an approach that adds *one* privilege bit
covering a range of operations that we agree to be useful. This will
avoid chewing a disproportionate amount of ACL storage space, and it
will force us to confront the decision about which operations are out
as well as which are in.

One last point: -patches is not the place for this type of discussion.

regards, tom lane

Tom, et al,

Sorry for the longish email; if you're most interested in a change to
the ACL system to allow more privileges then skip to the bottom where
I worked up a change to give us more options without much of a
performance impact (I don't think anyway). Personally, I'd prefer that
to overloading the bits we have (except perhaps for vacuum/analyze).

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Stephen Frost <sfrost(at)snowman(dot)net> writes:
> > GRANT ADD COLUMN, etc, aren't maintenance commands, they're DDL
> > statements and as such should be the purview of the owner. TRUNCATE,
> > VACUUM and ANALYZE are DML commands and are commands a user of
> > the table would use through the normal course of inserting, updating or
> > deleteing data in the table.
>
> I find this reasoning fairly dubious. In particular, it's hard to argue
> that there is no DDL component to TRUNCATE when it effectively does an
> implicit disable-triggers operation. Another thing setting TRUNCATE
> apart from run-of-the-mill DDL operations is that it inherently violates
> MVCC rules (by deleting rows that should still be visible to concurrent
> transactions).

Kind of makes one wish you could know what tables were going to be
touched for a given transaction at the start of the transaction. That's
not really here nor there tho. I could see limiting truncate privileges
to tables which don't have on-delete triggers, that doesn't help with
the MVCC problem though and ends up being why we can't just use 'delete'
privileges for it.

Could we base vacuum and analyze rights off of other privileges though?
vacuum allowed if yoou have 'delete' privileges, analyze if you have
'insert', 'update', or 'delete'? And for 'truncate' permissions...

> But my real problem with the approach is that I don't see where it
> stops. If you're allowed to do ANALYZE, why not ALTER TABLE SET
> STATISTICS? If you're allowed to do TRUNCATE, why not the
> recently-discussed ALTER TABLE SET RELIABILITY? And how about CLUSTER?
> All of these could be pretty useful for some applications not too far
> removed from yours. And there will be someone wanting a bit for
> DISABLE/ENABLE TRIGGER coming along right afterwards. Must we implement
> a separate nonstandard privilege bit for every operation that someone
> comes up and wants a bit for, if they have the necessary cut-and-paste
> skill to submit a patch for it?

I think analyze is distinct from set statistics. SET STATISTICS, if
used improperly (perhaps by mistake or misunderstanding), could cause
serious havoc on the system as potentially very poor plans are chosen
because the statistics aren't at all correct. I don't see how running
analyze a couple times would have a detrimental effect (except for the
effort of running the analyze itself, but that's really not all that
much and if they want to DoS the box in that way there are other
things they can do). SET STATISTICS is also hopefully something you're
not having to change or do every time you add/remove/update data.

SET RELIABILITY is a more interesting question since it could be used in
a situation similar to why truncate's popular (mass data
loading/reloading). The same is true for disable/enable triggers.

> I'd feel happier about an approach that adds *one* privilege bit
> covering a range of operations that we agree to be useful. This will
> avoid chewing a disproportionate amount of ACL storage space, and it
> will force us to confront the decision about which operations are out
> as well as which are in.

If we modify VACUUM/ANALYZE to be based off what I suggested above, then
we could add just one 'BYPASS' permission bit which would allow
TRUNCATE right now and then SET RELIABILITY, and DISABLE/ENABLE TRIGGER
later. I'm not a particularly big fan of this though because, while I'd
like to be able to give TRUNCATE permissions I'm not a big fan of SET
RELIABILITY because it would affect PITR backups. I suppose a user
would still have to intentionally do that though and so they'd have only
themselves to blame if the data they loaded wasn't part of the backup.
DISABLE/ENABLE TRIGGER has a similar issue though because that could
probably be used to bypass CHECK and REFERENCES constraints, which I
wouldn't want to allow.

A BYPASS bit would be better than having to give ownership rights
though. We could also look at restructuring the ACL system to be able
to handle more permissions better. As was suggested elsewhere, one
option would be to have a seperate set of ACLs (at least internally)
such that a given set of commands/permissions would be associated
with one set or the other set (and hopefully rarely, both). These could
be divided up by either level or frequency (which I think would actually
result in the same set). Level would be row/table/database, frequency
would be estimation of usage in the real world. This would seperate
SELECT, INSERT, UPDATE, DELETE into one set of permissions, and then
REFERENCES, RULE, TRIGGER, TRUNCATE, SET RELIABILITY, DISABLE/ENABLE
TRIGGER, CLUSTER into the other set. I havn't looked into this very
deeply but I like the basic idea of it better than having a 'catch-all'
permission bit.

Looking into utils/acl.h, grant options are pretty rarely accessed, even
less so than a 'truncate' permission would be, and I could see changing
AclItem to have:

typedef struct AclItem
{
Oid ai_grantee; /* ID that this item grants privs to */
Oid ai_grantor; /* grantor of privs */
AclMode ai_privs; /* privilege bits */
AclMode aig_privs; /* grant option bits */
} AclItem;

This makes AclItem slightly larger but I don't think it would have a
detrimental affect on performance other than that, it seems unlikely
that AclItem is scanned in such a way that the structure size would
adversely affect performance much. There's cacheing issues I suppose
but an extra couple of bytes doesn't seem all that terrible.

This would give us an extra 16 bits to work with though for ACL
privileges. At the moment we're talking about adding perhaps 6, or 4 if
we do the overloading for VACUUM/ANALYZE I suggested, which means still
having 15 or 17 bits left. We could postpone the grant split till we
actually go over 16 ACL bits used but I don't think there's much need
to.

If that's more palatable then I'd be happy to work up the changes to the
ACL system to implement that.

Thanks,

Stephen

Stephen Frost wrote:
> I'm not a particularly big fan of this though because, while I'd
> like to be able to give TRUNCATE permissions I'm not a big fan of SET
> RELIABILITY because it would affect PITR backups.

As far as I have understood the discussion... with WAL archiving turned on,
the whole RELIABILITY changes would be no-ops, no?
Just as the CTAS optimization etc. only skip WAL if WAL archiving is turned
off.

Best Regards,
Michael Paesold

* Michael Paesold (mpaesold(at)gmx(dot)at) wrote:
> Stephen Frost wrote:
> >I'm not a particularly big fan of this though because, while I'd
> >like to be able to give TRUNCATE permissions I'm not a big fan of SET
> >RELIABILITY because it would affect PITR backups.
>
> As far as I have understood the discussion... with WAL archiving turned on,
> the whole RELIABILITY changes would be no-ops, no?
> Just as the CTAS optimization etc. only skip WAL if WAL archiving is turned
> off.

Oh, I thought the reliability bit would bypass WAL even with archiving
turned on (which could be fine in some cases, just not all cases :). Of
course, all of this is still up in the air somewhat. :) If it's a noop
in that case then the 'bypass' bit might be alright to have control SET
RELIABILITY. I'd rather have the flexibility to have them be seperately
grantable though.

Thanks,

Stephen

On Thu, 2006-01-05 at 09:41 -0500, Stephen Frost wrote:
> * Michael Paesold (mpaesold(at)gmx(dot)at) wrote:
> > Stephen Frost wrote:
> > >I'm not a particularly big fan of this though because, while I'd
> > >like to be able to give TRUNCATE permissions I'm not a big fan of SET
> > >RELIABILITY because it would affect PITR backups.
> >
> > As far as I have understood the discussion... with WAL archiving turned on,
> > the whole RELIABILITY changes would be no-ops, no?
> > Just as the CTAS optimization etc. only skip WAL if WAL archiving is turned
> > off.
>
> Oh, I thought the reliability bit would bypass WAL even with archiving
> turned on (which could be fine in some cases, just not all cases :). Of

It might be better if this was an setting in postgresql.conf requiring a
restart to change, off by default.

I don't like the thought of a table owner or even a super-user being
able to throw away data because they failed to investigate the full
impact of the backup strategy. I.e. Someone missed the memo that backups
were changing from pg_dumps to PITR for database environment H.

--