On 2017/12/21 10:05, Masahiko Sawada wrote:
On Wed, Dec 20, 2017 at 5:39 PM, Huong Dangminh
<huo-dangm...@ys.jp.nec.com> wrote:
Hi Sawada-san,
Thank you for quick response. The changes look good to me. But I wonder
if the following changes needs some comments to describe what each checks
does for.
- if (errarg->attnum < 0)
+ if (errarg->local_attnum <0)
+ return;
+
+ rel = errarg->rel;
+ remote_attnum = rel->attrmap[errarg->local_attnum];
+
+ if (remote_attnum < 0)
return;
Thanks, I have added some comments as you mentioned.
Thank you for updating the patch.
- if (errarg->attnum < 0)
+ /* Check case of slot_store_error_callback() is called before
+ * errarg.local_attnum is set. */
+ if (errarg->local_attnum <0)
This comment style isn't preferred by PostgreSQL code. Please refer to
https://www.postgresql.org/docs/current/static/source-format.html
--
$ git diff --check
src/backend/replication/logical/worker.c:291: trailing whitespace.
+ /* Check case of slot_store_error_callback() is called before
Thanks, I will be careful in the next time.
There is an extra white space in the patch.
I'm inclined to think SlotErrCallbackArg can have remote_attnum and
pass it to the callback function. That way, we don't need to case the
case where local_attnum is not set yet.
Nice.
Attached a new version patch incorporated the aboves. Please review it.
Thanks for updating the patch.
It looks fine to me.
---
Thanks and best regards,
Dang Minh Huong
