Re: Largeobject Access Controls (r2460)

(2010/02/08 22:23), Alvaro Herrera wrote:
> Takahiro Itagaki escribió:
>>
>> KaiGai Kohei<kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>>
>>>> default: both contents and metadata
>>>> --data-only: same
>>>> --schema-only: neither
>>>
>>> However, it means only large object performs an exceptional object class
>>> that dumps its owner, acl and comment even if --data-only is given.
>>> Is it really what you suggested, isn't it?
>>
>> I wonder we still need to have both "BLOB ITEM" and "BLOB DATA"
>> even if we will take the all-or-nothing behavior. Can we handle
>> BLOB's owner, acl, comment and data with one entry kind?
>
> I don't think this is necessarily a good idea. We might decide to treat
> both things separately in the future and it having them represented
> separately in the dump would prove useful.

I agree. From design perspective, the single section approach is more
simple than dual section, but its change set is larger than the dual.

The attached patch revised the previous implementation which have
two types of sections, to handle options correctly, as follows:

* default: both contents and metadata
* --data-only: same
* --schema-only: neither

Below is the points to be introduced.

The _tocEntryRequired() makes its decision whether the given TocEntry
to be dumped here, or not, based on the given context.
Previously, all the sections which needs cleanups and access privileges
were not belonging to SECTION_DATA, so, data sections were ignored, even
if it needs to restore cleanup code and access privileges.

At the pg_backup_archiver.c:329 chunk, it checks whether we need to clean
up the existing object specified by the TocEntry. If the entry is "BLOB
ITEM", _tocEntryRequired() returns REQ_DATA (if --data-only given), then
it does not write out the cleanup code.
(We have to unlink the existing large objects to be restored prior to
creation of them, so it got unavailable to clean up at _StartBlob().)

At the pg_backup_archiver.c:381 chunk, it checks whether we need to restore
access privileges, or not, using the given "ACL" TocEntry. In similar way,
the caller does not expect access privileges being restored when --data-only
is given.

The _tocEntryRequired() was also modified to handle large objects correctly.
Previously, when TocEntry does not have its own dumper (except for "SEQUENCE
SET" section), it was handled as a SECTION_SCHEMA.
If the 16th argument of ArchiveEntry() was NULL, it does not have its own
dumper function, even if the section is SECTION_DATA. Also, the dumpBlobItem()
calls ArchiveEntry() without its dumper, so it is misidentified as a schema
section. The "ACL" section of large objects are also misidentified.
So, I had to add these special treatments.

The dumpACL() is a utility function to write out GRANT/REVOKE statement for
the given acl string. When --data-only is given, it returns immediately
without any works. It prevented to dump access privileges of large objects.
However, all the caller already checks "if (dataOnly)" cases prior to its
invocation. So, I removed this check from the dumpACL().

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