* Dean Rasheed (dean.a.rash...@gmail.com) wrote:
> On 26 February 2015 at 09:50, Dean Rasheed wrote:
> > On 26 February 2015 at 05:43, Stephen Frost wrote:
> >> I wonder if there are some documentation updates which need to be done
> >> for this also? I'm planning to look as I vauguely recall me
On 26 February 2015 at 09:50, Dean Rasheed wrote:
> On 26 February 2015 at 05:43, Stephen Frost wrote:
>> I wonder if there are some documentation updates which need to be done
>> for this also? I'm planning to look as I vauguely recall mentioning the
>> ordering of operations somewhere along th
On 26 February 2015 at 09:27, Dean Rasheed wrote:
> I think this should probably be committed as 2 separate patches...
On closer inspection, the 2 parts are interrelated since the new
self-join test that tests the inheritance planner changes also
requires the rewriter changes, otherwise it falls
On 26 February 2015 at 05:43, Stephen Frost wrote:
> Dean,
>
> * Dean Rasheed (dean.a.rash...@gmail.com) wrote:
>> Attached is a patch to make RLS checks run before attempting to
>> insert/update any data rather than afterwards.
>
> Excellent, this I really like and it's a pretty straight-forward
On 26 February 2015 at 05:41, Stephen Frost wrote:
> Dean,
>
> * Dean Rasheed (dean.a.rash...@gmail.com) wrote:
>> Here's an updated patch with a new test for this bug. I've been
>> developing the fixes for these RLS issues as one big patch, but I
>> suppose it would be easy to split up, if that's
Dean,
* Dean Rasheed (dean.a.rash...@gmail.com) wrote:
> Attached is a patch to make RLS checks run before attempting to
> insert/update any data rather than afterwards.
Excellent, this I really like and it's a pretty straight-forward change.
I wonder if there are some documentation updates which
Dean,
* Dean Rasheed (dean.a.rash...@gmail.com) wrote:
> Here's an updated patch with a new test for this bug. I've been
> developing the fixes for these RLS issues as one big patch, but I
> suppose it would be easy to split up, if that's preferred.
Thanks for working on all of this!
I've brough
Dean,
* Dean Rasheed (dean.a.rash...@gmail.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 ha
On 10 January 2015 at 15:12, Stephen Frost wrote:
> * Dean Rasheed (dean.a.rash...@gmail.com) wrote:
>> Currently we're applying RLS CHECKs after the INSERT or UPDATE, like
>> WITH CHECK OPTIONs on views. The SQL spec says that WITH CHECK OPTIONs
>> on views have to be applied after the INSERT/UPD
* Dean Rasheed (dean.a.rash...@gmail.com) wrote:
> Turns out it wasn't as simple as that. prepend_row_security_policies()
> really could get called multiple times for the same RTE, because the
> call to query_tree_walker() at the end of fireRIRrules() would descend
> into the just-added quals again
On 14 January 2015 at 08:43, Dean Rasheed wrote:
> On 12 January 2015 at 14:24, Stephen Frost wrote:
>> Interesting, thanks for the work! I had been suspicious that there was
>> an issue with the recursion handling.
>>
>
> So continuing to review the RLS code, I spotted the following in
> prepen
On 12 January 2015 at 14:24, Stephen Frost wrote:
> Interesting, thanks for the work! I had been suspicious that there was
> an issue with the recursion handling.
>
So continuing to review the RLS code, I spotted the following in
prepend_row_security_policies():
/*
* We may end up gett
On 12 January 2015 at 14:24, Stephen Frost wrote:
> Looking at the regression tests a bit though, I notice that this removes
> the lower-level LockRows.. There had been much discussion about that
> last spring which I believe you were a part of..? I specifically recall
> discussing it with Craig
Dean,
* Dean Rasheed (dean.a.rash...@gmail.com) wrote:
> On 10 January 2015 at 21:38, Dean Rasheed wrote:
> > OK, I'll take a look at it. I started reading the existing code that
> > expands RLS policies and spotted a couple of bugs that should be fixed
> > first:-
> >
> > 1). In prepend_row_secu
On 10 January 2015 at 21:38, Dean Rasheed wrote:
> OK, I'll take a look at it. I started reading the existing code that
> expands RLS policies and spotted a couple of bugs that should be fixed
> first:-
>
> 1). In prepend_row_security_policies(), defaultDeny should be
> initialised to false at the
On 10 January 2015 at 15:12, Stephen Frost wrote:
>> NOTE: If we do change RLS CHECKs to be executed prior to modifying any
>> data, that's potentially a change that could be made independently of
>> the UPSERT patch. We should also probably then stop referring to them
>> as WITH CHECK OPTIONs in
Dean,
* Dean Rasheed (dean.a.rash...@gmail.com) wrote:
> On 9 January 2015 at 20:26, Stephen Frost wrote:
> > Where this leaves me, at least, is feeling like we should always apply
> > the INSERT WITH CHECK policy, then if there is a conflict, check the
> > UPDATE USING policy and throw an error
On 9 January 2015 at 20:26, Stephen Frost wrote:
> Where this leaves me, at least, is feeling like we should always apply
> the INSERT WITH CHECK policy, then if there is a conflict, check the
> UPDATE USING policy and throw an error if the row isn't visible but
> otherwise perform the UPDATE and
* Peter Geoghegan (p...@heroku.com) wrote:
> On Fri, Jan 9, 2015 at 1:09 PM, Stephen Frost wrote:
> > Therefore,
> > I'm not sure that I see the point in checking the INSERT tuple against
> > the UPDATE policy.
>
> I guess it wouldn't be hard to modify the struct WithCheckOption to
> store the cm
On Fri, Jan 9, 2015 at 1:09 PM, Stephen Frost wrote:
> To flip it around a bit, I don't think we can avoid checking the
> *resulting* tuple from the UPDATE against the UPDATE policy.
We can avoid it - by not updating. What I'm suggesting is that an
enforcement occurs that is more or less equivale
* Peter Geoghegan (p...@heroku.com) wrote:
> On Fri, Jan 9, 2015 at 12:26 PM, Stephen Frost wrote:
> > Where this leaves me, at least, is feeling like we should always apply
> > the INSERT WITH CHECK policy, then if there is a conflict, check the
> > UPDATE USING policy and throw an error if the r
On Fri, Jan 9, 2015 at 12:53 PM, Stephen Frost wrote:
> I'm not sure how that would work exactly though, since the tuple the
> UPDATE results in might be different from what the INSERT has, as Dean
> pointed out. The INSERT tuple might even pass the UPDATE policy where
> the resulting tuple from
On Fri, Jan 9, 2015 at 12:26 PM, Stephen Frost wrote:
> Where this leaves me, at least, is feeling like we should always apply
> the INSERT WITH CHECK policy, then if there is a conflict, check the
> UPDATE USING policy and throw an error if the row isn't visible but
> otherwise perform the UPDATE
Peter,
* Peter Geoghegan (p...@heroku.com) wrote:
> On Fri, Jan 9, 2015 at 2:22 AM, Dean Rasheed wrote:
> > Whoa, hang on. I think you're being a bit quick to dismiss that
> > example. Why shouldn't I want an upsert where the majority of the
> > table columns follow the usual "make it so" pattern
On Fri, Jan 9, 2015 at 2:22 AM, Dean Rasheed wrote:
> Whoa, hang on. I think you're being a bit quick to dismiss that
> example. Why shouldn't I want an upsert where the majority of the
> table columns follow the usual "make it so" pattern of an upsert, but
> there is also this kind of audit colum
Dean, Peter,
* Dean Rasheed (dean.a.rash...@gmail.com) wrote:
> On 9 January 2015 at 08:49, Peter Geoghegan wrote:
> > On Fri, Jan 9, 2015 at 12:19 AM, Dean Rasheed
> > wrote:
> >> I was trying to think up an example where you might actually have
> >> different INSERT and UPDATE policies, and t
On 9 January 2015 at 08:49, Peter Geoghegan wrote:
> On Fri, Jan 9, 2015 at 12:19 AM, Dean Rasheed
> wrote:
>> I was trying to think up an example where you might actually have
>> different INSERT and UPDATE policies, and the best I can think of is
>> some sort of mod_count column where you have
On Fri, Jan 9, 2015 at 12:19 AM, Dean Rasheed wrote:
> I was trying to think up an example where you might actually have
> different INSERT and UPDATE policies, and the best I can think of is
> some sort of mod_count column where you have an INSERT CHECK
> (mod_count = 0) and an UPDATE CHECK (mod_
On 9 January 2015 at 00:49, Stephen Frost wrote:
> Peter,
>
> * Peter Geoghegan (p...@heroku.com) wrote:
>> For column level privileges, you wouldn't expect to only get an error
>> about not having the relevant update permissions at runtime, when the
>> update path happens to be taken. And so it i
Peter,
* Peter Geoghegan (p...@heroku.com) wrote:
> For column level privileges, you wouldn't expect to only get an error
> about not having the relevant update permissions at runtime, when the
> update path happens to be taken. And so it is for RLS.
Right, that's the precedent we should be consi
On Wed, Jan 7, 2015 at 5:33 PM, David Fetter wrote:
> Same database, same constraints, different outcome.
You're missing the point, I think. UPSERT means that the user doesn't
care about whether or not a given tuple proposed for insertion was
handled with an insert or an update. If you have disti
On Wed, Jan 07, 2015 at 05:18:58PM -0800, Peter Geoghegan wrote:
> On Wed, Jan 7, 2015 at 5:16 PM, David Fetter wrote:
> > There's precedent. Unique constraints, for example.
>
> I don't see that as any kind of precedent.
In the part you sliced off, Stephen described a situation where the
conte
On Wed, Jan 7, 2015 at 5:16 PM, David Fetter wrote:
> There's precedent. Unique constraints, for example.
I don't see that as any kind of precedent.
--
Peter Geoghegan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.post
On Wed, Jan 07, 2015 at 03:01:20PM -0500, Stephen Frost wrote:
> * Robert Haas (robertmh...@gmail.com) wrote:
> > On Wed, Jan 7, 2015 at 4:04 AM, Dean Rasheed
> > wrote:
> > > I think the policies applied should depend on the path taken, so if it
> > > does an INSERT, then only the INSERT CHECK p
On Wed, Jan 7, 2015 at 12:26 PM, Stephen Frost wrote:
> I agree with that up to a point- this case feels more like a POLA
> violation though rather than a run-of-the-mill "you need to test all
> that." I'm a bit worried this might lead to cases where users can't use
> UPSERT because they don't kn
On Wed, Jan 7, 2015 at 12:01 PM, Stephen Frost wrote:
> Other databases have this capability and have triggers and at least one
> ends up firing both INSERT and UPDATE triggers, with many complaints
> from users about how that ends up making the performance suck. Perhaps
> we could use that as a
* Robert Haas (robertmh...@gmail.com) wrote:
> On Wed, Jan 7, 2015 at 3:01 PM, Stephen Frost wrote:
> > * Robert Haas (robertmh...@gmail.com) wrote:
> >> On Wed, Jan 7, 2015 at 4:04 AM, Dean Rasheed
> >> wrote:
> >> > I think the policies applied should depend on the path taken, so if it
> >> >
On Wed, Jan 7, 2015 at 3:01 PM, Stephen Frost wrote:
> * Robert Haas (robertmh...@gmail.com) wrote:
>> On Wed, Jan 7, 2015 at 4:04 AM, Dean Rasheed
>> wrote:
>> > I think the policies applied should depend on the path taken, so if it
>> > does an INSERT, then only the INSERT CHECK policy should
* Robert Haas (robertmh...@gmail.com) wrote:
> On Wed, Jan 7, 2015 at 4:04 AM, Dean Rasheed wrote:
> > I think the policies applied should depend on the path taken, so if it
> > does an INSERT, then only the INSERT CHECK policy should be applied
> > (after the insert), but if it ends up doing an U
* Dean Rasheed (dean.a.rash...@gmail.com) wrote:
> On 6 January 2015 at 20:44, Peter Geoghegan wrote:
> > Another issue that I see is that there is only one resultRelation
> > between the INSERT and UPDATE. That means that without some additional
> > special measure, both UPDATE and INSERT "WITH C
On Wed, Jan 7, 2015 at 4:04 AM, Dean Rasheed wrote:
> I think the policies applied should depend on the path taken, so if it
> does an INSERT, then only the INSERT CHECK policy should be applied
> (after the insert), but if it ends up doing an UPDATE, I would expect
> the UPDATE USING policy to be
On 6 January 2015 at 20:44, Peter Geoghegan wrote:
> Another issue that I see is that there is only one resultRelation
> between the INSERT and UPDATE. That means that without some additional
> special measure, both UPDATE and INSERT "WITH CHECK ( ... ) " options
> are applied regardless of whethe
Another issue that I see is that there is only one resultRelation
between the INSERT and UPDATE. That means that without some additional
special measure, both UPDATE and INSERT "WITH CHECK ( ... ) " options
are applied regardless of whether the INSERT path was taken, or the
UPDATE path. Maybe that'
On Tue, Jan 6, 2015 at 9:39 AM, Robert Haas wrote:
> I think the INSERT .. ON CONFLICT UPDATE shouldn't be able to attempt
> an update unless the UPDATE policies of the table are such that a
> regular UPDATE would find the affected row. The post-image of the row
> needs to satisfy any UPDATE CHEC
On Mon, Jan 5, 2015 at 12:54 PM, Peter Geoghegan wrote:
> The patch that implements INSERT ... ON CONFLICT UPDATE has support
> and tests for per-column privileges (which are not relevant to the
> IGNORE variant, AFAICT). However, RLS support is another thing
> entirely. It has not been properly t
The patch that implements INSERT ... ON CONFLICT UPDATE has support
and tests for per-column privileges (which are not relevant to the
IGNORE variant, AFAICT). However, RLS support is another thing
entirely. It has not been properly thought out, and unlike per-column
privileges requires careful con
46 matches
Mail list logo
