|From:||Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>|
|To:||Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, 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|
|Views:||Raw Message | Whole Thread | Download mbox|
On 2018/07/10 22:50, Ashutosh Bapat wrote:
> I didn't see any hackers thread linked to this CF entry. Hence sending this mail through CF app.
Hmm, yes. I hadn't posted the patch to -hackers.
> 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;
Agreed, done like that.
> 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.
I have modified the comments around this code in the updated patch.
> On a side note, I see
> coldef->constraints = restdef->constraints;
> Shouldn't we merge the constraints instead of just overwriting those?
Actually, I checked that coldef->constraints is always NIL in this case.
coldef (ColumnDef) is constructed from a member in the parent's TupleDesc
earlier in that function and any constraints that may be present in the
parent's definition of the column are saved in a separate variable that's
returned to the caller as one list containing "old"/inherited constraints.
So, no constraints are getting lost here.
Attached is an updated patch. I've updated some nearby comments as the
code around here seems pretty confusing, which I thought after having
returned to it after a while.
I have attached both a patch for PG10 and PG11/HEAD branches, which are
actually not all that different from each other.
|Next Message||Michael Paquier||2018-07-12 09:06:16||Re: Negotiating the SCRAM channel binding type|
|Previous Message||Marina Polyakova||2018-07-12 08:57:01||Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors|
|Next Message||Arthur Zakirov||2018-07-12 09:22:06||Re: BUG #15277: ts_headline strips things that look like HTML tags and it cannot be disabled|
|Previous Message||PG Bug reporting form||2018-07-12 07:59:40||BUG #15277: ts_headline strips things that look like HTML tags and it cannot be disabled|