Re: Simplify ACL handling for large objects and removal of superuser() checks

On Thu, Nov 9, 2017 at 6:05 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
>> On Tue, Sep 26, 2017 at 11:42 AM, Vaishnavi Prabakaran
>> <vaishnaviprabakaran(at)gmail(dot)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