Re: Largeobject Access Controls (r2460)

(2009/12/21 9:39), KaiGai Kohei wrote:
> (2009/12/19 12:05), Robert Haas wrote:
>> On Fri, Dec 18, 2009 at 9:48 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Robert Haas<robertmhaas(at)gmail(dot)com> writes:
>>>> Oh. This is more complicated than it appeared on the surface. It
>>>> seems that the string "BLOB COMMENTS" actually gets inserted into
>>>> custom dumps somewhere, so I'm not sure whether we can just change it.
>>>> Was this issue discussed at some point before this was committed?
>>>> Changing it would seem to require inserting some backward
>>>> compatibility code here. Another option would be to add a separate
>>>> section for "BLOB METADATA", and leave "BLOB COMMENTS" alone. Can
>>>> anyone comment on what the Right Thing To Do is here?
>>>
>>> The BLOB COMMENTS label is, or was, correct for what it contained.
>>> If this patch has usurped it to contain other things
>>
>> It has.
>>
>>> I would argue
>>> that that is seriously wrong. pg_dump already has a clear notion
>>> of how to handle ACLs for objects. ACLs for blobs ought to be
>>> made to fit into that structure, not dumped in some random place
>>> because that saved a few lines of code.
>>
>> OK. Hopefully KaiGai or Takahiro can suggest a fix.

The patch has grown larger than I expected before, because the way
to handle large objects are far from any other object classes.

Here are three points:

1) The new BLOB ACLS section was added.

It is a single purpose section to describe GRANT/REVOKE statements
on large objects, and BLOB COMMENTS section was reverted to put
only descriptions.

Because we need to assume a case when the database holds massive
number of large objects, it is not reasonable to store them using
dumpACL(). It chains an ACL entry with the list of TOC entries,
then, these are dumped. It means pg_dump may have to register massive
number of large objects in the local memory space.

Currently, we also store GRANT/REVOKE statements in BLOB COMMENTS
section, but confusable. Even if pg_restore is launched with
--no-privileges options, it cannot ignore GRANT/REVOKE statements
on large objects. This fix enables to distinguish ACLs on large
objects from other properties, and to handle them correctly.

2) The BLOBS section was separated for each database users.

Currently, the BLOBS section does not have information about owner
of the large objects to be restored. So, we tried to alter its
ownership in the BLOB COMMENTS section, but incorrect.

The --use-set-session-authorization option requires to restore
ownership of objects without ALTER ... OWNER TO statements.
So, we need to set up correct database username on the section
properties.

This patch renamed the hasBlobs() by getBlobs(), and changed its
purpose. It registers DO_BLOBS, DO_BLOB_COMMENTS and DO_BLOB_ACLS
for each large objects owners, if necessary.
For example, if here are five users owning large objects, getBlobs()
shall register five TOC entries for each users, and dumpBlobs(),
dumpBlobComments() and dumpBlobAcls() shall be also invoked five
times with the username.

3) _LoadBlobs()

For regular database object classes, _printTocEntry() can inject
"ALTER xxx OWNER TO ..." statement on the restored object based on
the ownership described in the section header.
However, we cannot use this infrastructure for large objects as-is,
because one BLOBS section can restore multiple large objects.

_LoadBlobs() is a routine to restore large objects within a certain
section. This patch modifies this routine to inject "ALTER LARGE
OBJECT <loid> OWNER TO <user>" statement for each large objects
based on the ownership of the section.
(if --use-set-session-authorization is not given.)

$ diffstat pgsql-fix-pg_dump-blob-privs.patch
pg_backup_archiver.c | 4
pg_backup_custom.c | 11 !
pg_backup_files.c | 9 !
pg_backup_tar.c | 9 !
pg_dump.c | 312 +++++++----!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
pg_dump.h | 9 !
pg_dump_sort.c | 8 !
7 files changed, 68 insertions(+), 25 deletions(-), 269 modifications(!)

Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>