Subject: Re: Remove some redundant set_cheapest() calls

From: Richard Guo <[email-masked]>
Date: Wed, 27 Mar 2024 15:06:50 +0800

On Wed, Mar 27, 2024 at 4:06 AM Tom Lane <[email-masked]> wrote:

> Richard Guo <[email-masked]> 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

---

From: Tom Lane <[email-masked]>
Date: Wed, 27 Mar 2024 10:59:06 -0400

> 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