public inbox for [email protected]  

help / color / mirror / Atom feed

From: Richard Guo <[email protected]>
To: David Rowley <[email protected]>
Cc: PostgreSQL Developers <[email protected]>
Subject: Re: Remove WindowClause PARTITION BY items belonging to redundant pathkeys
Date: Fri, 9 Jun 2023 16:57:02 +0800
Message-ID: <CAMbWs4_drpKVckSys8-pZe8OcFSPgoRZHseJ4KP2tfVPePYTog@mail.gmail.com> (raw)
In-Reply-To: <CAApHDvpc=Gu4J47iSzRENGfbxDeYnBhDqzs4_+Z+3LE3iswq0Q@mail.gmail.com>
References: <CAApHDvo2ji+hdxrxfXtRtsfSVw3to2o1nCO20qimw0dUGK8hcQ@mail.gmail.com>
	<CAMbWs49jxZkfSP-9rG0Go4W7=pXCgS7jcjYJEERGJF3SoOFAqg@mail.gmail.com>
	<CAApHDvpc=Gu4J47iSzRENGfbxDeYnBhDqzs4_+Z+3LE3iswq0Q@mail.gmail.com>

On Fri, Jun 9, 2023 at 8:13 AM David Rowley <[email protected]> wrote:

> After looking again at nodeWindowAgg.c, I think it might be possible
> to do a bit more work and apply this to ORDER BY items too.  Without
> an ORDER BY clause, all rows in the partition are peers of each other,
> and if the ORDER BY column is redundant due to belonging to a
> redundant pathkey, then those rows must also be peers too since the
> redundant pathkey must mean all rows have an equal value in the
> redundant column.
>
> However, there is a case where we must be much more careful.  The
> comment you highlighted in create_windowagg_plan() does mention this.
> It reads "we must *not* remove the ordering column for RANGE OFFSET
> cases".


I see.  I tried to run the query below

select a, b, sum(a) over (order by b range between 10 preceding and current
row) from t where b = 2;
server closed the connection unexpectedly

and if we've removed redundant items in wc->orderClause the query would
trigger the Assert in update_frameheadpos().

            /* We must have an ordering column */
            Assert(node->ordNumCols == 1);


>
> It might be possible to make adjustments in nodeWindowAgg.c to have
> the equality checks come out as true when there is no ORDER BY.
> update_frameheadpos() is one location that would need to be adjusted.
> It would need further study to ensure we don't accidentally break
> anything. I've not done that study, so won't be adjusting the patch
> for now.


I'm also not sure if doing that is safe in all cases.  Hmm, do you think
we can instead check wc->frameOptions to see if it is the RANGE OFFSET
case in make_pathkeys_for_window(), and decide to not remove or remove
redundant ORDER BY items according to whether it is or not RANGE OFFSET?

Thanks
Richard

view thread (5+ messages)

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]
  Subject: Re: Remove WindowClause PARTITION BY items belonging to redundant pathkeys
  In-Reply-To: <CAMbWs4_drpKVckSys8-pZe8OcFSPgoRZHseJ4KP2tfVPePYTog@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