Re: [Bug] Inconsistent result for inheritance and FOR UPDATE.

Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> writes:
>> (2014/12/12 10:37), Tom Lane wrote:
>>> Yeah, this is clearly a thinko: really, nothing in the planner should
>>> be using get_parse_rowmark(). I looked around for other errors of the
>>> same type and found that postgresGetForeignPlan() is also using
>>> get_parse_rowmark(). While that's harmless at the moment because we
>>> don't support foreign tables as children, it's still wrong. Will
>>> fix that too.

> While updating the inheritance patch, I noticed that the fix for
> postgresGetForeignPlan() is not right. Since PlanRowMarks for foreign
> tables get the ROW_MARK_COPY markType during preprocess_rowmarks(), so
> we can't get the locking strength from the PlanRowMarks, IIUC.

Ugh, you're right.

> In order
> to get the locking strength, I think we need to see the RowMarkClauses
> and thus still need to use get_parse_rowmark() in
> postgresGetForeignPlan(), though I agree with you that that is ugly.

I think this needs more thought; I'm still convinced that having the FDW
look at the parse rowmarks is the Wrong Thing. However, we don't need
to solve it in existing branches. With 9.4 release so close, the right
thing is to revert that change for now and consider a HEAD-only patch
later. (One idea is to go ahead and make a ROW_MARK_COPY item, but
add a field to PlanRowMark to record the original value. We should
probably also think about allowing FDWs to change these settings if
they want to. The real source of trouble here is that planner.c
has a one-size-fits-all approach to row locking for FDWs; and we're
now seeing that that one size doesn't fit postgres_fdw.)

regards, tom lane