MIME-Version: 1.0
References: <CAO6_Xqr9+51NxgO=XospEkUeAg-p=EjAWmtpdcZwjRgGKJ53iA@mail.gmail.com>
 <CA+FpmFfaCbKy1D9r2cr90oKXRwDiat4mKWFVnT5b82xm8xidRw@mail.gmail.com> <CAO6_XqqQRSfZms8JRmOnikC_e8zJZuWPqzEezw_9HVyUw5bhUw@mail.gmail.com>
In-Reply-To: <CAO6_XqqQRSfZms8JRmOnikC_e8zJZuWPqzEezw_9HVyUw5bhUw@mail.gmail.com>
From: Richard Guo <guofenglinux@gmail.com>
Date: Wed, 17 Jul 2024 09:59:05 +0800
Message-ID: <CAMbWs4-ZkH9t40LH8LMyZUuqbBww1k9OD+CH+O_7LJ7TwP3Zhw@mail.gmail.com>
Subject: Re: Possible incorrect row estimation for Gather paths
To: Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com>
Cc: Rafia Sabih <rafia.pghackers@gmail.com>, 
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAMbWs4-ZkH9t40LH8LMyZUuqbBww1k9OD%2BCH%2BO_7LJ7TwP3Zhw%40mail.gmail.com>
Precedence: bulk

I can reproduce this problem with the query below.

explain (costs on) select * from tenk1 order by twenty;
                                   QUERY PLAN
---------------------------------------------------------------------------=
------
 Gather Merge  (cost=3D772.11..830.93 rows=3D5882 width=3D244)
   Workers Planned: 1
   ->  Sort  (cost=3D772.10..786.80 rows=3D5882 width=3D244)
         Sort Key: twenty
         ->  Parallel Seq Scan on tenk1  (cost=3D0.00..403.82 rows=3D5882 w=
idth=3D244)
(5 rows)

On Tue, Jul 16, 2024 at 3:56=E2=80=AFPM Anthonin Bonnefoy
<anthonin.bonnefoy@datadoghq.com> wrote:
> The initial goal was to use the source tuples if available and avoid
> possible rounding errors. Though I realise that the difference would
> be minimal. For example, 200K tuples and 3 workers would yield
> int(int(200000 / 2.4) * 2.4)=3D199999. That is probably not worth the
> additional complexity, I've updated the patch to just use
> gather_rows_estimate.

I wonder if the changes in create_ordered_paths should also be reduced
to 'total_groups =3D gather_rows_estimate(path);'.

> I've also realised from the comments in optimizer.h that
> nodes/pathnodes.h should not be included there and fixed it.

I think perhaps it's better to declare gather_rows_estimate() in
cost.h rather than optimizer.h.
(BTW, I wonder if compute_gather_rows() would be a better name?)

I noticed another issue in generate_useful_gather_paths() -- *rowsp
would have a random value if override_rows is true and we use
incremental sort for gather merge.  I think we should fix this too.

Thanks
Richard