On Thu, Apr 7, 2022 at 7:11 AM David Rowley <dgrowle...@gmail.com> wrote:
> On Thu, 7 Apr 2022 at 19:01, David Rowley <dgrowle...@gmail.com> wrote: > > > > On Thu, 7 Apr 2022 at 15:41, Zhihong Yu <z...@yugabyte.com> wrote: > > > + * We must keep the original qual in place if there is > a > > > + * PARTITION BY clause as the top-level WindowAgg > remains in > > > + * pass-through mode and does nothing to filter out > unwanted > > > + * tuples. > > > + */ > > > + *keep_original = false; > > > > > > The comment talks about keeping original qual but the assignment uses > the value false. > > > Maybe the comment can be rephrased so that it matches the assignment. > > > > Thanks. I've just removed that comment locally now. You're right, it > > was out of date. > > I've attached the updated patch with the fixed comment and a few other > comments reworded slightly. > > I've also pgindented the patch. > > Barring any objection, I'm planning to push this one in around 10 hours > time. > > David > Hi, + WINDOWAGG_PASSTHROUGH_STRICT /* Pass-through plus don't store new + * tuples during spool */ I think the comment in code is illustrative: + * STRICT pass-through mode is required for the top window + * when there is a PARTITION BY clause. Otherwise we must + * ensure we store tuples that don't match the + * runcondition so they're available to WindowAggs above. If you think the above is too long where WINDOWAGG_PASSTHROUGH_STRICT is defined, maybe point to the longer version so that people can find that more easily. Cheers
