|From:||Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>|
|To:||Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>|
|Cc:||Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Amit Langote <amitlangote09(at)gmail(dot)com>|
|Subject:||Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
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
|Next Message||Michael Paquier||2018-11-06 00:14:07||Re: BUG #15437: Segfault during insert into declarative partitioned table with a trigger creating partition|
|Previous Message||Tom Lane||2018-11-05 20:16:17||Re: Ris: BUG #15482: Foreign keys to a partition (NOT A PARTITIONED) break the server|
|Next Message||Tatsuo Ishii||2018-11-05 23:43:03||Re: Code of Conduct plan,Re: Code of Conduct plan,Re: Code of Conduct plan,Re: Code of Conduct plan,Re: Code of Conduct plan,Re: Code of Conduct plan,Re: Code of Conduct plan,Re: Code of Conduct plan|
|Previous Message||Andres Freund||2018-11-05 22:11:42||Re: Why do pg_upgrade's test use the serial schedule?|