| From: | Henson Choi <assam258(at)gmail(dot)com> |
|---|---|
| To: | Tatsuo Ishii <ishii(at)postgresql(dot)org>, zsolt(dot)parragi(at)percona(dot)com |
| Cc: | sjjang112233(at)gmail(dot)com, vik(at)postgresfriends(dot)org, er(at)xs4all(dot)nl, jacob(dot)champion(at)enterprisedb(dot)com, david(dot)g(dot)johnston(at)gmail(dot)com, peter(at)eisentraut(dot)org, pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: Row pattern recognition |
| Date: | 2026-03-19 02:28:29 |
| Message-ID: | CAAAe_zBz7mjNVms-ooF79+p_7eydkONBmz8YCk=oS0+WOfj3fQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Tatsuo, Zsolt,
I just confirmed the crash.
>
> Henson, What do you think?
>
Good catch by Zsolt. Your careful review is really helping a lot.
I've fixed both the prefix merge and suffix merge paths in
mergeGroupPrefixSuffix() to also increment child->max. I'll
include the fix with a regression test in the next patch series.
Regarding the suggested fix:
if (child->max != RPR_QUANTITY_INF)
child->max += 1;
While the quantifier value is int (INT32_MAX = RPR_QUANTITY_INF),
making such repetition counts practically impossible, the +1
approach is not semantically equivalent -- it could silently turn
a finite max into infinity. Instead, I took a fallback approach:
skip the merge entirely when min or max would reach
RPR_QUANTITY_INF after increment, consistent with the overflow
checks in mergeConsecutiveVars/Groups.
> Good point. You are right, the plan cache should be read only.
> However, Henson is working on a different approach and the code will
> not be used any more...
>
Yes, I'm currently working on a slot-based approach (1-slot
PREV/NEXT) that won't need attno_map, so the plan cache mutation
issue will be gone.
> I forggot to include the data file. Please apply attached patch on top
> of v45.
>
Got it, thanks. By the way, how about splitting the test patch
into two -- one for sql + data files and another for expected
output files? The single test patch is getting quite large and
cfbot often fails to apply it.
Right. Maybe ERRCODE_TOO_MANY_ARGUMENTS?
>
I'm currently reworking PREV/NEXT to accept 2 arguments, so this
error handling will change. I'll take care of the proper error
codes along with the GROUPS typo and ERRCODE_WINDOWING_ERROR change.
Best regards,
Henson
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Xuneng Zhou | 2026-03-19 02:33:44 | Re: [WIP] Pipelined Recovery |
| Previous Message | Chao Li | 2026-03-19 02:27:50 | Re: Avoiding memory leakage in jsonpath evaluation |