Re: BUG #15212: Default values in partition tables don't work as expected and allow NOT NULL violation

From: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: 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
Date: 2018-07-10 13:50:47
Message-ID: 153123064797.1384.12444178573294974260.pgcf@coridan.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

I didn't see any hackers thread linked to this CF entry. Hence sending this mail through CF app.

The patch looks good to me. It applies cleanly, compiles cleanly and make check passes.

I think you could avoid condition
+ /* Do not override parent's NOT NULL constraint. */
+ if (restdef->is_not_null)
+ coldef->is_not_null = restdef->is_not_null;

by rewriting this line as
coldef->is_not_null = coldef->is_not_null || restdef->is_not_null;

The comment looks a bit odd either way since we are changing coldef->is_not_null based on restdef->is_not_null. That's because of the nature of boolean variables. May be a bit of explanation is needed.

On a side note, I see
coldef->constraints = restdef->constraints;
Shouldn't we merge the constraints instead of just overwriting those?
--
Best Wishesh
Ashutosh

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Tom Lane 2018-07-10 13:58:28 Re: BUG #15272: creating an index function terminate with error
Previous Message PG Bug reporting form 2018-07-10 13:49:49 BUG #15272: creating an index function terminate with error

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2018-07-10 13:51:27 Re: Non-reserved replication slots and slot advancing
Previous Message Tom Lane 2018-07-10 13:47:09 Re: How can we submit code patches that implement our (pending) patents?