Re: INSERT ... ON CONFLICT UPDATE and RLS

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
Cc: Peter Geoghegan <pg(at)heroku(dot)com>, David Fetter <david(at)fetter(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: INSERT ... ON CONFLICT UPDATE and RLS
Date: 2015-01-20 02:56:14
Message-ID: 20150120025614.GX3062@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Dean,

* Dean Rasheed (dean(dot)a(dot)rasheed(at)gmail(dot)com) wrote:
> Attached is a patch to make RLS checks run before attempting to
> insert/update any data rather than afterwards.

Excellent, many thanks!

> In the end I decided not to create a new structure for RLS checks
> because most of the code that handles them treats them the same as
> WCOs. Instead, I just added a new 'kind' enum field to the existing
> structure and renamed/reworded things a bit.

Makes sense to me. I like being able to easily differentiate the two
from each other now while also not needing to duplicate a bunch of code.
I also like the reworded error messages.

I am wondering if we should, perhaps, rename ri_WithCheckOptions or
ExecWithCheckOptions() (or even more..) to indicate that they're used
for both view-based WITH CHECK options and for RLS.

We'll need to update this patch once the fixes for the WITH CHECK leaks
go in, of course.

> The patch also changes the error message for a RLS check violation, to
> make the cause of the error clearer. One thing I'm not sure about is
> what sqlstate code to use for this error, but I don't think that using
> WITH_CHECK_OPTION_VIOLATION is appropriate, because that seems to be
> specifically intended for views.

Hmm, agreed. Any thoughts on what to use? We could fall back on
something like ERRCODE_INSUFFICIENT_PRIVILEGE if necessary, I suppose.

Thanks!

Stephen

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Etsuro Fujita 2015-01-20 03:00:43 Re: Another comment typo in src/backend/executor/execMain.c
Previous Message Jim Nasby 2015-01-20 02:33:24 Re: Reducing buildfarm disk usage: remove temp installs when done