From: | Amit Langote <amitlangote09(at)gmail(dot)com> |
---|---|
To: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: a thinko in b676ac443b6 |
Date: | 2021-07-28 01:15:34 |
Message-ID: | CA+HiwqHL2SD3-bbk-AOTU0c-A0e1V0T6pF6+4Rdmg2AztFJrsw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Jul 28, 2021 at 1:07 AM Tomas Vondra
<tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
> On 7/27/21 4:28 AM, Amit Langote wrote:
> > I think it can be incorrect to use the same TupleDesc for both the
> > slots in ri_Slots (for ready-to-be-inserted tuples) and ri_PlanSlots
> > (for subplan output tuples). Especially if you consider what we did
> > in 86dc90056df that was committed into v14. In that commit, we
> > changed the way a subplan under ModifyTable produces its output for an
> > UPDATE statement. Previously, it would produce a tuple matching the
> > target table's TupleDesc exactly (plus any junk columns), but now it
> > produces only a partial tuple containing the values for the changed
> > columns.
> >
> > So it's better to revert to using planSlot->tts_tupleDescriptor for
> > the slots in ri_PlanSlots. Attached a patch to do so.
>
> Yeah, this seems like a clear mistake - thanks for noticing it! Clearly
> no regression test triggered the issue, so I wonder what's the best way
> to test it - any idea what would the test need to do?
Ah, I should've mentioned that this is only a problem if the original
query is an UPDATE. With v14, only INSERTs can use batching and the
subplan does output a tuple matching the target table's TupleDesc in
their case, so the code seems to work fine.
As I said, I noticed a problem when rebasing my patch to allow
cross-partition UPDATEs to use batching for the inserts that are
performed internally to implement such UPDATEs. The exact problem I
noticed is that the following Assert tts_virtual_copyslot() (via
ExecCopySlot called with an ri_PlanSlots[] entry) failed:
Assert(srcdesc->natts <= dstslot->tts_tupleDescriptor->natts);
srcdesc in this case is a slot in ri_PlanSlots[] initialized with the
target table's TupleDesc (the "thinko") and dstslot is the slot that
holds subplan's output tuple ('planSlot' passed to ExecInsert). As I
described in my previous email, dstslot's TupleDesc can be narrower
than the target table's TupleDesc in the case of an UPDATE, so the
Assert can fail in theory.
> I did some quick experiments with batched INSERTs with RETURNING clauses
> and/or subplans, but I haven't succeeded in triggering the issue :-(
Yeah, no way to trigger this except UPDATEs. It still seems like a
good idea to fix this in v14.
--
Amit Langote
EDB: http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | David Rowley | 2021-07-28 01:17:43 | Re: Slim down integer formatting |
Previous Message | Alvaro Herrera | 2021-07-28 01:04:58 | Re: archive status ".ready" files may be created too early |