Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1ncaEN-0006Ut-Oo for pgsql-hackers@arkaria.postgresql.org; Thu, 07 Apr 2022 22:03:55 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1ncaEM-0006Mm-Jn for pgsql-hackers@arkaria.postgresql.org; Thu, 07 Apr 2022 22:03:54 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1ncaEM-0006Md-5V for pgsql-hackers@lists.postgresql.org; Thu, 07 Apr 2022 22:03:54 +0000 Received: from mail-lj1-x232.google.com ([2a00:1450:4864:20::232]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1ncaEH-00035r-P4 for pgsql-hackers@lists.postgresql.org; Thu, 07 Apr 2022 22:03:53 +0000 Received: by mail-lj1-x232.google.com with SMTP id bn33so9196328ljb.6 for ; Thu, 07 Apr 2022 15:03:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yugabyte.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=cyuhsjwzJW3g5rxvA8o7lyJ9fzm5MVWGwhWNoxwS9gg=; b=Dxbq3hKNEee1wUfro05wskWbm/6gAxHp6TTYezNI3nf309LVBY7EukMWGH8KEyxUqR nEeClZDC6DQY9+FaOnKeFj5ktFNG8Z6gr8F9jg8+AOd9XPrjcMB6JKLB/JHk6ih8XOtW uz8DXBXWavpv8aqQt4Y5GAbN4yucfa0lnDQkQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=cyuhsjwzJW3g5rxvA8o7lyJ9fzm5MVWGwhWNoxwS9gg=; b=qfOmKCSxTtJmfhWcfgGth2Q1aNY1uNeQBwkuvBrhEyKf5ytbg8yTME+b9YKTMLI11j 3qanw5NHQvFC61l7y9W/x3tGCEflX3N0Ntnn333ZvXcNSxJU+kk5oH+u9dK487E8S8hj l29Mh9JXfmIlAdmXrfvAzp2GPzREWzMYLPjg2b4/zZt1+ZuyBstcdcLcIcIdFTcPKdHL A3DHICu14ouADNCPr0b/ooQvvpXnWYmgsJ44Aq+fywmpLX/yQFdNBX+/Lc5G6VwGzXt8 kTlxhurxbf+Ytg/mduH5mOwtqMiKYyT7xed69N4mUQRp/9asOA04yN6A10QiPsXjIugG su3Q== X-Gm-Message-State: AOAM532GoAbPekPAP9IiytxKT8ypR3Fw6X9Ar6iRO+E+p/OkuMOX/rTf WRfAdcIcyua5zoAnN+mB76ol2d+oux9ZVy+Q3U+psA== X-Google-Smtp-Source: ABdhPJzodFjAhJbJzqB+NAsYC9Cyb0nSzX3yF9riPuNE68bYnjxg6YA5sv6ceraFzjywPv0Y4YKS1Zk1rwCpPcHNSZM= X-Received: by 2002:a2e:9ac5:0:b0:24b:1263:d780 with SMTP id p5-20020a2e9ac5000000b0024b1263d780mr9737779ljj.384.1649369027618; Thu, 07 Apr 2022 15:03:47 -0700 (PDT) MIME-Version: 1.0 References: <20220329221615.hule724qcaarhe3y@alap3.anarazel.de> In-Reply-To: From: Zhihong Yu Date: Thu, 7 Apr 2022 15:08:10 -0700 Message-ID: Subject: Re: Window Function "Run Conditions" To: David Rowley Cc: Andy Fan , Andres Freund , PostgreSQL Developers Content-Type: multipart/alternative; boundary="000000000000fec39d05dc17a6fd" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --000000000000fec39d05dc17a6fd Content-Type: text/plain; charset="UTF-8" On Thu, Apr 7, 2022 at 7:11 AM David Rowley wrote: > On Thu, 7 Apr 2022 at 19:01, David Rowley wrote: > > > > On Thu, 7 Apr 2022 at 15:41, Zhihong Yu 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 --000000000000fec39d05dc17a6fd Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Thu, Apr 7, 2022 at 7:11 AM David = Rowley <dgrowleyml@gmail.com= > wrote:
On T= hu, 7 Apr 2022 at 19:01, David Rowley <dgrowleyml@gmail.com> wrote:
>
> On Thu, 7 Apr 2022 at 15:41, Zhihong Yu <zyu@yugabyte.com> wrote:
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 * We mus= t keep the original qual in place if there is a
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 * PARTIT= ION BY clause as the top-level WindowAgg remains in
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 * pass-t= hrough mode and does nothing to filter out unwanted
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 * tuples= .
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 */
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*keep_ori= ginal =3D 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 assignm= ent.
>
> Thanks. I've just removed that comment locally now. You're rig= ht, it
> was out of date.

I've attached the updated patch with the fixed comment and a few other<= br> 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,

+ =C2=A0 WINDOWAGG_PASST= HROUGH_STRICT =C2=A0 =C2=A0/* Pass-through plus don't store new
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* tuples during spoo= l */

I think the comment in code is illustrative:<= /div>

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0* STRICT pass-through mode is required for the top window<= br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*= when there is a PARTITION BY clause.=C2=A0 Otherwise we must
+ =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* ensure we s= tore tuples that don't match the
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* runcondition so they're avai= lable to WindowAggs above.=C2=A0

If you think the = above is too long where WINDOWAGG_PASSTHROUGH_STRICT is defined, maybe poin= t to the longer version so that people can find that more easily.

Cheers
--000000000000fec39d05dc17a6fd--