pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: David Rowley <drowley@postgresql.org>
To: pgsql-committers@lists.postgresql.org
Subject: pgsql: Fix possible crash during WindowAgg evaluation
Date: Mon, 09 Dec 2024 01:24:27 +0000
Message-ID: <E1tKSVe-001nJr-W3@gemulon.postgresql.org> (raw)

Fix possible crash during WindowAgg evaluation

When short-circuiting WindowAgg node evaluation on the top-level
WindowAgg node using quals on monotonic window functions, because the
WindowAgg run condition can mean there's no need to evaluate subsequent
window function results in the same partition once the run condition
becomes false, it was possible that the executor would use stale results
from the previous invocation of the window function in some cases.

A fix for this was partially done by a5832722, but that commit only
fixed the issue for non-top-level WindowAgg nodes.  I mistakenly thought
that the top-level WindowAgg didn't have this issue, but Jayesh's example
case clearly shows that's incorrect.  At the time, I also thought that
this only affected 32-bit systems as all window functions which then
supported run conditions returned BIGINT, however, that's wrong as
ExecProject is still called and that could cause evaluation of any other
window function belonging to the same WindowAgg node, one of which may
return a byref type.

The only queries affected by this are WindowAggs with a "Run Condition"
which contains at least one window function with a byref result type,
such as lead() or lag() on a byref column.  The window clause must also
contain a PARTITION BY clause (without a PARTITION BY, execution of the
WindowAgg stops immediately when the run condition becomes false and
there's no risk of using the stale results).

Reported-by: Jayesh Dehankar
Discussion: https://postgr.es/m/193261e2c4d.3dd3cd7c1842.871636075166132237@zohocorp.com
Backpatch-through: 15, where WindowAgg run conditions were added

Branch
------
REL_17_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/9d5ce4f1a00aae95193b9737c4e8ced7f0aa4aaa

Modified Files
--------------
src/backend/executor/nodeWindowAgg.c | 35 +++++++++++++++++------------------
src/test/regress/expected/window.out | 10 ++++++++++
src/test/regress/sql/window.sql      |  7 +++++++
3 files changed, 34 insertions(+), 18 deletions(-)



view thread (4+ messages)  latest in thread

Message-ID: <E1tKSVe-001nJr-W3@gemulon.postgresql.org>
Permalink:  ../E1tKSVe-001nJr-W3@gemulon.postgresql.org/
Also on:    postgresql.org/message-id/E1tKSVe-001nJr-W3@gemulon.postgresql.org

 · 

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: pgsql-committers@postgresql.org
  Cc: drowley@postgresql.org, pgsql-committers@lists.postgresql.org
  Subject: Re: pgsql: Fix possible crash during WindowAgg evaluation
  In-Reply-To: <E1tKSVe-001nJr-W3@gemulon.postgresql.org>

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

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