Hi Alvaro,
On Tue, Sep 8, 2020 at 8:44 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> On 2020-Sep-07, Alvaro Herrera wrote:
>
> > Ah, it looks like we can get away with initializing the RRI to 0, and
> > then explicitly handle that case in ExecPartitionCheckEmitError, as in
> > the attached (which means reindenting, but I left it alone to make it
> > easy to read).
At this point, I think it may be clear that ri_RangeTableIndex being
set to a dummy value isn't problematic. I checked the code paths you
mentioned upthread, but don't see anything that would be broken by
ri_RangeTableIndex being set to a dummy value of 1.
Moving on from that...
> Well, that was silly -- this seems fixable by mapping the tuple columns
> prior to ExecPartitionCheck, which is achievable with something like
>
> if (partidx == partdesc->boundinfo->default_index)
> {
> TupleTableSlot *tempslot;
>
> if (dispatch->tupmap != NULL)
> {
> tempslot = MakeSingleTupleTableSlot(RelationGetDescr(rri->ri_RelationDesc),
>
&TTSOpsHeapTuple);
> tempslot = execute_attr_map_slot(dispatch->tupmap, slot, tempslot);
> }
> else
> tempslot = slot;
> ExecPartitionCheck(rri, tempslot, estate, true);
> if (dispatch->tupmap != NULL)
> ExecDropSingleTupleTableSlot(tempslot);
> }
>
> though this exact incantation, apart from being pretty horrible, doesn't
> actually work (other than to fix this specific test case -- it crashes
> elsewhere.)
Yeah, we need to make sure that ExecPartitionCheck gets a slot whose
TupleDesc matches the partition's. Actually we do have such dedicated
slots for partitions around (for both sub-partitioned and leaf
partitions), so we can simply use them instead of creating one from
scratch for every use. It did take some code rearrangement to
actually make that work though.
Attached is the latest patch including those changes. Also, I have
merged your earlier "fixes" patch and updated the isolation test to
exercise a case with sub-partitioned default partition, as well as
varying attribute numbers. Patch applies as-is to both HEAD and v13
branches, but needs slight changes to apply to v12 branch, so also
attaching one for that branch.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
