Hi Vignesh.
Some review comments for patch v2-0001.
======
append_tuple_value_detail:
1.
+ if (first)
+ appendStringInfoString(buf, "tuple data not available (insufficient
privileges)");
The logic to use first==true to mean "insufficient privilege" seems
strange. Presumably, the only way to get this message is when the
`continue` of the prior loop happened at *every* iteration.
Isn't it ambiguous? e.g. What if there are multiple tuple_values but
only 1 tuple_value was NULL? Then the boolean `first` will not be
true, so there was something unavailable due to insufficient
privileges that has gone unreported (???).
~~~
errdetail_apply_conflict:
2.
case CT_UPDATE_DELETED:
- appendStringInfoString(&err_detail, _("Could not find the row to be
updated"));
- append_tuple_value_detail(&err_detail,
- list_make2(remote_desc, search_desc),
- true);
+ append_tuple_value_detail(&tuple_buf,
+ list_make2(remote_desc, search_desc));
+ appendStringInfo(&err_detail, _("Could not find the row to be
updated: %s.\n"),
+ tuple_buf.data);
Results in a whitespace line after the CT_UPDATE_DELETED: that was not
there before.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
