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

Reply via email to