2009/12/22 KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>:
> (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
> 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(!)
I will review this sooner if I have time, but please make sure it gets
added to the next CommitFest so we don't lose it. I think it also
needs to be added here, since AFAICS this is a must-fix for 8.5.
In response to
pgsql-hackers by date
|Next:||From: Simon Riggs||Date: 2009-12-22 17:15:42|
|Subject: Re: alpha3 release schedule?|
|Previous:||From: Heikki Linnakangas||Date: 2009-12-22 16:40:24|
|Subject: Re: alpha3 release schedule?|