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. -- Michael
0003-Move-ACL-checks-for-large-objects-when-opening-them.patch
Description: Binary data
0001-Remove-ALLOW_DANGEROUS_LO_FUNCTIONS-for-LO-related-s.patch
Description: Binary data
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: http://www.postgresql.org/mailpref/pgsql-hackers