Rework manipulation and structure of attribute mappings
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Cc: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Date: 2019-11-21T04:25:56Z
Lists: pgsql-hackers
Attachments
- v1-0001-Rework-attribute-mappings.patch (text/x-diff) patch v1-0001
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
Commits
-
Refactor attribute mappings used in logical tuple conversion
- e1551f96e643 13.0 landed