public inbox for [email protected]  

help / color / mirror / Atom feed

From: Henson Choi <[email protected]>
To: Tatsuo Ishii <[email protected]>
To: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: Row pattern recognition
Date: Thu, 19 Mar 2026 11:28:29 +0900
Message-ID: <CAAAe_zBz7mjNVms-ooF79+p_7eydkONBmz8YCk=oS0+WOfj3fQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAE+cgNgSG6oiAXT=FL+gK71Squ_eacyTCxSVXaAUTwODrkzqFg@mail.gmail.com>
	<[email protected]>
	<CAN4CZFOw4X27zgYdUgbVMmT3x_22a9kdGnTe6U+2TV1jJv-1ow@mail.gmail.com>
	<[email protected]>

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

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Row pattern recognition
  In-Reply-To: <CAAAe_zBz7mjNVms-ooF79+p_7eydkONBmz8YCk=oS0+WOfj3fQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox