Re: problem with RETURNING and update row movement

On Mon, Aug 3, 2020 at 4:39 PM Amit Langote <amitlangote09@gmail.com> wrote:
> On Mon, Aug 3, 2020 at 2:54 PM Amit Langote <amitlangote09@gmail.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].  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.

Best regards,
Etsuro Fujita

[2] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=da7d44b627ba839de32c9409aca659f60324de76