Re: problem with RETURNING and update row movement - Mailing list pgsql-hackers

On Fri, Aug 7, 2020 at 10:45 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> 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].

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