On Fri, Dec 18, 2009 at 9:00 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> 2009/12/18 KaiGai Kohei <kaigai@ak.jp.nec.com>:
>> (2009/12/18 15:48), Takahiro Itagaki wrote:
>>>
>>> Robert Haas<robertmhaas@gmail.com> wrote:
>>>
>>>> In both cases, I'm lost. Help?
>>>
>>> They might be contrasted with the comments for myLargeObjectExists.
>>> Since we use MVCC visibility in loread(), metadata for large object
>>> also should be visible in MVCC rule.
>>>
>>> If I understand them, they say:
>>> * pg_largeobject_aclmask_snapshot requires a snapshot which will be
>>> used in loread().
>>> * Don't use LargeObjectExists if you need MVCC visibility.
>>
>> Yes, correct.
>>
>>>> In acldefault(), there is this comment:
>>>> /* Grant SELECT,UPDATE by default, for now */
>>>> This doesn't seem to match what the code is doing, so I think we
>>>> should remove it.
>>>
>>> Ah, ACL_NO_RIGHTS is the default.
>>
>> Oops, it reflects very early phase design, but fixed later.
>>
>>>> I also notice that dumpBlobComments() is now misnamed, but it seems
>>>> we've chosen to add a comment mentioning that fact rather than fixing it.
>>>
>>> Hmmm, now it dumps not only comments but also ownership of large objects.
>>> Should we rename it dumpBlobMetadata() or so?
>>
>> It seems to me quite natural.
>>
>> The attached patch fixes them.
>
> I think we might want to go with dumpBlobProperties(), because
> dumpBlobMetadata() might lead you to think that all of the properties
> being dumped are stored in pg_largeobject_metadata, which is not the
> case.
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
thiswas 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?
...Robert
