From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
cc: xinwen@stu.scu.edu.cn, pgsql-bugs@lists.postgresql.org,
        David Rowley <david.rowley@2ndquadrant.com>
Subject: Re: BUG #17777: An assert failed in nodeWindowAgg.c
In-reply-to: <20230210230230.scuk6gqrks6c7sc7@awork3.anarazel.de>
References: <17777-860b739b6efde977@postgresql.org>
 <20230210230230.scuk6gqrks6c7sc7@awork3.anarazel.de>
Comments: In-reply-to Andres Freund <andres@anarazel.de>
	message dated "Fri, 10 Feb 2023 15:02:30 -0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <439390.1676070752.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Fri, 10 Feb 2023 18:12:32 -0500
Message-ID: <439391.1676070752@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/439391.1676070752%40sss.pgh.pa.us>
Precedence: bulk

Andres Freund <andres@anarazel.de> writes:
> The problem is that normally we supress the moving aggregate optimizatio=
n if a
> volatile function is contained in the filter. But unfortunately,
> contain_volatile_functions() doesn't descend into subplans. So we don't =
see
> the volatile expression.

I would say that if a volatile function in the argument crashes things,
that's an executor bug.  You won't get any sympathy from me for
complaints about whether contain_volatile_functions noticed that,
because *immutability markings can be lies*.  It is not acceptable
to crash if they're wrong.

It looks to me like maybe we could just remove the Assert and do

-	if (peraggstate->transValueCount =3D=3D 1)
+	if (peraggstate->transValueCount < 2)

a few lines further down?  I've not dug into the details though.

			regards, tom lane