Re: [PATCH] Largeobject access controls

Jaime, Thanks for your reviewing.

Jaime Casanova wrote:
> 2009/9/6 KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>:
>> The attached patch is an update of largeobject access controls.
>>
>
> it applies fine (just 3 succeded hunks), compiles and passes
> regression tests...
>
> ALTER LARGE OBJECT is working, but now that we can change the owner of
> a LO we should be able to see who the actual owner is... i mean we
> should add an owner column in \dl for psql (maybe \dl+) and maybe an
> lo_owner() function.

I would like to buy your idea at the revised patch.

> the GRANTs (and REVOKEs) work just fine too...
> Another question is what we want that only the LO owner can delete it
> (via lo_unlink)?

It is the standard manner in PostgreSQL. The native database privilege mechanism
checks ownership of the database object when a user tries to drop the object.
I don't think we have good reason that only largeobejct has its own principle.

Please note that it is different from ACL_DELETE permission, because it does not
control privilege to drop the table itself. It allows a certain user to delete
tuples within the table.

> another one, is it possible for us to have a CREATE LARGE OBJECT statement?
> the reason for this is:
> 1) it is a little ugly to use the OID in ALTER/GRANT/REVOKE
> statements, in a create statement we can assign a name to the LO
> 2) it could be more consistent with other ALTER/GRANT/REVOKE that acts
> over objects created with CREATE while large objects are created via
> lo_import() which makes obvious that are just records in meta-data
> table (hope this is understandable)

It is not difficult to implement the named-largeobejct.

However, the matter is whether it is really wanted feature to decorate
a largeobject, or not. At least, access controls on largeobjects is a
todo item, but symbolic identifier on largeobject is not the one.

BTW, we already have COMMENT ON LARGE OBJECT <loid> statement now.
How should it be handled, if a largeobject has its symbolic identifier?

>> It adds a new guc variable to turn on/off compatible behavior in
>> largeobejct access controls.
>>
>> largeobject_compat_dac = [on | off] (default: off)
>>
>> If the variable is turned on, all the new access control features
>> on largeobjects are bypassed, as if v8.4.x or prior did.
>
> the GUC works as intended
> but i don't like the name, it is not very meaningful for those of us
> that doesn't know what DAC or MAC are...

Do you think the "largeobject_compat_acl" is a meaningful name, instead?

> another point, you really have to put the GUC in the postgresql.conf
> file if you hope people know about it ;)
> it is not documented either

I'll add a description about the GUC at the document.
Is it also necessary to add postgresql.conf.sample??

> About the code...
> - I don't like the name pg_largeobject_meta why not pg_largeobject_acl
> (put here any other name you like)? or there was a reason for that
> choose?

My preference is a pair of pg_largeobject and pg_largeobject_data (which
has an identical structure to the current pg_largeobject).

However, it seems to me the pg_largeobject_acl is an incorrect name,
because it also contains the owner identifier which is a part of metadata,
but not an acl.

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>