Re: patch: Add a separate TRUNCATE permission

Here's a patch implementing the TODO item "Add a separate TRUNCATE
permission". Hopefully I found all the bits that needed to be
modified to make this work.

Any feedback appreciated.

Thanks,

...Robert

Robert Haas napsal(a):
> Here's a patch implementing the TODO item "Add a separate TRUNCATE
> permission". Hopefully I found all the bits that needed to be
> modified to make this work.
>
> Any feedback appreciated.
>

Added to the next commit fest patch list.
http://wiki.postgresql.org/wiki/CommitFest:2008-09

Zdenek

Am Sunday, 27. July 2008 schrieb Robert Haas:
> Here's a patch implementing the TODO item "Add a separate TRUNCATE
> permission". Hopefully I found all the bits that needed to be
> modified to make this work.

Looks very good to me.

* Peter Eisentraut (peter_e(at)gmx(dot)net) wrote:
> Am Sunday, 27. July 2008 schrieb Robert Haas:
> > Here's a patch implementing the TODO item "Add a separate TRUNCATE
> > permission". Hopefully I found all the bits that needed to be
> > modified to make this work.
>
> Looks very good to me.

We've been through this before, I believe.. The concern is that it
chews up another bit in the acl structure, leaving not a huge number
left. My suggested approach to fixing this was to split the "grantable"
bits up from the regular usage bits. That's, unfortunately, a
non-trivial amount of work, however.

Perhaps just having a plan for how we would increase the number of
available bits would be enough to go ahead and add this option though?
I'd certainly like to see a truncate permission, I wrote a patch for it
myself back in January of 2006. That thread starts here:

http://archives.postgresql.org/pgsql-patches/2006-01/msg00028.php

I think someone else submitted a patch for it last year too, actually.

Thanks,

Stephen

Am Tuesday, 29. July 2008 schrieb Stephen Frost:
> I'd certainly like to see a truncate permission, I wrote a patch for it
> myself back in January of 2006.  That thread starts here:
>
> http://archives.postgresql.org/pgsql-patches/2006-01/msg00028.php
>
> I think someone else submitted a patch for it last year too, actually.

Well, that certainly indicates some demand.

I think we should worry about adding more bits when we need them. It's
certainly possible to add more bits, so it is not like we need to save the
ones we have forever.

> We've been through this before, I believe.. The concern is that it
> chews up another bit in the acl structure, leaving not a huge number
> left. My suggested approach to fixing this was to split the "grantable"
> bits up from the regular usage bits. That's, unfortunately, a
> non-trivial amount of work, however.

Writing this patch was more of a "learn the PostgreSQL source" project
for me than a feature that I, personally, have a need for, so I have
no dog in this race other than that, if the feature was actually not
wanted, then it shouldn't be on the TODO list, possibly causing people
to waste time implementing it. :-)

The question of using up all the bits seems purely speculative to me
at this point. I agree that we don't want to fritter them away, but
this is the only TODO item proposes using any of those bits. Tom's
complaint about your patch seems to have been that it uses three of
the five remaining ACL bits; this patch uses only one, and arguably
TRUNCATE is more like a DML command than a utility command (which, as
Tom pointed out, there are certainly too many of to ever allocate a
bit for each one).

I would argue that if we're ever going to start adding permissions for
things like those types of utility command then we ought to create
some separate mechanism for storing permissions that are not likely to
need to be checked very frequently. Then things like INSERT and
UPDATE that happen often can benefit from a 16-bit field, and things
like ANALYZE and ADD COLUMN that are only executed occasionally can
use a separate, more heavy-weight mechanism.

In any event, however we ultimately decide to implement it, we don't
need to solve this problem now.

> I think someone else submitted a patch for it last year too, actually.

I talked about submitting one last year but didn't actually do it
since it seemed to be the wrong point in the development cycle.

...Robert

* Peter Eisentraut (peter_e(at)gmx(dot)net) wrote:
> Am Tuesday, 29. July 2008 schrieb Stephen Frost:
> > I'd certainly like to see a truncate permission, I wrote a patch for it
> > myself back in January of 2006.  That thread starts here:
> >
> > http://archives.postgresql.org/pgsql-patches/2006-01/msg00028.php
> >
> > I think someone else submitted a patch for it last year too, actually.
>
> Well, that certainly indicates some demand.
>
> I think we should worry about adding more bits when we need them. It's
> certainly possible to add more bits, so it is not like we need to save the
> ones we have forever.

I would agree with this. Others?

Stephen

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> > We've been through this before, I believe.. The concern is that it
> > chews up another bit in the acl structure, leaving not a huge number
> > left. My suggested approach to fixing this was to split the "grantable"
> > bits up from the regular usage bits. That's, unfortunately, a
> > non-trivial amount of work, however.
>
> Writing this patch was more of a "learn the PostgreSQL source" project
> for me than a feature that I, personally, have a need for, so I have
> no dog in this race other than that, if the feature was actually not
> wanted, then it shouldn't be on the TODO list, possibly causing people
> to waste time implementing it. :-)

Oh, it's wanted, I've been asking after it for 3 years. The problem is
just in getting people to agree to address it in this way, or if not to
accept some other way to address it (in which case the TODO should be
updated to reflect that).

> The question of using up all the bits seems purely speculative to me
> at this point. I agree that we don't want to fritter them away, but
> this is the only TODO item proposes using any of those bits. Tom's
> complaint about your patch seems to have been that it uses three of
> the five remaining ACL bits; this patch uses only one, and arguably
> TRUNCATE is more like a DML command than a utility command (which, as
> Tom pointed out, there are certainly too many of to ever allocate a
> bit for each one).

With the advent of autovacuum, my additional permissions for vacuum and
analyze are much less necessary. I'm fine with just adding a permission
for truncate.

> I would argue that if we're ever going to start adding permissions for
> things like those types of utility command then we ought to create
> some separate mechanism for storing permissions that are not likely to
> need to be checked very frequently. Then things like INSERT and
> UPDATE that happen often can benefit from a 16-bit field, and things
> like ANALYZE and ADD COLUMN that are only executed occasionally can
> use a separate, more heavy-weight mechanism.

That's along the same lines as I was proposing, except I would put all
of the "grantable" options in the "not often needed" column, which would
give us a full 32-bit field for "common" permissions.

> In any event, however we ultimately decide to implement it, we don't
> need to solve this problem now.

Agreed.

> > I think someone else submitted a patch for it last year too, actually.
>
> I talked about submitting one last year but didn't actually do it
> since it seemed to be the wrong point in the development cycle.

That might have been it, not sure.

Thanks!

Stephen

"Robert Haas" <robertmhaas(at)gmail(dot)com> writes:
> The question of using up all the bits seems purely speculative to me
> at this point. I agree that we don't want to fritter them away, but
> this is the only TODO item proposes using any of those bits. Tom's
> complaint about your patch seems to have been that it uses three of
> the five remaining ACL bits;

Yeah, exactly, and it also started us down the path of wanting a
separate permission bit for every DDL command. I don't have a
problem with the idea of just eating one bit for TRUNCATE. That
would leave us with four free out of sixteen, which hardly seems
like the usage level at which to start sounding alarm bells.

I believe it would be easy and cheap to adjust the representation
of ACLs to support 32 permissions instead of 16; so I won't cry
if we someday push past 16. Beyond that, though, things get very
much more expensive and complicated (as per the speculations in
this thread). So what I was really resisting was the notion of
"permission per DDL command" --- I don't want to go that way.

regards, tom lane

On Tue, 2008-07-29 at 09:03 -0400, Stephen Frost wrote:
> * Peter Eisentraut (peter_e(at)gmx(dot)net) wrote:
> > Am Tuesday, 29. July 2008 schrieb Stephen Frost:
> > > I'd certainly like to see a truncate permission, I wrote a patch for it
> > > myself back in January of 2006. That thread starts here:
> > >
> > > http://archives.postgresql.org/pgsql-patches/2006-01/msg00028.php
> > >
> > > I think someone else submitted a patch for it last year too, actually.

It was raised in January and rejected again then. Glad to see it raised
again here. I believe Tom's previous concerns about allowing truncate
permissions were related to the potentially increased number of truncate
commands this would cause and the need to tune invalidation messages.
That's done now.

> > Well, that certainly indicates some demand.
> >
> > I think we should worry about adding more bits when we need them. It's
> > certainly possible to add more bits, so it is not like we need to save the
> > ones we have forever.
>
> I would agree with this. Others?

I've no problem with running out of bits. At this rate, we have enough
some for some years yet. But I don't see too many additional permissions
coming along anyhow.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support