On Sun, Apr 18, 2021 at 1:21 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > > I wrote: > > I think it's time for some refactoring of this code so that we can > > actually share the logic. Accordingly, I propose the attached. > > After sleeping on it, here's an improved version that gets rid of > an unnecessary assumption about ECs usually not containing both > parallel-safe and parallel-unsafe members. I'd tried to do this > yesterday but didn't like the amount of side-effects on createplan.c > (caused by the direct call sites not being passed the "root" pointer). > However, we can avoid refactoring createplan.c APIs by saying that it's > okay to pass root = NULL to find_computable_ec_member if you're not > asking it to check parallel safety. And there's not really a need to > put a parallel-safety check into find_ec_member_matching_expr at all; > that task can be left with the one caller that cares.
I like the refactoring here. Two things I wonder: 1. Should we add tests for the relabel code path? 2. It'd be nice not to have the IS_SRF_CALL duplicated, but that might add enough complexity that it's not worth it. Thanks, James Coleman
