Re: Fix incorrect start up costs for WindowAgg paths (bug #17862)

From: Andy Fan <zhihui(dot)fan1213(at)gmail(dot)com>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Tim Palmer <tim3sp(at)gmail(dot)com>
Subject: Re: Fix incorrect start up costs for WindowAgg paths (bug #17862)
Date: 2023-08-03 06:49:23
Message-ID: CAKU4AWp6E83yMNORaKPpvrLWCd2Ay8mSzdbHvnA6L_z_Z+YfEw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi David:

Sorry for feedback at the last minute! I study the patch and find the
following cases.

1. ORDER BY or PARTITION BY

select *, count(two) over (order by unique1) from tenk1 limit 1;
DEBUG: startup_tuples = 1
DEBUG: startup_tuples = 1

select *, count(two) over (partition by unique1) from tenk1 limit 1;
DEBUG: startup_tuples = 1
DEBUG: startup_tuples = 1

Due to the Executor of nodeWindowAgg, we have to fetch the next tuple
until it mismatches with the current one, then we can calculate the
WindowAgg function. In the current patch, we didn't count the
mismatched tuple. I verified my thought with 'break at IndexNext'
function and see IndexNext is called twice, so in the above case the
startup_tuples should be 2?

2. ORDER BY and PARTITION BY

select two, hundred,
count(two) over (partition by ten order by hundred)
from tenk1 limit 1;

DEBUG: startup_tuples = 10
two | hundred | count
-----+---------+-------
0 | 0 | 100

If we consider the mismatched tuples, it should be 101?

3. As we can see the log for startup_tuples is logged twice sometimes,
the reason is because it is used in cost_windowagg, so it is calculated
for every create_one_window_path. I think the startup_tuples should be
independent with the physical path, maybe we can cache it somewhere to
save some planning cycles?

Thanks for the patch!

--
Best Regards
Andy Fan

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2023-08-03 06:50:28 Re: Faster "SET search_path"
Previous Message Amit Kapila 2023-08-03 06:38:31 Re: [PoC] pg_upgrade: allow to upgrade publisher node