2009/12/22 KaiGai Kohei <kai...@ak.jp.nec.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<t...@sss.pgh.pa.us> wrote: >>>> Robert Haas<robertmh...@gmail.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(!)
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. http://wiki.postgresql.org/wiki/PostgreSQL_8.5_Open_Items Thanks, ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
