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
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 |