• Tom Lane's avatar
    Fix postgres_fdw to return the right ctid value in EvalPlanQual cases. · 0bb8528b
    Tom Lane authored
    If a postgres_fdw foreign table is a non-locked source relation in an
    UPDATE, DELETE, or SELECT FOR UPDATE/SHARE, and the query selects its
    ctid column, the wrong value would be returned if an EvalPlanQual
    recheck occurred.  This happened because the foreign table's result row
    was copied via the ROW_MARK_COPY code path, and EvalPlanQualFetchRowMarks
    just unconditionally set the reconstructed tuple's t_self to "invalid".
    
    To fix that, we can have EvalPlanQualFetchRowMarks copy the composite
    datum's t_ctid field, and be sure to initialize that along with t_self
    when postgres_fdw constructs a tuple to return.
    
    If we just did that much then EvalPlanQualFetchRowMarks would start
    returning "(0,0)" as ctid for all other ROW_MARK_COPY cases, which perhaps
    does not matter much, but then again maybe it might.  The cause of that is
    that heap_form_tuple, which is the ultimate source of all composite datums,
    simply leaves t_ctid as zeroes in newly constructed tuples.  That seems
    like a bad idea on general principles: a field that's really not been
    initialized shouldn't appear to have a valid value.  So let's eat the
    trivial additional overhead of doing "ItemPointerSetInvalid(&(td->t_ctid))"
    in heap_form_tuple.
    
    This closes out our handling of Etsuro Fujita's report that tableoid and
    ctid weren't correctly set in postgres_fdw EvalPlanQual cases.  Along the
    way we did a great deal of work to improve FDWs' ability to control row
    locking behavior; which was not wasted effort by any means, but it didn't
    end up being a fix for this problem because that feature would be too
    expensive for postgres_fdw to use all the time.
    
    Although the fix for the tableoid misbehavior was back-patched, I'm
    hesitant to do so here; it seems far less likely that people would care
    about remote ctid than tableoid, and even such a minor behavioral change
    as this in heap_form_tuple is perhaps best not back-patched.  So commit
    to HEAD only, at least for the moment.
    
    Etsuro Fujita, with some adjustments by me
    0bb8528b
execMain.c 84.2 KB