Re: Remove some redundant set_cheapest() calls
On Wed, Mar 27, 2024 at 10:59 PM Tom Lane wrote: > Richard Guo writes: > > On Wed, Mar 27, 2024 at 4:06 AM Tom Lane wrote: > >> I'm less convinced about changing this. I'd rather keep it consistent > >> with mark_dummy_rel. > > > Hm, I wonder if we should revise the comment there that states "but not > > when called from elsewhere", as it does not seem to be true. > > I'd be okay with wording like "This is redundant in current usage > because set_rel_pathlist will do it later, but it's cheap so we keep > it for consistency with mark_dummy_rel". What do you think? That works for me. Thanks for the wording. Thanks Richard
Re: Remove some redundant set_cheapest() calls
Richard Guo writes: > On Wed, Mar 27, 2024 at 4:06 AM Tom Lane wrote: >> I'm less convinced about changing this. I'd rather keep it consistent >> with mark_dummy_rel. > Hm, I wonder if we should revise the comment there that states "but not > when called from elsewhere", as it does not seem to be true. I'd be okay with wording like "This is redundant in current usage because set_rel_pathlist will do it later, but it's cheap so we keep it for consistency with mark_dummy_rel". What do you think? regards, tom lane
Re: Remove some redundant set_cheapest() calls
On Wed, Mar 27, 2024 at 4:06 AM Tom Lane wrote: > Richard Guo writes: > > I happened to notice that the set_cheapest() calls in functions > > set_namedtuplestore_pathlist() and set_result_pathlist() are redundant, > > as we've centralized the set_cheapest() calls in set_rel_pathlist(). > > > Attached is a trivial patch to remove these calls. > > Agreed, and pushed. Thanks for pushing! > > BTW, I suspect that the set_cheapest() call in set_dummy_rel_pathlist() > > is also redundant. The comment there says "This is redundant when we're > > called from set_rel_size(), but not when called from elsewhere". I > > doubt it. The other places where it is called are set_append_rel_size() > > and set_subquery_pathlist(), both being called from set_rel_size(). So > > set_cheapest() would ultimately be called in set_rel_pathlist(). > > I'm less convinced about changing this. I'd rather keep it consistent > with mark_dummy_rel. Hm, I wonder if we should revise the comment there that states "but not when called from elsewhere", as it does not seem to be true. Thanks Richard
Re: Remove some redundant set_cheapest() calls
Richard Guo writes: > I happened to notice that the set_cheapest() calls in functions > set_namedtuplestore_pathlist() and set_result_pathlist() are redundant, > as we've centralized the set_cheapest() calls in set_rel_pathlist(). > Attached is a trivial patch to remove these calls. Agreed, and pushed. > BTW, I suspect that the set_cheapest() call in set_dummy_rel_pathlist() > is also redundant. The comment there says "This is redundant when we're > called from set_rel_size(), but not when called from elsewhere". I > doubt it. The other places where it is called are set_append_rel_size() > and set_subquery_pathlist(), both being called from set_rel_size(). So > set_cheapest() would ultimately be called in set_rel_pathlist(). I'm less convinced about changing this. I'd rather keep it consistent with mark_dummy_rel. regards, tom lane
Remove some redundant set_cheapest() calls
I happened to notice that the set_cheapest() calls in functions set_namedtuplestore_pathlist() and set_result_pathlist() are redundant, as we've centralized the set_cheapest() calls in set_rel_pathlist(). Attached is a trivial patch to remove these calls. BTW, I suspect that the set_cheapest() call in set_dummy_rel_pathlist() is also redundant. The comment there says "This is redundant when we're called from set_rel_size(), but not when called from elsewhere". I doubt it. The other places where it is called are set_append_rel_size() and set_subquery_pathlist(), both being called from set_rel_size(). So set_cheapest() would ultimately be called in set_rel_pathlist(). Thoughts? Thanks Richard v1-0001-Remove-some-redundant-set_cheapest-calls.patch Description: Binary data
