Horton

Thread

Commits

  1. Adjust tuples estimate for appendrels

  1. Adjust tuples estimate for appendrels

    Richard Guo <guofenglinux@gmail.com> — 2025-01-27T08:05:50Z

    (This was briefly discussed in [1], which primarily focuses on the
incremental sort regression.  So start a new thread for this topic.)

In set_append_rel_size(), we currently set rel->tuples to rel->rows
for an appendrel.  Generally, rel->tuples is the raw number of tuples
in the relation and rel->rows is the estimated number of tuples after
the relation's restriction clauses have been applied.  Although an
appendrel itself doesn't directly enforce any quals today, its child
relations may.  Therefore, setting rel->tuples equal to rel->rows for
an appendrel isn't always appropriate.

AFAICS, doing so can lead to issues in cost estimates.  For instance,
when estimating the number of distinct values from an appendrel, we
would not be able to adjust the estimate based on the restriction
selectivity (see estimate_num_groups()).

Attached is a patch that sets an appendrel's tuples to the total
number of tuples accumulated from each live child, which I believe
aligns better with reality.

Here's a simple example that demonstrates how this change improves
cost estimates in certain cases.

create table p (a int, b int, c float) partition by range(a);
create table p1 partition of p for values from (0) to (1000);
create table p2 partition of p for values from (1000) to (2000);

insert into p select i%2000, random(1, 100000), random(1, 100000) from
generate_series(1, 1000000)i;

analyze p;

explain analyze select b from p where c < 10000 group by b;

-- without this patch
 HashAggregate  (cost=18651.38..19568.54 rows=91716 width=4)
                (actual time=467.859..487.227 rows=63346 loops=1)

-- with this patch
 HashAggregate  (cost=18651.38..19275.60 rows=62422 width=4)
                (actual time=447.383..466.351 rows=63346 loops=1)

Unfortunately, I failed to come up with a stable test case that shows
a plan diff with this change.  So the attached patch does not include
a test case for now.

BTW, the comment within set_append_rel_size() says that:

   /*
    * Set "raw tuples" count equal to "rows" for the appendrel; needed
    * because some places assume rel->tuples is valid for any baserel.
    */

I wonder if this assumption still holds today.  If we create an empty
table, analyze it, and then use it in a query, the table will have
rel->tuples set to zero and rel->rows set to one, which doesn't cause
any issues today.

[1] https://postgr.es/m/CAMbWs4-ocromEKMtVDH3RBMuAJQaQDK0qi4k6zOuvpOnMWZauw@mail.gmail.com

Thanks
Richard

  2. Re: Adjust tuples estimate for appendrels

    Tender Wang <tndrwang@gmail.com> — 2025-01-27T13:40:15Z

    Richard Guo <guofenglinux@gmail.com> 于2025年1月27日周一 16:06写道：

> (This was briefly discussed in [1], which primarily focuses on the
> incremental sort regression.  So start a new thread for this topic.)
>
> In set_append_rel_size(), we currently set rel->tuples to rel->rows
> for an appendrel.  Generally, rel->tuples is the raw number of tuples
> in the relation and rel->rows is the estimated number of tuples after
> the relation's restriction clauses have been applied.  Although an
> appendrel itself doesn't directly enforce any quals today, its child
> relations may.  Therefore, setting rel->tuples equal to rel->rows for
> an appendrel isn't always appropriate.
>
> AFAICS, doing so can lead to issues in cost estimates.  For instance,
> when estimating the number of distinct values from an appendrel, we
> would not be able to adjust the estimate based on the restriction
> selectivity (see estimate_num_groups()).
>
> Attached is a patch that sets an appendrel's tuples to the total
> number of tuples accumulated from each live child, which I believe
> aligns better with reality.
>

I basically agree with you.  I just have a little advice.
Can we give some explanation about why we do this adjust.
For example, adding below sentence you said above:
"doing so can lead to issues in cost estimates.  For instance,
when estimating the number of distinct values from an appendrel, we
would not be able to adjust the estimate based on the restriction
selectivity (see estimate_num_groups())."

Others looks good to me.


>
> Here's a simple example that demonstrates how this change improves
> cost estimates in certain cases.
>
> create table p (a int, b int, c float) partition by range(a);
> create table p1 partition of p for values from (0) to (1000);
> create table p2 partition of p for values from (1000) to (2000);
>
> insert into p select i%2000, random(1, 100000), random(1, 100000) from
> generate_series(1, 1000000)i;
>
> analyze p;
>
> explain analyze select b from p where c < 10000 group by b;
>
> -- without this patch
>  HashAggregate  (cost=18651.38..19568.54 rows=91716 width=4)
>                 (actual time=467.859..487.227 rows=63346 loops=1)
>
> -- with this patch
>  HashAggregate  (cost=18651.38..19275.60 rows=62422 width=4)
>                 (actual time=447.383..466.351 rows=63346 loops=1)
>
> Unfortunately, I failed to come up with a stable test case that shows
> a plan diff with this change.  So the attached patch does not include
> a test case for now.
>

Yeah, it seems not easy to write test case to reflect this change. We
usually set costs off in EXPLAIN.
I think it's ok without a test case.


> BTW, the comment within set_append_rel_size() says that:
>
>    /*
>     * Set "raw tuples" count equal to "rows" for the appendrel; needed
>     * because some places assume rel->tuples is valid for any baserel.
>     */
>
> I wonder if this assumption still holds today.  If we create an empty
> table, analyze it, and then use it in a query, the table will have
> rel->tuples set to zero and rel->rows set to one, which doesn't cause
> any issues today.
>
> [1]
> https://postgr.es/m/CAMbWs4-ocromEKMtVDH3RBMuAJQaQDK0qi4k6zOuvpOnMWZauw@mail.gmail.com
>
> Thanks
> Richard
>


-- 
Thanks,
Tender Wang

  3. Re: Adjust tuples estimate for appendrels

    Alena Rybakina <a.rybakina@postgrespro.ru> — 2025-01-27T17:52:34Z

    Hi!

On 27.01.2025 11:05, Richard Guo wrote:
> (This was briefly discussed in [1], which primarily focuses on the
> incremental sort regression.  So start a new thread for this topic.)
>
> In set_append_rel_size(), we currently set rel->tuples to rel->rows
> for an appendrel.  Generally, rel->tuples is the raw number of tuples
> in the relation and rel->rows is the estimated number of tuples after
> the relation's restriction clauses have been applied.  Although an
> appendrel itself doesn't directly enforce any quals today, its child
> relations may.  Therefore, setting rel->tuples equal to rel->rows for
> an appendrel isn't always appropriate.
>
> AFAICS, doing so can lead to issues in cost estimates.  For instance,
> when estimating the number of distinct values from an appendrel, we
> would not be able to adjust the estimate based on the restriction
> selectivity (see estimate_num_groups()).
>
> Attached is a patch that sets an appendrel's tuples to the total
> number of tuples accumulated from each live child, which I believe
> aligns better with reality.
>
> Here's a simple example that demonstrates how this change improves
> cost estimates in certain cases.
>
> create table p (a int, b int, c float) partition by range(a);
> create table p1 partition of p for values from (0) to (1000);
> create table p2 partition of p for values from (1000) to (2000);
>
> insert into p select i%2000, random(1, 100000), random(1, 100000) from
> generate_series(1, 1000000)i;
>
> analyze p;
>
> explain analyze select b from p where c < 10000 group by b;
>
> -- without this patch
>   HashAggregate  (cost=18651.38..19568.54 rows=91716 width=4)
>                  (actual time=467.859..487.227 rows=63346 loops=1)
>
> -- with this patch
>   HashAggregate  (cost=18651.38..19275.60 rows=62422 width=4)
>                  (actual time=447.383..466.351 rows=63346 loops=1)
I looked at it and agree with your solution.
>
> Unfortunately, I failed to come up with a stable test case that shows
> a plan diff with this change.  So the attached patch does not include
> a test case for now.

I created a stable test:

create table p (a int, b int, c float) partition by range(a);
create table p1 partition of p for values from (0) to (100);
create table p2 partition of p for values from (100) to (1000);
insert into p select i%200, i%300, i%400from
generate_series(1, 1000)i;
analyze p;
SELECT * FROM check_estimated_rows('select b from p where c < 10 group 
by b');
estimated | actual
-----------+--------
27| 29
(1row)
drop table p;
I added it in the diff file.

-- 
Regards,
Alena Rybakina
Postgres Professional

  4. Re: Adjust tuples estimate for appendrels

    Richard Guo <guofenglinux@gmail.com> — 2025-02-10T08:22:22Z

    Thanks for all the reviews.  Attached is an updated patch that
resolves the review comments.

Thanks
Richard

  5. Re: Adjust tuples estimate for appendrels

    Richard Guo <guofenglinux@gmail.com> — 2025-02-17T02:18:24Z

    On Mon, Feb 10, 2025 at 5:22 PM Richard Guo <guofenglinux@gmail.com> wrote:
> Thanks for all the reviews.  Attached is an updated patch that
> resolves the review comments.

Pushed.  Thanks for all the reviews.

Thanks
Richard