Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation
Alvaro Herrera <alvherre@2ndquadrant.com>
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Amit Langote <amitlangote09@gmail.com>
Date: 2018-11-05T22:25:09Z
Lists: pgsql-bugs, pgsql-hackers
Attachments
- PG10-v4-0001-Fix-bug-regarding-partition-column-option-inherit.patch (text/x-diff) patch v4-0001
On 2018-Aug-07, Amit Langote wrote: > > But in > > case of partitioning there is only one parent and hence > > coldef->constraints is NULL and hence just overwriting it works. I > > think we need comments mentioning this special case. > > That's what I wrote in this comment: > > /* > * Parent's constraints, if any, have been saved in > * 'constraints', which are passed back to the caller, > * so it's okay to overwrite the variable like this. > */ What is this for? I tried commenting out that line to see what test breaks, and found none. I tried to figure it out, so while thinking what exactly is "restdef" in that block, it struck me that we seem to be doing things in quite a strange way there by concatenating both schema lists. I changed it so that that block scans only the "saved_schema" list (ie. the partition-local column list definition), searching the other list for each matching item. This seems a much more natural implementation -- at least, it results in less list mangling and shorter code, so. One thing that broke was that duplicate columns in the partition-local definition would not be caught. However, we already have that check a few hundred lines above in the same function ... which was skipped for partitions because of list-mangling that was done before that. I moved the NIL-setting of schema one block below, and then all tests pass. One thing that results is that is_from_parent becomes totally useless and can be removed. It could in theory be removed from ColumnDef, if it wasn't for the ABI incompatibility that would cause. BTW this line: coldef->is_not_null == (coldef->is_not_null || restdef->is_not_null) can be written more easily like: coldef->is_not_null |= restdef->is_not_null; -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Commits
-
Revise attribute handling code on partition creation
- e0c05bf4a7e6 11.2 landed
- 705d433fd5e2 12.0 landed
- 21c9e4973cec 10.7 landed