|From:||Michael Paquier <michael(dot)paquier(at)gmail(dot)com>|
|To:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>|
|Cc:||Vaishnavi Prabakaran <vaishnaviprabakaran(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: Simplify ACL handling for large objects and removal of superuser() checks|
|Views:||Raw Message | Whole Thread | Download mbox|
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
>> 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.
|Next Message||Michael Paquier||2017-11-09 02:09:20||Re: [PATCH] A hook for session start|
|Previous Message||Amit Langote||2017-11-09 01:02:03||Re: [PATCH] fix wrong create table statement in documentation|