Skip to content

Commit

Permalink
Fix handling of outer-join relids
Browse files Browse the repository at this point in the history
find_join_hint() calculates the join level using joinrelids which
is a bitmapset of relations.  However, since the upstream commit
2489d76c introduced in 16, it is possible that these include outer-join
relids, something that pg_hint_plan cannot digest because it expects
only case relations.

This commit filters out any outer-join relids from the lists of relids
considered during the join calculations.  This could cause crashes
because of corrupted relids lists.

There may be a smarter way to fix that, but this is proving to be good
enough for now.

Per issue #139.

Reported-by: Michael Paquier
Author: Masahiro Ikeda
Backpatch-through: 16
  • Loading branch information
michaelpq committed Aug 30, 2023
1 parent 219b658 commit dee15ec
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 0 deletions.
16 changes: 16 additions & 0 deletions expected/pg_hint_plan.out
Original file line number Diff line number Diff line change
Expand Up @@ -9131,6 +9131,22 @@ set max_parallel_workers_per_gather to DEFAULT;
-> Seq Scan on t3 (cost=xxx..xxx rows=100 width=xxx)

\! rm results/pg_hint_plan.tmpout
-- Query with join RTE and outer-join relids
/*+Leading(ft_1 ft_2 t1)*/
SELECT relname, seq_scan > 0 AS seq_scan, idx_scan > 0 AS idx_scan
FROM pg_stat_user_tables WHERE schemaname = 'public' AND relname = 't1';
LOG: pg_hint_plan:
used hint:
not used hint:
Leading(ft_1 ft_2 t1)
duplication hint:
error hint:

relname | seq_scan | idx_scan
---------+----------+----------
t1 | f | f
(1 row)

-- hint error level
set client_min_messages to 'DEBUG1';
/*+ SeqScan( */ SELECT 1;
Expand Down
20 changes: 20 additions & 0 deletions pg_hint_plan.c
Original file line number Diff line number Diff line change
Expand Up @@ -4177,6 +4177,12 @@ OuterInnerJoinCreate(OuterInnerRels *outer_inner, LeadingHint *leading_hint,

join_relids = bms_add_members(outer_relids, inner_relids);

/*
* join_relids may include outer-join relids since PostgreSQL 16, so
* filter them out as hints can only handle base relations.
*/
join_relids = bms_intersect(join_relids, root->all_baserels);

if (bms_num_members(join_relids) > nbaserel)
return join_relids;

Expand Down Expand Up @@ -4530,6 +4536,13 @@ make_join_rel_wrapper(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2)
int save_nestlevel;

joinrelids = bms_union(rel1->relids, rel2->relids);

/*
* joinrelids may include outer-join relids since PostgreSQL 16, so
* filter them out as hints can only handle base relations.
*/
joinrelids = bms_intersect(joinrelids, root->all_baserels);

join_hint = find_join_hint(joinrelids);
memoize_hint = find_memoize_hint(joinrelids);
bms_free(joinrelids);
Expand Down Expand Up @@ -4597,6 +4610,13 @@ add_paths_to_joinrel_wrapper(PlannerInfo *root,
int save_nestlevel;

joinrelids = bms_union(outerrel->relids, innerrel->relids);

/*
* joinrelids may include outer-join relids since PostgreSQL 16, so
* filter them out as hints can only handle base relations.
*/
joinrelids = bms_intersect(joinrelids, root->all_baserels);

join_hint = find_join_hint(joinrelids);
memoize_hint = find_memoize_hint(joinrelids);
bms_free(joinrelids);
Expand Down
5 changes: 5 additions & 0 deletions sql/pg_hint_plan.sql
Original file line number Diff line number Diff line change
Expand Up @@ -1159,6 +1159,11 @@ set max_parallel_workers_per_gather to DEFAULT;
\! sql/maskout.sh results/pg_hint_plan.tmpout
\! rm results/pg_hint_plan.tmpout

-- Query with join RTE and outer-join relids
/*+Leading(ft_1 ft_2 t1)*/
SELECT relname, seq_scan > 0 AS seq_scan, idx_scan > 0 AS idx_scan
FROM pg_stat_user_tables WHERE schemaname = 'public' AND relname = 't1';

-- hint error level
set client_min_messages to 'DEBUG1';
/*+ SeqScan( */ SELECT 1;
Expand Down

0 comments on commit dee15ec

Please sign in to comment.