On Wed, Oct 9, 2024 at 8:58 AM shveta malik <shveta.malik@gmail.com> wrote:
>
> On Tue, Oct 8, 2024 at 3:12 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> >
>
Please find few comments on v14-patch004:
patch004:
1)
GetConflictResolver currently errors out when the resolver is
last_update_wins and track_commit_timestamp is disabled. It means
every conflict resolution with this resolver will keep on erroring
out. I am not sure if we should emit ERROR here. We do emit ERROR when
someone tries to configure last_update_wins but track_commit_timestamp
is disabled. I think that should suffice. The one in
GetConflictResolver can be converted to WARNING max.
What could be the side-effect if we do not emit error here? In such a
case, the local timestamp will be 0 and remote change will always win.
Is that right? If so, then if needed, we can emit a warning saying
something like: 'track_commit_timestamp is disabled and thus remote
change is applied always.'
Thoughts?
2)
execReplication.c:
There are some optimizations in this file (moving duplicate code to
has_conflicting_tuple), I think these optimizations are applicable
even to patch003 (or patch002 as well?) and thus can be moved there.
Please review once.
thanks
Shveta