Re: problem with RETURNING and update row movement

On Fri, Aug 7, 2020 at 10:45 PM Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> wrote:
> On Mon, Aug 3, 2020 at 4:39 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > On Mon, Aug 3, 2020 at 2:54 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > > By the way, you'll need two adjustments to even get this example
> > > working, one of which is a reported problem that system columns in
> > > RETURNING list during an operation on partitioned table stopped
> > > working in PG 12 [1] for which I've proposed a workaround (attached).
> > > Another is that we forgot in our patch to "materialize" the virtual
> > > tuple after conversion, which means slot_getsysattr() can't find it to
> > > access system columns like xmin, etc.
> >
> > The only solution I could think of for this so far is this:
> >
> > + if (map)
> > + {
> > + orig_slot = execute_attr_map_slot(map,
> > + res_slot,
> > + orig_slot);
> > +
> > + /*
> > + * A HACK to install system information into the just
> > + * converted tuple so that RETURNING computes any
> > + * system columns correctly. This would be the same
> > + * information that would be present in the HeapTuple
> > + * version of the tuple in res_slot.
> > + */
> > + tuple = ExecFetchSlotHeapTuple(orig_slot, true,
> > + &should_free);
> > + tuple->t_data->t_infomask &= ~(HEAP_XACT_MASK);
> > + tuple->t_data->t_infomask2 &= ~(HEAP2_XACT_MASK);
> > + tuple->t_data->t_infomask |= HEAP_XMAX_INVALID;
> > + HeapTupleHeaderSetXmin(tuple->t_data,
> > + GetCurrentTransactionId());
> > + HeapTupleHeaderSetCmin(tuple->t_data,
> > + estate->es_output_cid);
> > + HeapTupleHeaderSetXmax(tuple->t_data, 0); /*
> > for cleanliness */
> > + }
> > + /*
> > + * Override tts_tableOid with the OID of the destination
> > + * partition.
> > + */
> > + orig_slot->tts_tableOid =
> > RelationGetRelid(destRel->ri_RelationDesc);
> > + /* Also the TID. */
> > + orig_slot->tts_tid = res_slot->tts_tid;
> >
> > ..but it might be too ugly :(.
>
> Yeah, I think that would be a bit ugly, and actually, is not correct
> in case of postgres_fdw foreign table, in which case Xmin and Cmin are
> also set to 0 [2].

Yeah, need to think a bit harder.

> I think we should probably first address the
> tableam issue that you pointed out, but I don't think I'm the right
> person to do so.

Okay, I will try to revive that thread.

--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com