Re: posgres 12 bug (partitioned table)

From: Andres Freund <andres(at)anarazel(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Amit Langote <amitlangote09(at)gmail(dot)com>, David Steele <david(at)pgmasters(dot)net>, Pavel Biryukov <79166341370(at)yandex(dot)ru>, PostgreSQL mailing lists <pgsql-bugs(at)lists(dot)postgresql(dot)org>
Subject: Re: posgres 12 bug (partitioned table)
Date: 2021-04-22 18:31:48
Message-ID: 20210422183148.xsopy2qsym7a7app@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

Hi,

On 2021-04-22 14:21:05 -0400, Tom Lane wrote:
> 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?

I think it's too useful to support ExecStoreVirtualTuple() in a
heap[buffer] slot to disallow that. Seems like we should make
tts_buffer_heap_getsysattr() error out if there's no tuple?

> (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.

Garbage values as in 0's, or random data? Seems like it should be the
former?

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Tom Lane 2021-04-22 18:37:26 Re: posgres 12 bug (partitioned table)
Previous Message Tom Lane 2021-04-22 18:21:05 Re: posgres 12 bug (partitioned table)

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2021-04-22 18:35:13 Re: multi-install PostgresNode fails with older postgres versions
Previous Message Peter Geoghegan 2021-04-22 18:30:21 Re: decoupling table and index vacuum