Re: Subscription tests fail under CLOBBER_CACHE_ALWAYS

Amit Langote <amitlangote09(at)gmail(dot)com> writes:
> IOW, the patch you posted earlier seems like the way to go.

I really dislike that patch. I think it's doubling down on the messy,
unstructured coding patterns that got us into this situation to begin
with. I'd prefer to expend a little effort on refactoring so that
the ExecCleanupTupleRouting call can be moved to the cleanup function
where it belongs.

So, I propose the attached, which invents a new struct to carry
the stuff we've discovered to be necessary. This makes the APIs
noticeably cleaner IMHO.

I did not touch the APIs of the apply_XXX_internal functions,
as it didn't really seem to offer any notational advantage.
We can't simply collapse them to take an "edata" as I did for
apply_handle_tuple_routing, because the ResultRelInfo they're
supposed to operate on could be different from the original one.
I considered a couple of alternatives:

* Replace their estate arguments with edata, but keep the separate
ResultRelInfo arguments. This might be worth doing in future, if we
add more fields to ApplyExecutionData. Right now it'd save nothing,
and it'd create a risk of confusion about when to use the
ResultRelInfo argument vs. edata->resultRelInfo.

* Allow apply_handle_tuple_routing to overwrite edata->resultRelInfo
with the partition child's RRI, then simplify the apply_XXX_internal
functions to take just edata instead of separate estate and
resultRelInfo args. I think this would work right now, but it seems
grotty, and it might cause problems in future.

* Replace the edata->resultRelInfo field with two fields, one for
the original parent and one for the actual/current target. Perhaps
this is worth doing, not sure.

Thoughts?

regards, tom lane