On 2015/02/18 7:44, Stephen Frost wrote:
> * Etsuro Fujita (fujita.etsuro@lab.ntt.co.jp) wrote:
>> On 2015/02/11 4:06, Stephen Frost wrote:
>>> I had been trying to work out an FDW-specific way to address this, but I
>>> think Dean's right that this should be addressed in
>>> expand_security_qual(), which means it'll apply to all cases and not
>>> just these FDW calls. I don't think that's actually an issue though and
>>> it'll match up to how SELECT FOR UPDATE is handled today.
>>
>> Sorry, my explanation was not accurate, but I also agree with Dean's
>> idea. In the above, I just wanted to make it clear that such a lock
>> request done by expand_security_qual() should be limited to the case
>> where the relation that is a former result relation is a foreign
>> table.
>
> Attached is a patch which should address this. Would love your (or
> anyone else's) feedback on it. It appears to address the issue which
> you raised and the regression test changes are all in-line with
> inserting a LockRows into the subquery, as anticipated.
I've looked into the patch.
* The patch applies to the latest head, 'make' passes successfully, but
'make check' fails in the rowsecurity test.
* I found one place in expand_security_qual that I'm concerned about:
+ if (targetRelation)
+ applyLockingClause(subquery, 1, LCS_FORUPDATE,
+ false, false);
ISTM that it'd be better to use LockWaitBlock as the fourth argument of
applyLockingClause.
Other than that, the patch looks good to me.
Thanks for the work!
Best regards,
Etsuro Fujita