On Thu, Nov 9, 2017 at 6:05 AM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Michael Paquier <michael.paqu...@gmail.com> writes:
>> On Tue, Sep 26, 2017 at 11:42 AM, Vaishnavi Prabakaran
>> <vaishnaviprabaka...@gmail.com> wrote:
>>> I moved the cf entry to "ready for committer", and though my vote is for
>>> keeping the existing API behavior with write implying read, I let the
>>> committer decide whether the following behavior change is Ok or not.
>>> "Reading from a large-object descriptor opened with INV_WRITE is NOT
>>> possible"
>> Thanks for the review!
> After chewing on this some more, I'm inclined to agree that we should
> not change the behavior of the read/write flags.  There's been no
> field requests for a true-write-only mode, so I think we're much more
> likely to get complaints from users whose code we broke than plaudits
> from people who think the change is helpful.

Thanks for the input. Then that's two people favoring no changes.

> I believe it would be easy enough to adjust the patch so that we can
> still have the refactoring benefits; we'd just need the bit that
> translates from external to internal flags to go more like
>         if (flags & INV_WRITE)
>                 descflags |= IFS_WRLOCK | IFS_RDLOCK;
>         if (flags & INV_READ)
>                 descflags |= IFS_RDLOCK;
> (Preferably with a comment about why it's like this.)

Sure, I have updated the patch with this comment:
+   /*
+    * Historically, no difference is made between (INV_WRITE) and
+    * (INV_WRITE | INV_READ), the caller being allowed to read the large
+    * object descriptor in either case.
+    */
Of course please feel free to reword if you find something better-suited.

> Another idea would be to invent a new external flag bit "INV_WRITE_ONLY",
> so that people who wanted true write-only could get it, without breaking
> backwards-compatible behavior.  But I'm inclined to wait for some field
> demand to show up before adding even that little bit of complication.

Demand that may never show up, and the current behavior on HEAD does
not receive any complains either. I am keeping the patch simple for
now. That's less aspirin needed for everybody.

Attachment: 0003-Move-ACL-checks-for-large-objects-when-opening-them.patch
Description: Binary data

Attachment: 0001-Remove-ALLOW_DANGEROUS_LO_FUNCTIONS-for-LO-related-s.patch
Description: Binary data

Attachment: 0002-Replace-superuser-checks-of-large-object-import-expo.patch
Description: Binary data

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to