Re: posgres 12 bug (partitioned table)

Amit Langote <amitlangote09@gmail.com> writes:
> On Thu, Apr 22, 2021 at 4:10 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> In the meantime, if the back branches fail with something like
>> "virtual tuple table slot does not have system attributes" when
>> trying to do this, that's not great but I'm not sure we should
>> be putting effort into improving it.

> Got it.  That sounds like an acceptable compromise.

Hm, we're not done with this.  Whatever you think of the merits of
throwing an implementation-level error, what's actually happening
in v12 and v13 in the wake of a71cfc56b is that an attempt to do
"RETURNING xmin" works in a non-cross-partition UPDATE, but in
a cross-partition UPDATE it dumps core.  What I'm seeing is that
the result of the execute_attr_map_slot() call looks like

(gdb) p *(BufferHeapTupleTableSlot*) scanslot
$3 = {base = {base = {type = T_TupleTableSlot, tts_flags = 8, tts_nvalid = 3, 
      tts_ops = 0xa305c0 <TTSOpsBufferHeapTuple>, 
      tts_tupleDescriptor = 0x7f1c7798f920, tts_values = 0x2077100, 
      tts_isnull = 0x2075fb0, tts_mcxt = 0x2015ea0, tts_tid = {ip_blkid = {
          bi_hi = 0, bi_lo = 0}, ip_posid = 3}, tts_tableOid = 37812}, 
    tuple = 0x0, off = 0, tupdata = {t_len = 0, t_self = {ip_blkid = {
          bi_hi = 0, bi_lo = 0}, ip_posid = 0}, t_tableOid = 0, 
      t_data = 0x0}}, buffer = 0}

and since base.tuple is NULL, heap_getsysattr dies on either
"Assert(tup)" or a null-pointer dereference.

So

(1) It seems like this is exposing a shortcoming in the
multiple-slot-types logic.  It's not hard to understand why the slot would
look like this after execute_attr_map_slot does ExecStoreVirtualTuple,
but if this is not a legal state for a BufferHeapTuple slot, why didn't
ExecStoreVirtualTuple raise a complaint?

(2) It also seems like we can't use the srcSlot if we want to have
the fail-because-its-a-virtual-tuple behavior.  I experimented with
doing ExecMaterializeSlot on the result of execute_attr_map_slot,
and that stops the crash, but then we're returning garbage values
of xmin etc, which does not seem good.

            regards, tom lane