Rework manipulation and structure of attribute mappings

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Cc: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Subject: Rework manipulation and structure of attribute mappings
Date: 2019-11-21 04:25:56
Message-ID: 20191121042556.GD153437@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi all,

After working on dc816e58, I have noticed that what we are doing with
attribute mappings is not that good. In a couple of code paths of the
rewriter, the executor, and more particularly ALTER TABLE, when
working on the creation of inherited relations or partitions an
attribute mapping gets used to make sure that the new cloned elements
(indexes, fk, etc.) have correct definitions linked correctly from the
parent to the child's attributes.

Sometimes things can go wrong, because the attribute array is just an
AttrNumber pointer and it is up to the caller building the map to
guess which length it has. Existing callers do that fine, but this
can lead to errors as recent history has proved.

Attached is a patch to refactor all that which simply adds the
attribute mapping length directly with the attribute list. The nice
effect of the refactoring is that now callers willing to get attribute
maps don't need to think about which length it should have, and this
allows to perform checks on the expected number of attributes in the
map particularly in the executor part. A couple of structures also
have their logic simplified.

On top of that, I have spotted two fishy attribute mapping calculations
in addFkRecurseReferencing() when adding a foreign key for partitions
when there are dropped columns and in CloneFkReferencing(). The
mapping was using the number of attributes from the foreign key, which
can be lower than the mapping of the parent if there are dropped
columns in-between. I am pretty sure that if some attributes of the
parent are dropped (aka mapping set to 0 in the middle of its array
then we could finish with an incorrect attribute mapping, and I
suspect that this could lead to failures similar to what was fixed in
dc816e58, but I have not spent much time yet into that part.

I'll add this patch to the next CF for review. The patch compiles and
passes all regression tests.

Thanks,
--
Michael

Attachment Content-Type Size
v1-0001-Rework-attribute-mappings.patch text/x-diff 44.1 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro Horiguchi 2019-11-21 04:33:48 Re: an OID >= 8000 in master
Previous Message Dilip Kumar 2019-11-21 04:25:21 Re: [HACKERS] Block level parallel vacuum