| From: | Chengpeng Yan <chengpeng_yan(at)outlook(dot)com> |
|---|---|
| To: | Tatsuya Kawata <kawatatatsuya0913(at)gmail(dot)com> |
| Cc: | Ilia Evdokimov <ilya(dot)evdokimov(at)tantorlabs(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, John Naylor <johncnaylorls(at)gmail(dot)com> |
| Subject: | Re: [PATCH] ANALYZE: hash-accelerate MCV tracking for equality-only types |
| Date: | 2026-02-03 04:33:35 |
| Message-ID: | 094AA26A-BB1C-475E-ADB2-BF14F5B1E778@Outlook.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Feb 2, 2026, at 21:09, Tatsuya Kawata <kawatatatsuya0913(at)gmail(dot)com> wrote:
>
> Hi,
>
> Thank you for the detailed explanation! Your explanation helped me understand the design much better.
> I hope my understanding is now on the right track.
>
> I tested v3 both approaches:
>
> 1. Ilia's proposal with corrected increment and <= condition:
> if (was_count1 && j <= firstcount1)
> firstcount1++;
>
> 2. The original patch with while loop:
> while (use_hash && firstcount1 < track_cnt &&
> track[firstcount1].count > 1)
> firstcount1++;
>
> I verified the following cases and both approaches produced correct
> track array values after the loop completed:
>
> Case 1: c1_cursor == match_index
> c1_cursor points to a singleton, that singleton is matched again,
> bubble-up occurs, then a new value arrives triggering eviction.
>
> Case 2: c1_cursor < match_index
> c1_cursor is in the earlier part of the singleton region,
> and a singleton further back is matched.
>
> Case 3: c1_cursor > match_index
> c1_cursor has advanced past match_index due to previous evictions,
> and an earlier singleton is matched.
>
> Both approaches seem to work correctly. The code reduction from 1 is minimal, so either approach should be fine.
> I believe the while loop exists to handle potential edge cases,
> though in typical scenarios firstcount1 would only increment once per match (since one singleton is promoted at a time).
>
> Overall, the patch looks good to me.
Hi Tatsuya,
Thank you for the detailed testing and for validating those
c1_cursor/match_index cases. I agree with your conclusion that both
variants behave correctly, and that the code reduction from the
single-step approach is small.
On firstcount1: in the typical case it should advance by one when a
singleton is promoted. I kept the loop-style adjustment mainly as an
invariant repair step in hash mode — after bubble-up, it simply advances
firstcount1 until it again points to the first singleton (or track_cnt).
That makes the update less dependent on subtle index relationships and
is a bit more robust against potential corner cases (or future tweaks to
the reordering), while still being cheap since firstcount1 only moves
forward and is bounded by track_cnt/track_max.
That said, if other community members would prefer the simpler one-step
update for readability, I’m happy to switch.
--
Best regards,
Chengpeng Yan
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Chao Li | 2026-02-03 04:47:34 | Re: Fix pg_stat_get_backend_wait_event() for aux processes |
| Previous Message | Kyotaro Horiguchi | 2026-02-03 04:07:08 | Re: pg_resetwal: Fix wrong directory in log output |