On Fri, Apr 14, 2017 at 7:41 AM, Rod Taylor <rod.tay...@gmail.com> wrote:
> > > > On Thu, Apr 13, 2017 at 5:31 PM, Stephen Frost <sfr...@snowman.net> wrote: > >> Rod, all, >> >> * Joe Conway (m...@joeconway.com) wrote: >> > On 04/13/2017 01:31 PM, Stephen Frost wrote: >> > > * Robert Haas (robertmh...@gmail.com) wrote: >> > >> On Thu, Apr 6, 2017 at 4:05 PM, Rod Taylor <rod.tay...@gmail.com> >> wrote: >> > >> > I'm a little confused on why a SELECT policy fires against the NEW >> record >> > >> > for an UPDATE when using multiple FOR policies. The ALL policy >> doesn't seem >> > >> > to have that restriction. >> > >> >> > >> My guess is that you have found a bug. >> > > >> > > Indeed. Joe's been looking into it and I'm hoping to find some time >> to >> > > dig into it shortly. >> > >> > >> CREATE POLICY split_select ON t FOR SELECT TO split >> > >> USING (value > 0); >> > >> CREATE POLICY split_update ON t FOR UPDATE TO split >> > >> USING (true) WITH CHECK (true); >> > >> > Yes -- from what I can see in gdb: >> >> Actually, looking at this again, the complaint appears to be that you >> can't "give away" records. That was a topic of much discussion and I'm >> reasonably sure that was what we ended up deciding made the most sense. >> You have to be able to see records to be able to update them (you can't >> update records you can't see), and you have to be able to see the result >> of your update. I don't doubt that we could improve the documentation >> around this (and apparently the code comments, according to Joe..). >> >> > Then there is a bug in the simpler statement which happily lets you give > away records. > > CREATE POLICY simple_all ON t TO simple USING (value > 0) WITH CHECK > (true); > > SET session authorization simple; > SELECT * FROM t; > UPDATE t SET value = value * -1 WHERE value = 1; > -- No error and value is -1 at the end. > My actual use-case involves a range. Most users can see and manipulate the record when CURRENT_TIMESTAMP is within active_period. Some users (staff/account admins) can see recently dead records too. And a 3rd group (senior staff) have no time restriction, though there are a few customers they cannot see due to their information being a touch more sensitive. I've simplified the below rules to just deal with active_period and the majority of user view (@> CURRENT_TIMESTAMP). NOTE: the active_period range is '[)' by default, so records with upper() = CURRENT_TIMESTAMP are not visible with @> CURRENT_TIMESTAMP restriction. CREATE A TABLE t (id integer, active_period tstzrange NOT NULL DEFAULT tstzrange(current_timestamp, NULL)); The below policy is allowed but requires that 1ms slop to accommodate the wi Updated record invisible to USING but requires a trigger to enforce specific upper and starting values. I have a trigger enforcing specific upper/lower values for the range for specific ROLEs. So I had the thought that I might move ROLE specific trigger logic into the RLS mechanism. CREATE POLICY hide_old ON t TO s; USING ( active_period @> CURRENT_TIMESTAMP) WITH CHECK ( active_period && tstzrange(current_timestamp - interval '0.001 seconds', current_timestamp, '[]')); -- This is effectively a delete for the above policy. It becomes immediately invisible due to USING restriction. UPDATE t SET active_period = tstzrange(lower(active_period), CURRENT_TIMESTAMP); SELECT count(*) FROM t; -- 0 records I tried to tighten the above rules, so INSERT must have upper of NULL and UPDATE must set upper to exactly CURRENT_TIMESTAMP. Clearly I can achieve this using triggers for enforcement but I tried to abuse RLS instead because it is a role specific restriction. I was surprised when hide_old_select->USING was preventing the UPDATE when the simple single policy version let it through. CREATE POLICY hide_old_select ON t FOR SELECT TO s USING ( active_period @> CURRENT_TIMESTAMP); CREATE POLICY hide_old_insert ON t FOR INSERT to s WITH CHECK ( lower(active_period) = CURRENT_TIMESTAMP AND upper(active_period) IS NULL); CREATE POLICY hide_old_update ON t FOR UPDATE TO s USING ( active_period @> CURRENT_TIMESTAMP) WITH CHECK ( upper(active_period) = CURRENT_TIMESTAMP); -- Disallowed due to hide_old_select policy. UPDATE t SET active_period = tstzrange(lower(active_period), CURRENT_TIMESTAMP); I'm happy to help with testing and documentation but first I need to understand what the intended functionality was. Right now it seems inconsistent between the simple single policy version and the multi policy version; the docs imply the single policy version is correct (it only seems to mention SELECT checks on RETURNING clauses). -- Rod Taylor