Re: [BUG] Partition creation fails after dropping a column and addinga partial index - Mailing list pgsql-hackers

From Amit Langote
Subject Re: [BUG] Partition creation fails after dropping a column and addinga partial index
Date
Msg-id CA+HiwqE3BBvnpXyEgOnbp1aENy=8s8QFHcp6wNPY5X2YE71OKQ@mail.gmail.com
Whole thread Raw
In response to Re: [BUG] Partition creation fails after dropping a column andadding a partial index  (Michael Paquier <michael@paquier.xyz>)
Responses Re: [BUG] Partition creation fails after dropping a column andadding a partial index  (Michael Paquier <michael@paquier.xyz>)
List pgsql-hackers
On Thu, Oct 31, 2019 at 1:45 PM Michael Paquier <michael@paquier.xyz> wrote:
> On Tue, Oct 29, 2019 at 01:16:58PM +0900, Michael Paquier wrote:
> > Yes, something looks wrong with that.  I have not looked at it in
> > details yet though.  I'll see about that tomorrow.
>
> So..  When building the attribute map for a cloned index (with either
> LIKE during the transformation or for partition indexes), we store
> each attribute number with 0 used for dropped columns.  Unfortunately,
> if you look at the way the attribute map is built in this case the
> code correctly generates the mapping with convert_tuples_by_name_map.
> But, the length of the mapping used is incorrect as this makes use of
> the number of attributes for the newly-created child relation, and not
> the parent which includes the dropped column in its count.  So the
> answer is simply to use the parent as reference for the mapping
> length.
>
> The patch is rather simple as per the attached, with extended
> regression tests included.  I have not checked on back-branches yet,
> but that's visibly wrong since 8b08f7d down to v11 (will do that when
> back-patching).

The patch looks correct and applies to both v12 and v11.

> There could be a point in changing convert_tuples_by_name_map & co so
> as they return the length of the map on top of the map to avoid such
> mistakes in the future.  That's a more invasive patch not really
> adapted for a back-patch, but we could do that on HEAD once this bug
> is fixed.  I have also checked other calls of this API and the
> handling is done correctly.

I've been bitten by this logical error when deciding what length to
use for the map, so seems like a good idea.

Thanks,
Amit



pgsql-hackers by date:

Previous
From: "Moon, Insung"
Date:
Subject: Re: [Proposal] Table-level Transparent Data Encryption (TDE) and KeyManagement Service (KMS)
Next
From: Quan Zongliang
Date:
Subject: Re: Restore replication settings when modifying a field type