Re: pg16: XX000: could not find pathkey item to sort - Mailing list pgsql-hackers

From Richard Guo
Subject Re: pg16: XX000: could not find pathkey item to sort
Date
Msg-id CAMbWs480F9sTNdgcpTYKFa-sLaOQ0kq=6=T5pz2yXWWQoK-pmg@mail.gmail.com
Whole thread Raw
In response to Re: pg16: XX000: could not find pathkey item to sort  (David Rowley <dgrowleyml@gmail.com>)
List pgsql-hackers

On Mon, Oct 9, 2023 at 7:42 AM David Rowley <dgrowleyml@gmail.com> wrote:
On Sun, 8 Oct 2023 at 23:52, Richard Guo <guofenglinux@gmail.com> wrote:
> If the pathkeys that were added by adjust_group_pathkeys_for_groupagg()
> are computable from the targetlist, it seems that we do not need to trim
> them off, because prepare_sort_from_pathkeys() will add resjunk target
> entries for them.  But it's also no harm if we trim them off.  So I
> think the patch is a pretty safe fix.  +1 to it.

hmm, I think one of us does not understand what is going on here. I
tried to explain in [1] why we *need* to strip off the pathkeys added
by adjust_group_pathkeys_for_groupagg().

Sorry I didn't make myself clear.  I understand why we need to trim off
the pathkeys added by adjust_group_pathkeys_for_groupagg().  What I
meant was that if the new added pathkeys are *computable* from the
existing target entries, then prepare_sort_from_pathkeys() will add
resjunk target entries for them, so there seems to be no problem even if
we do not trim them off.  For example

explain (verbose, costs off)
select a, count(distinct a+1) from prt1 group by a order by a;
                                     QUERY PLAN
------------------------------------------------------------------------------------
 Result
   Output: prt1.a, (count(DISTINCT ((prt1.a + 1))))
   ->  Merge Append
         Sort Key: prt1.a, ((prt1.a + 1))
         ->  GroupAggregate
               Output: prt1.a, count(DISTINCT ((prt1.a + 1))), ((prt1.a + 1))
               Group Key: prt1.a
               ->  Sort
                     Output: prt1.a, ((prt1.a + 1))
                     Sort Key: prt1.a, ((prt1.a + 1))
                     ->  Seq Scan on public.prt1_p1 prt1
                           Output: prt1.a, (prt1.a + 1)
    ...

Expression 'a+1' is *computable* from the existing entry 'a', so we just
add a new resjunk target entry for 'a+1', and there is no error planning
this query.  But if we change 'a+1' to something that is not computable,
then we would have problems (without your fix), and the reason has been
well explained by your messages.

explain (verbose, costs off)
select a, count(distinct b) from prt1 group by a order by a;
ERROR:  could not find pathkey item to sort

Having said that, I think it's the right thing to do to trim off the new
added pathkeys, even if they are *computable*.  In the plan above, the
'(prt1.a + 1)' in GroupAggregate's targetlist and MergeAppend's
pathkeys are actually redundant.  It's good to remove it.
 
Can you explain why you think we can put a resjunk "b" in the target
list of the GroupAggregate in the above case?

Hmm, I don't think we can do that, because 'b' is not *computable* from
the existing target entries, as I explained above.

Thanks
Richard

pgsql-hackers by date:

Previous
From: Erik Wienhold
Date:
Subject: Re: Fix output of zero privileges in psql
Next
From: Noah Misch
Date:
Subject: Re: REL_15_STABLE: pgbench tests randomly failing on CI, Windows only