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
becauseof 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
