Re: EvalPlanQual behaves oddly for FDW queries involving system columns - Mailing list pgsql-hackers

I wrote:
> I did a very basic update of your postgres_fdw patch to test this with,
> and attach that so that you don't have to repeat the effort.  I'm not sure
> whether we want to try to convert that into something committable.  I'm
> afraid that the extra round trips involved in doing row locking this way
> will be so expensive that no one really wants it for postgres_fdw.  It's
> more credible that FDWs operating against local storage would have use
> for it.

Of course, if we don't do that then we still have your original gripe
about ctid not being correct in EvalPlanQual results.  I poked at this
a bit and it seems like we could arrange to pass ctid through even in
the ROW_MARK_COPY case, if we define the t_ctid field of a composite
Datum as being the thing to use.  The attached WIP, mostly-comment-free
patch seems to fix the original test case.  It would likely result in
ctid coming out as (0,0) not (4294967295,0) for FDWs that don't take
any special steps to return a valid ctid, as a result of the fact that
heap_form_tuple just leaves that field zeroed.  We could fix that by
adding an ItemPointerSetInvalid(&(td->t_ctid)) call to heap_form_tuple,
but I dunno if we want to add even a small number of cycles for this
purpose to such a core function.

            regards, tom lane

diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 0c44260..2350de3 100644
*** a/contrib/postgres_fdw/postgres_fdw.c
--- b/contrib/postgres_fdw/postgres_fdw.c
*************** make_tuple_from_result_row(PGresult *res
*** 2965,2971 ****
--- 2965,2974 ----
      tuple = heap_form_tuple(tupdesc, values, nulls);

      if (ctid)
+     {
          tuple->t_self = *ctid;
+         tuple->t_data->t_ctid = *ctid;
+     }

      /* Clean up */
      MemoryContextReset(temp_context);
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 43d3c44..4c39e06 100644
*** a/src/backend/executor/execMain.c
--- b/src/backend/executor/execMain.c
*************** EvalPlanQualFetchRowMarks(EPQState *epqs
*** 2613,2621 ****

              /* build a temporary HeapTuple control structure */
              tuple.t_len = HeapTupleHeaderGetDatumLength(td);
-             ItemPointerSetInvalid(&(tuple.t_self));
              /* relation might be a foreign table, if so provide tableoid */
              tuple.t_tableOid = erm->relid;
              tuple.t_data = td;

              /* copy and store tuple */
--- 2613,2622 ----

              /* build a temporary HeapTuple control structure */
              tuple.t_len = HeapTupleHeaderGetDatumLength(td);
              /* relation might be a foreign table, if so provide tableoid */
              tuple.t_tableOid = erm->relid;
+             /* also copy t_ctid in case somebody supplied it */
+             tuple.t_self = td->t_ctid;
              tuple.t_data = td;

              /* copy and store tuple */