On Wed, Jun 20, 2012 at 01:54:29AM -0400, Greg Stein wrote:
> On Jun 19, 2012 9:52 PM, <stsp_at_apache.org> wrote:
> >...
> > + if (resolve_conflicts_post_switch)
> > + {
> > + /* Remove the conflict resolution callback from the client context.
> > + * We invoke it after of the switch instead of during the switch.
> */
> > + conflict_func2 = ctx->conflict_func2;
> > + conflict_baton2 = ctx->conflict_baton2;
> > + ctx->conflict_func2 = NULL;
> > + ctx->conflict_baton2 = NULL;
> > + }
>
> This feels like a terrible hack, of the worst kind.
>
> The caller gives you a client context, and you change it on them. It's
> theirs. Not yours to redefine.
> Even worse, you never restore it! The context is now damaged because you
> monkeyed with it. The context is NOT a single-use structure. It can live
> for hours, over multiple operations, by design. But the above hack destroys
> it.
>
> I think you need to find a new approach.
Agreed, plus I'm not happy with changing the behaviour for 1.7 API users.
Keeping 1.5 behaviour the same while changing behaviour of already released
1.7 APIs is... inconsistent.
Maybe we need a new-in-1.8 conflict_func3 that has different invocation
rules than conflict_func2, but otherwise the same signature (unless we
decide to change it before release for some reason)? Would that be better?
Received on 2012-06-20 12:00:18 CEST