ossc-db / pg_hint_plan

Extension adding support for optimizer hints in PostgreSQL
Other
710 stars 103 forks source link

Row hints doesn't work for TABLE functions #157

Closed dgkimura closed 2 months ago

dgkimura commented 1 year ago

It doesn't seem possible to assign a row hint on table functions. For example:

CREATE FUNCTION my_table_func(int) RETURNS TABLE(f1 int, f2 text)
    AS $$ SELECT $1, CAST($1 AS text) || ' is text' $$
    LANGUAGE SQL;

/*+
    Rows(my_table_func #1)
 */
EXPLAIN SELECT my_table_func(1);

NOTICE:  pg_hint_plan: hint syntax error at or near "
 "
DETAIL:  Rows hint requires at least two relations.

Any reason why we can't or shouldn't allow that?

michaelpq commented 1 year ago

Any reason why we can't or shouldn't allow that?

With a use case and the code to be able to achieve so, I don't see why not. Now it is not something I would put into a stable branch as this would be a new feature. So at this point, assuming that this happens, this would be available with the pg_hint_plan release compatible with PostgreSQL 17.

fvannee commented 1 year ago

I've got a use case, which is related to this, but slightly different. Let me know if I should open a new ticket for this or if we combine them in this ticket. What is the same is the fact that I would like to override the number of rows that are returned by a single relation scan. Currently, the Rows hint requires specifying at least two relations.

Why do I want to do this? In production systems, the most common cause of queries going wrong, is due to the planner making wrong assumptions on the number of rows (because often the statistics that you have are not good enough). Where I most often see "cliff-like" behavior, is when the planner estimates "1 row" to be returned from scanning a relation, whereas in reality there are many more rows. Often, this is due to specifying multiple WHERE-clauses which are somewhat correlated, but the planner grossly underestimating the result. The correlated statistics don't always help here. What this leads to is the planner picking a Nested Loop, because rows=1 means it'd only execute the outer part once. In reality, this outer part then gets executed many times which makes it very slow.

A very simple example would be something like the following. It's not perfect, mostly as this most often happens in very complex queries with many joins, but I hope it illustrates the problem.

create table t1 (a int, b int, c int);
create index on t1 (a);
insert into t1 select a, b, c from generate_series(1, 10) a, generate_series(1, 1000) b, generate_series(1, 10) c;
insert into t1 values (1001, 10, 10), (1011, 10, 10);
analyze t1;

explain(analyze)
select * from t1
join t1 tt on tt.a=t1.a
where t1.a>=1001 and t1.b=10;

Nested Loop  (cost=0.58..786.64 rows=9999 width=24) (actual time=0.014..0.039 rows=72 loops=1)
  ->  Index Scan using t1_a_idx on t1  (cost=0.29..14.42 rows=1 width=12) (actual time=0.010..0.012 rows=12 loops=1)
        Index Cond: (a >= 1001)
        Filter: (b = 10)
  ->  Index Scan using t1_a_idx on t1 tt  (cost=0.29..688.87 rows=8334 width=12) (actual time=0.001..0.001 rows=6 loops=12)
        Index Cond: (a = t1.a)
Planning Time: 0.100 ms
Execution Time: 0.056 ms

The rows estimation for t1 is very off here (rows=1 estimated, rows=12 real) and I'd like to correct that. In this case the nested loop is actually still the fastest plan, so it's not an idea example :( but I hope the idea is clear that this misestimation leads it to pick a nested loop, whereas often it should actually pick a hash join or something similar. Now you could say: why don't you just override it and tell it to use a hash join with pg_hint? I believe, especially in more complex plans with many possible jobs, it's better to correct the planner as close as possible to the source. And in this case, that means correcting the number of rows. I trust the planner to do the right thing afterwards, as most of the time, if it has the right statistics, it actually makes very sensible choices. It just doesn't have the right statistics all the time.

michaelpq commented 2 months ago

@dgkimura If you are planning to see this feature into the version of pg_hint_plan planned for PostgreSQL 17, I would suggest to send a patch shortly so as we could discuss the implementation of a Row hint to force the number of rows for a single scan.

For now, you could as well force the hand of the planner in the definition of the function itself with ROWS, like your first example which was a SRF?

I am closing the ticket for now. If you have a proposal of patch later, feel free to open a pull request.