Re: Remove some redundant set_cheapest() calls

From: Richard Guo <guofenglinux(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Remove some redundant set_cheapest() calls
Date: 2024-03-27 07:06:50
Message-ID: CAMbWs4_zhKyQaxOFe8mViY_fsi9rO=wsALijRg+PvXvsr9=U+w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Mar 27, 2024 at 4:06 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Richard Guo <guofenglinux(at)gmail(dot)com> 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bertrand Drouvot 2024-03-27 07:13:16 Re: pgsql: Track last_inactive_time in pg_replication_slots.
Previous Message Donghang Lin 2024-03-27 07:03:08 Re: Parallel Bitmap Heap Scan reports per-worker stats in EXPLAIN ANALYZE