Re: Remove redundant NULL check in clause_selectivity_ext() function

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Ilia Evdokimov <ilya(dot)evdokimov(at)tantorlabs(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Remove redundant NULL check in clause_selectivity_ext() function
Date: 2024-09-23 00:43:21
Message-ID: CAApHDvoXqxi_+X=X8TYa5oywPVJVWx2Fs+yQJ_GvxbvS3X_5nQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, 20 Aug 2024 at 03:48, Ilia Evdokimov
<ilya(dot)evdokimov(at)tantorlabs(dot)com> wrote:
> Let's assume that this check needs to remain, and the length check doesn't guarantee anything. However, I'm a bit concerned that there's a NULL check here, but it's missing in the clauselist_selectivity_ext() function. For the reasons mentioned above, I would suggest the following: either we perform the NULL check in both places, or we don't perform it in either.

I don't follow this comparison. clauselist_selectivity_ext() is
perfectly capable of accepting a NIL List of clauses.

I agree with Tom that it's unlikely to be worth the risk removing the
NULL check from clause_selectivity_ext(). From my point of view, the
risk-to-reward ratio is nowhere near the level of being worth it.
There'd just be no way to measure any sort of speedup from this change
as there are far too many other things going on during planning. This
one is a drop in the ocean.

However, I'd like to encourage you to look for other places that might
have a more meaningful impact on performance. For those, it's best to
come armed with a benchmark and results that demonstrate the speedup
along with your justification as to why you think the change is
worthwhile. We've not received the former and you've not convinced two
committers with your attempt on the latter.

I suggest marking the CF entry for this patch as rejected.

David

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andy Fan 2024-09-23 00:53:24 Re: FullTransactionIdAdvance question
Previous Message David Rowley 2024-09-22 23:55:07 Re: Add memory/disk usage for WindowAgg nodes in EXPLAIN