Re: Condition pushdown: why (=) is pushed down into join, but BETWEEN or >= is not?

Поиск
Список
Период
Сортировка
От Andy Fan
Тема Re: Condition pushdown: why (=) is pushed down into join, but BETWEEN or >= is not?
Дата
Msg-id CAKU4AWppVGVLLV0GrwNMdYo=rdYUDka5RbeUdC528bSg4s5Dsw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Condition pushdown: why (=) is pushed down into join, but BETWEEN or >= is not?  (Robert Haas <robertmhaas@gmail.com>)
Ответы Re: Condition pushdown: why (=) is pushed down into join, but BETWEEN or >= is not?  (Robert Haas <robertmhaas@gmail.com>)
Список pgsql-hackers


On Fri, Feb 18, 2022 at 4:15 AM Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Feb 1, 2022 at 10:08 AM Andy Fan <zhihui.fan1213@gmail.com> wrote:
> To address the row estimation issue, The most straightforward way to fix this is to
> ignore the derived clauses when figuring out the RelOptInfo->rows on base relation.
> To note which clause is derived from this patch, I added a new field "EquivalenceClass *
> derived" in RestrictInfo. and then added a  included_derived  option in clauselist_selectivity_ext,
> during the set_xxx_rel_size function, we can pass the included_derived=false.  This strategy
> should be used in get_parameterized_baserel_size.   In all the other cases, include_derived=true
> is used. which are finished in commit 2. (Commit 1 is Daivd's patch, I just  rebased it)

That doesn't sound correct to me.

Suppose that we have A.x = B.x and also A.x < 42. We can choose to
enforce A.x < 42 or we can choose to enforce B.x < 42 or we can do
both. In general, any of those could be right:

This is impressive.  To achieve this, we have to treat a.x < 42 and 
b.x < 42 equally rather than b.x < 42 is derived from a.x < 42, 
and enforce the final plan to execute 1 qual in such a group at least.  
This introduces some more complexity at the first glance, but I think 
it is a great aspect to think about. 
 
.., which is good. However, the
row count estimate for B will be too high, because it will not include
the effect of B.x < 42. And that means that the cost estimate for
join(A, B) will be wrong. It will be too high, because it's going to
think that it has more rows coming from the B side of the join than
what is actually the case. And that can also mess up the plan at
higher levels.


IIUC, this would not happen if we apply the commit 3.

In commit 3, the real rows for the scan path are adjusted by RelOptInfo->filter_rows,
which take the effect of B.x < 42.  It is used in cost_{ascan}_path lately. 
Here is an example:

regression=# explain analyze select * from tenk1 a, tenk1 b where a.thousand = b.thousand and a.thousand < 100;
QUERY PLAN
-----------------------------------------------------------------------------------------------------------------------------------------------
Nested Loop  (cost=24.90..459.16 rows=10740 width=488) (actual time=0.416..17.459 rows=10000 loops=1)
->  Bitmap Heap Scan on tenk1 b  (cost=24.61..383.03 rows=1074 width=244) (actual time=0.369..1.801 rows=1000 loops=1)
Recheck Cond: (thousand < 100)
Heap Blocks: exact=324
->  Bitmap Index Scan on tenk1_thous_tenthous  (cost=0.00..24.34 rows=1074 width=0) (actual time=0.251..0.251 rows=1000 loops=1)
Index Cond: (thousand < 100)
->  Memoize  (cost=0.30..0.47 rows=1 width=244) (actual time=0.002..0.006 rows=10 loops=1000)
Cache Key: b.thousand
Cache Mode: logical
Hits: 900  Misses: 100  Evictions: 0  Overflows: 0  Memory Usage: 277kB
->  Index Scan using tenk1_thous_tenthous on tenk1 a  (cost=0.29..0.46 rows=1 width=244) (actual time=0.012..0.033 rows=10 loops=100)
Index Cond: ((thousand = b.thousand) AND (thousand < 100))
Planning Time: 0.934 ms
Execution Time: 18.496 ms
(14 rows)

b.thousand < 100 is derived from a.thousand < 100; and the final path cost is:
->  Bitmap Heap Scan on tenk1 b  (cost=24.61..383.03 rows=1074 width=244) (actual time=0.369..1.801 rows=1000 loops=1)
Recheck Cond: (thousand < 100)
Heap Blocks: exact=324
->  Bitmap Index Scan on tenk1_thous_tenthous  (cost=0.00..24.34 rows=1074 width=0) (actual time=0.251..0.251 rows=1000 loops=1)
Index Cond: (thousand < 100)

Which is exactly same as select * from tenk1 where thousand < 100; 

== Commit 3 [1] message with some modification == 

    Introduce RelOptInfo.filtered_rows.

    Previously the Path.rows (shown in the explain output) and RelOptInfo.rows
    (which would be used to calculating joinrel's estimated rows) are same
    at many scan paths, like SeqScan, IndexScan, BitmapHeapScan and so on. But
    they would be different after distributing a new restrictinfo from ec_filter.
    So I developed RelOptInfo.filtered_rows to take some duty out of RelOptInfo.rows.
    RelOptInfo.filtered_rows would count the effect of derived qual, and be used for
    cost_xxx function. 



On the other hand, I completely agree with David's comments on the
other thread to the effect that holding our breath is not getting us
anywhere. 

+1 with this as well.  PostgreSQL community has an great reputation
in the world,  and mostly because the authority figures in this community
has set up a good example for the following people.  and it is not
easy.  But if the authority figures are too restrict with code quality,  this
is not good for the community as well, we should encourage more people
to have a try,  to some extent. 

Taking the current feature for example,  the estimation issue absolutely 
needs a fix.  While I do listen/think carefully how to reduce planner
extra cost or rethink if any important items are missed by me. I also think
David's method is not unacceptable at all. 

What do you think about moving on this feature?  The items known by me 
are: 1).  Make sure the estimation error can be fixed or discuss if my current
solution is workable.  b).  Just distribute some selectivity restrictinfo to 
RelOptInfo.  c).  See how hard it is to treat the original / derived qual equally.
d).  Reduce the extra planner cost at much as possible.  Any other important
items I missed? 

--
Best Regards
Andy Fan

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Andres Freund
Дата:
Сообщение: Re: Timeout control within tests
Следующее
От: Dilip Kumar
Дата:
Сообщение: Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints