database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-09 22:41:56 +03:00
Code Activity

Change more places to be less trusting of RestrictInfo.is_pushed_down.

Browse Source 
On further reflection, commit e5d83995e didn't go far enough: pretty much
everywhere in the planner that examines a clause's is_pushed_down flag
ought to be changed to use the more complicated behavior where we also
check the clause's required_relids.  Otherwise we could make incorrect
decisions about whether, say, a clause is safe to use as a hash clause.

Some (many?) of these places are safe as-is, either because they are
never reached while considering a parameterized path, or because there
are additional checks that would reject a pushed-down clause anyway.
However, it seems smarter to just code them all the same way rather
than rely on easily-broken reasoning of that sort.

In support of that, invent a new macro RINFO_IS_PUSHED_DOWN that should
be used in place of direct tests on the is_pushed_down flag.

Like the previous patch, back-patch to all supported branches.

Discussion: https://postgr.es/m/f8128b11-c5bf-3539-48cd-234178b2314d@proxel.se
This commit is contained in:
Tom Lane
2018-04-20 15:19:16 -04:00
parent 68c23cba34
commit c792c7db41
10 changed files with 64 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
src/backend/optimizer/path/costsize.c
View File

@ -159,6 +159,7 @@ static bool has_indexed_join_quals(NestPath *joinpath);
static double approx_tuple_count(PlannerInfo *root, JoinPath *path,
List *quals);
static double calc_joinrel_size_estimate(PlannerInfo *root,
RelOptInfo *joinrel,
RelOptInfo *outer_rel,
RelOptInfo *inner_rel,
double outer_rows,
@ -4055,12 +4056,14 @@ compute_semi_anti_join_factors(PlannerInfo *root,
*/
if (IS_OUTER_JOIN(jointype))
{
Relids joinrelids = bms_union(outerrel->relids, innerrel->relids);
joinquals = NIL;
foreach(l, restrictlist)
{
RestrictInfo *rinfo = lfirst_node(RestrictInfo, l);
if (!rinfo->is_pushed_down)
if (!RINFO_IS_PUSHED_DOWN(rinfo, joinrelids))
joinquals = lappend(joinquals, rinfo);
}
}
@ -4375,6 +4378,7 @@ set_joinrel_size_estimates(PlannerInfo *root, RelOptInfo *rel,
List *restrictlist)
{
rel->rows = calc_joinrel_size_estimate(root,
rel,
outer_rel,
inner_rel,
outer_rel->rows,
@ -4417,6 +4421,7 @@ get_parameterized_joinrel_size(PlannerInfo *root, RelOptInfo *rel,
* estimate for any pair with the same parameterization.
*/
nrows = calc_joinrel_size_estimate(root,
rel,
outer_path->parent,
inner_path->parent,
outer_path->rows,
@ -4440,6 +4445,7 @@ get_parameterized_joinrel_size(PlannerInfo *root, RelOptInfo *rel,
*/
static double
calc_joinrel_size_estimate(PlannerInfo *root,
RelOptInfo *joinrel,
RelOptInfo *outer_rel,
RelOptInfo *inner_rel,
double outer_rows,
@ -4492,7 +4498,7 @@ calc_joinrel_size_estimate(PlannerInfo *root,
{
RestrictInfo *rinfo = lfirst_node(RestrictInfo, l);
if (rinfo->is_pushed_down)
if (RINFO_IS_PUSHED_DOWN(rinfo, joinrel->relids))
pushedquals = lappend(pushedquals, rinfo);
else
joinquals = lappend(joinquals, rinfo);

4
src/backend/optimizer/path/joinpath.c
View File

@ -1700,7 +1700,7 @@ hash_inner_and_outer(PlannerInfo *root,
* If processing an outer join, only use its own join clauses for
* hashing. For inner joins we need not be so picky.
*/
if (isouterjoin && restrictinfo->is_pushed_down)
if (isouterjoin && RINFO_IS_PUSHED_DOWN(restrictinfo, joinrel->relids))
continue;
if (!restrictinfo->can_join ||
@ -1947,7 +1947,7 @@ select_mergejoin_clauses(PlannerInfo *root,
* we don't set have_nonmergeable_joinclause here because pushed-down
* clauses will become otherquals not joinquals.)
*/
if (isouterjoin && restrictinfo->is_pushed_down)
if (isouterjoin && RINFO_IS_PUSHED_DOWN(restrictinfo, joinrel->relids))
continue;
/* Check that clause is a mergeable operator clause */

34
src/backend/optimizer/path/joinrels.c
View File

@ -35,6 +35,7 @@ static bool has_join_restriction(PlannerInfo *root, RelOptInfo *rel);
static bool has_legal_joinclause(PlannerInfo *root, RelOptInfo *rel);
static bool is_dummy_rel(RelOptInfo *rel);
static bool restriction_is_constant_false(List *restrictlist,
RelOptInfo *joinrel,
bool only_pushed_down);
static void populate_joinrel_with_paths(PlannerInfo *root, RelOptInfo *rel1,
RelOptInfo *rel2, RelOptInfo *joinrel,
@ -780,7 +781,7 @@ populate_joinrel_with_paths(PlannerInfo *root, RelOptInfo *rel1,
{
case JOIN_INNER:
if (is_dummy_rel(rel1) || is_dummy_rel(rel2) ||
restriction_is_constant_false(restrictlist, false))
restriction_is_constant_false(restrictlist, joinrel, false))
{
mark_dummy_rel(joinrel);
break;
@ -794,12 +795,12 @@ populate_joinrel_with_paths(PlannerInfo *root, RelOptInfo *rel1,
break;
case JOIN_LEFT:
if (is_dummy_rel(rel1) ||
restriction_is_constant_false(restrictlist, true))
restriction_is_constant_false(restrictlist, joinrel, true))
{
mark_dummy_rel(joinrel);
break;
}
if (restriction_is_constant_false(restrictlist, false) &&
if (restriction_is_constant_false(restrictlist, joinrel, false) &&
bms_is_subset(rel2->relids, sjinfo->syn_righthand))
mark_dummy_rel(rel2);
add_paths_to_joinrel(root, joinrel, rel1, rel2,
@ -811,7 +812,7 @@ populate_joinrel_with_paths(PlannerInfo *root, RelOptInfo *rel1,
break;
case JOIN_FULL:
if ((is_dummy_rel(rel1) && is_dummy_rel(rel2)) ||
restriction_is_constant_false(restrictlist, true))
restriction_is_constant_false(restrictlist, joinrel, true))
{
mark_dummy_rel(joinrel);
break;
@ -847,7 +848,7 @@ populate_joinrel_with_paths(PlannerInfo *root, RelOptInfo *rel1,
bms_is_subset(sjinfo->min_righthand, rel2->relids))
{
if (is_dummy_rel(rel1) || is_dummy_rel(rel2) ||
restriction_is_constant_false(restrictlist, false))
restriction_is_constant_false(restrictlist, joinrel, false))
{
mark_dummy_rel(joinrel);
break;
@ -870,7 +871,7 @@ populate_joinrel_with_paths(PlannerInfo *root, RelOptInfo *rel1,
sjinfo) != NULL)
{
if (is_dummy_rel(rel1) || is_dummy_rel(rel2) ||
restriction_is_constant_false(restrictlist, false))
restriction_is_constant_false(restrictlist, joinrel, false))
{
mark_dummy_rel(joinrel);
break;
@ -885,12 +886,12 @@ populate_joinrel_with_paths(PlannerInfo *root, RelOptInfo *rel1,
break;
case JOIN_ANTI:
if (is_dummy_rel(rel1) ||
restriction_is_constant_false(restrictlist, true))
restriction_is_constant_false(restrictlist, joinrel, true))
{
mark_dummy_rel(joinrel);
break;
}
if (restriction_is_constant_false(restrictlist, false) &&
if (restriction_is_constant_false(restrictlist, joinrel, false) &&
bms_is_subset(rel2->relids, sjinfo->syn_righthand))
mark_dummy_rel(rel2);
add_paths_to_joinrel(root, joinrel, rel1, rel2,
@ -1249,10 +1250,13 @@ mark_dummy_rel(RelOptInfo *rel)
* decide there's no match for an outer row, which is pretty stupid. So,
* we need to detect the case.
*
* If only_pushed_down is true, then consider only pushed-down quals.
* If only_pushed_down is true, then consider only quals that are pushed-down
* from the point of view of the joinrel.
*/
static bool
restriction_is_constant_false(List *restrictlist, bool only_pushed_down)
restriction_is_constant_false(List *restrictlist,
RelOptInfo *joinrel,
bool only_pushed_down)
{
ListCell *lc;
@ -1266,7 +1270,7 @@ restriction_is_constant_false(List *restrictlist, bool only_pushed_down)
{
RestrictInfo *rinfo = lfirst_node(RestrictInfo, lc);
if (only_pushed_down && !rinfo->is_pushed_down)
if (only_pushed_down && !RINFO_IS_PUSHED_DOWN(rinfo, joinrel->relids))
continue;
if (rinfo->clause && IsA(rinfo->clause, Const))
@ -1411,8 +1415,9 @@ try_partitionwise_join(PlannerInfo *root, RelOptInfo *rel1, RelOptInfo *rel2,
* partition keys from given relations being joined.
*/
bool
have_partkey_equi_join(RelOptInfo *rel1, RelOptInfo *rel2, JoinType jointype,
List *restrictlist)
have_partkey_equi_join(RelOptInfo *joinrel,
RelOptInfo *rel1, RelOptInfo *rel2,
JoinType jointype, List *restrictlist)
{
PartitionScheme part_scheme = rel1->part_scheme;
ListCell *lc;
@ -1438,7 +1443,8 @@ have_partkey_equi_join(RelOptInfo *rel1, RelOptInfo *rel2, JoinType jointype,
int ipk2;
/* If processing an outer join, only use its own join clauses. */
if (IS_OUTER_JOIN(jointype) && rinfo->is_pushed_down)
if (IS_OUTER_JOIN(jointype) &&
RINFO_IS_PUSHED_DOWN(rinfo, joinrel->relids))
continue;
/* Skip clauses which can not be used for a join. */