As I’m sure you know Jian this needs a rebase now that a0b6ef29a518 has been 
merged. It’s a bit hard for me to review in this state when I can’t apply the 
patches cleanly.

+ /*
+ * we can not use ExecEvalExprNoReturn here, because we
+ * use ExecInitExpr compile NewColumnValue->expr. Here,
+ * we only check whether the oldslot value satisfies the
+ * domain constraint. So it is ok to override the value
+ * evaluated by ExecEvalExpr.
+ */
+ values = ExecEvalExpr(ex->exprstate, econtext, &isnull);
+ values = (Datum) 0;
+ isnull = true;

I don’t understand this piece of code, and why value is re-assigned right away. 
Not saying it’s wrong but if you could explain why it is like that to someone 
not well versed in C. Would something like (void) ExecEvalExpr(ex->exprstate, 
econtext, &isnull); do?

There are other things I don’t quite understand so will give it another pass 
once it’s been rebased.

/Viktor

Reply via email to