| From: | Damil Shahzad <shahzaddamil(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>, ZizhuanLiu X-MAN <44973863(at)qq(dot)com>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Fix var_eq_const: sum selectivity of all matching MCV entries instead of stopping at first match |
| Date: | 2026-08-05 08:09:09 |
| Message-ID: | CALRDUFqLyBfmMzOkYqN828bkFwWMv5WCQeGd+WkbhMLCUXnuiA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Ilia,
Thanks for the patch. I tested it on current master.
Your patch by itself keeps the same estimates as unpatched code on my tests.
The case insensitive example still estimates 10 rows instead of 19. So it
looks like a performance cleanup, not a fix for the original issue.
I also tested your patch combined with the multi MCV match change from the
original patch. On that combined version I got the same estimates as the
original patch alone:
case insensitive 'b' -> estimated 19, actual 19
exact 'B' -> estimated 10, actual 10
exact 'b' -> estimated 9, actual 9
no match constant -> estimated 1, actual 0
So I agree with your idea. Merging the frequency sum work into the same loop
is a good cleanup once we keep the multi match accumulation fix. It avoids
the extra second pass on the no match path without changing the estimates I
checked.
I think the next step is to combine both changes in one patch series.
Thanks,
Damil Shahzad
On Wed, 5 Aug 2026 at 12:33, Ilia Evdokimov <ilya(dot)evdokimov(at)tantorlabs(dot)com>
wrote:
> On 8/4/26 14:29, Damil Shahzad wrote:
>
> Tom's concern still seems important. Scanning the whole MCV list every
> time would cost more in the common case, and it only changes the result
> when the comparison operator or collation is different from the equality
> used to build the statistics. Before this can move forward, I think we need
> a stronger reason for that tradeoff.
>
> +1
>
> A cheaper way to get some benefit here without touching that tradeoff:
> when there are no MCV matches, var_eq_const does a second full pass over
> MCV list just to compute `sumcommon` - but that branch is only reached
> after the first loop has already scanned every entry. So `sumcommon` can be
> accumulated inline in that same scan, and the separate summing loop
> dropped. The match case is unaffected; only the no-match path gets faster,
> by skipping a redundant second traversal.
>
> I attached patch with these changes. What do you think?
>
> --
> Best regards,
> Ilia Evdokimov,
> Tantor Labs LLC,
> https://tantorlabs.com
>
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Vitaly Davydov | 2026-08-05 08:10:34 | Re: Deadlock detector fails to activate on a hot standby replica |
| Previous Message | Michael Paquier | 2026-08-05 08:00:42 | Re: Streamify more code paths |