Calculation of param_source_rels in add_paths_to_joinrel

From: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Calculation of param_source_rels in add_paths_to_joinrel
Date: 2016-09-20 06:11:21
Message-ID: CAFjFpRcLnMyX2ObxuPxVRu0Bm+8J6PoFS_N7f5yC8O+aF9pLcg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

Hi,
There's code in add_paths_to_joinrel() which computes the set of
target relations that should overlap parameterization of any proposed
join path.

120 /*
121 * Decide whether it's sensible to generate parameterized
paths for this
122 * joinrel, and if so, which relations such paths should
require. There
123 * is usually no need to create a parameterized result path
unless there
124 * is a join order restriction that prevents joining one of
our input rels
125 * directly to the parameter source rel instead of joining to the other
126 * input rel. (But see allow_star_schema_join().) This restriction
127 * reduces the number of parameterized paths we have to deal with at
128 * higher join levels, without compromising the quality of
the resulting
129 * plan. We express the restriction as a Relids set that
must overlap the
130 * parameterization of any proposed join path.
131 */

The calculations that follow are based on joinrel->relids (baserels
covered by the join) and SpecialJoinInfo list in PlannerInfo. It is
not based on specific combination of relations being joined or the
paths being generated. We should probably do this computation once and
store the result in the joinrel and use it multiple times. That way we
can avoid computing the same set again and again for every pair of
joining relations and their order. Any reasons why we don't do this?

Attached patch moves this code to build_join_rel() and uses it in
add_paths_to_joinrel(). make check-world does not show any failures.

If this change is acceptable, we might actually remove
param_source_rels from JoinPathExtraData and directly access it from
joinrel in try_nestloop_path(), try_mergejoin_path() and
try_hashjoin_path().

Also, the way this code has been written, the declaration of variable
sjinfo masks the earlier declaration with the same name. I am not sure
if that's intentional, but may be we should use another variable name
for the inner sjinfo. I have not included that change in the patch.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Attachment Content-Type Size
pg_param_source_rels.patch invalid/octet-stream 10.9 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tsunakawa, Takayuki 2016-09-20 06:20:37 [RFC] Should we fix postmaster to avoid slow shutdown?
Previous Message Amit Kapila 2016-09-20 05:21:15 Re: WIP: Covering + unique indexes.