Skip to content

Commit 7a39b5e

Browse files
robertmhaasrobertmhaas
committed
Abstract logic to allow for multiple kinds of child rels.
Currently, the only type of child relation is an "other member rel", which is the child of a baserel, but in the future joins and even upper relations may have child rels. To facilitate that, introduce macros that test to test for particular RelOptKind values, and use them in various places where they help to clarify the sense of a test. (For example, a test may allow RELOPT_OTHER_MEMBER_REL either because it intends to allow child rels, or because it intends to allow simple rels.) Also, remove find_childrel_top_parent, which will not work for a child rel that is not a baserel. Instead, add a new RelOptInfo member top_parent_relids to track the same kind of information in a more generic manner. Ashutosh Bapat, slightly tweaked by me. Review and testing of the patch set from which this was taken by Rajkumar Raghuwanshi and Rafia Sabih. Discussion: http://postgr.es/m/CA+TgmoagTnF2yqR3PT2rv=om=wJiZ4-A+ATwdnriTGku1CLYxA@mail.gmail.com
1 parent 93cd768 commit 7a39b5e

File tree

16 files changed

+104
-90
lines changed

16 files changed

+104
-90
lines changed

‎contrib/postgres_fdw/deparse.c

Lines changed: 16 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ is_foreign_expr(PlannerInfo *root,
243243
* because the upperrel's own relids currently aren't set to anything
244244
* meaningful by the core code. For other relation, use their own relids.
245245
*/
246-
if (baserel->reloptkind == RELOPT_UPPER_REL)
246+
if (IS_UPPER_REL(baserel))
247247
glob_cxt.relids = fpinfo->outerrel->relids;
248248
else
249249
glob_cxt.relids = baserel->relids;
@@ -677,7 +677,7 @@ foreign_expr_walker(Node *node,
677677
ListCell *lc;
678678

679679
/* Not safe to pushdown when not in grouping context */
680-
if (glob_cxt->foreignrel->reloptkind != RELOPT_UPPER_REL)
680+
if (!IS_UPPER_REL(glob_cxt->foreignrel))
681681
return false;
682682

683683
/* Only non-split aggregates are pushable. */
@@ -874,7 +874,7 @@ build_tlist_to_deparse(RelOptInfo *foreignrel)
874874
* For an upper relation, we have already built the target list while
875875
* checking shippability, so just return that.
876876
*/
877-
if (foreignrel->reloptkind == RELOPT_UPPER_REL)
877+
if (IS_UPPER_REL(foreignrel))
878878
return fpinfo->grouped_tlist;
879879

880880
/*
@@ -929,17 +929,13 @@ deparseSelectStmtForRel(StringInfo buf, PlannerInfo *root, RelOptInfo *rel,
929929
* We handle relations for foreign tables, joins between those and upper
930930
* relations.
931931
*/
932-
Assert(rel->reloptkind == RELOPT_JOINREL ||
933-
rel->reloptkind == RELOPT_BASEREL ||
934-
rel->reloptkind == RELOPT_OTHER_MEMBER_REL ||
935-
rel->reloptkind == RELOPT_UPPER_REL);
932+
Assert(IS_JOIN_REL(rel) || IS_SIMPLE_REL(rel) || IS_UPPER_REL(rel));
936933

937934
/* Fill portions of context common to upper, join and base relation */
938935
context.buf = buf;
939936
context.root = root;
940937
context.foreignrel = rel;
941-
context.scanrel = (rel->reloptkind == RELOPT_UPPER_REL) ?
942-
fpinfo->outerrel : rel;
938+
context.scanrel = IS_UPPER_REL(rel) ? fpinfo->outerrel : rel;
943939
context.params_list = params_list;
944940

945941
/* Construct SELECT clause */
@@ -950,7 +946,7 @@ deparseSelectStmtForRel(StringInfo buf, PlannerInfo *root, RelOptInfo *rel,
950946
* conditions of the underlying scan relation; otherwise, we can use the
951947
* supplied list of remote conditions directly.
952948
*/
953-
if (rel->reloptkind == RELOPT_UPPER_REL)
949+
if (IS_UPPER_REL(rel))
954950
{
955951
PgFdwRelationInfo *ofpinfo;
956952

@@ -963,7 +959,7 @@ deparseSelectStmtForRel(StringInfo buf, PlannerInfo *root, RelOptInfo *rel,
963959
/* Construct FROM and WHERE clauses */
964960
deparseFromExpr(quals, &context);
965961

966-
if (rel->reloptkind == RELOPT_UPPER_REL)
962+
if (IS_UPPER_REL(rel))
967963
{
968964
/* Append GROUP BY clause */
969965
appendGroupByClause(tlist, &context);
@@ -1020,8 +1016,7 @@ deparseSelectSql(List *tlist, bool is_subquery, List **retrieved_attrs,
10201016
*/
10211017
deparseSubqueryTargetList(context);
10221018
}
1023-
else if (foreignrel->reloptkind == RELOPT_JOINREL ||
1024-
foreignrel->reloptkind == RELOPT_UPPER_REL)
1019+
else if (IS_JOIN_REL(foreignrel) || IS_UPPER_REL(foreignrel))
10251020
{
10261021
/*
10271022
* For a join or upper relation the input tlist gives the list of
@@ -1062,9 +1057,8 @@ deparseFromExpr(List *quals, deparse_expr_cxt *context)
10621057
RelOptInfo *scanrel = context->scanrel;
10631058

10641059
/* For upper relations, scanrel must be either a joinrel or a baserel */
1065-
Assert(context->foreignrel->reloptkind != RELOPT_UPPER_REL ||
1066-
scanrel->reloptkind == RELOPT_JOINREL ||
1067-
scanrel->reloptkind == RELOPT_BASEREL);
1060+
Assert(!IS_UPPER_REL(context->foreignrel) ||
1061+
IS_JOIN_REL(scanrel) || IS_SIMPLE_REL(scanrel));
10681062

10691063
/* Construct FROM clause */
10701064
appendStringInfoString(buf, " FROM ");
@@ -1219,7 +1213,7 @@ deparseLockingClause(deparse_expr_cxt *context)
12191213
appendStringInfoString(buf, " FOR UPDATE");
12201214

12211215
/* Add the relation alias if we are here for a join relation */
1222-
if (rel->reloptkind == RELOPT_JOINREL)
1216+
if (IS_JOIN_REL(rel))
12231217
appendStringInfo(buf, " OF %s%d", REL_ALIAS_PREFIX, relid);
12241218
}
12251219
else
@@ -1384,8 +1378,7 @@ deparseSubqueryTargetList(deparse_expr_cxt *context)
13841378
ListCell *lc;
13851379

13861380
/* Should only be called in these cases. */
1387-
Assert(foreignrel->reloptkind == RELOPT_BASEREL ||
1388-
foreignrel->reloptkind == RELOPT_JOINREL);
1381+
Assert(IS_SIMPLE_REL(foreignrel) || IS_JOIN_REL(foreignrel));
13891382

13901383
first = true;
13911384
foreach(lc, foreignrel->reltarget->exprs)
@@ -1417,7 +1410,7 @@ deparseFromExprForRel(StringInfo buf, PlannerInfo *root, RelOptInfo *foreignrel,
14171410
{
14181411
PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) foreignrel->fdw_private;
14191412

1420-
if (foreignrel->reloptkind == RELOPT_JOINREL)
1413+
if (IS_JOIN_REL(foreignrel))
14211414
{
14221415
StringInfoData join_sql_o;
14231416
StringInfoData join_sql_i;
@@ -1495,8 +1488,7 @@ deparseRangeTblRef(StringInfo buf, PlannerInfo *root, RelOptInfo *foreignrel,
14951488
PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) foreignrel->fdw_private;
14961489

14971490
/* Should only be called in these cases. */
1498-
Assert(foreignrel->reloptkind == RELOPT_BASEREL ||
1499-
foreignrel->reloptkind == RELOPT_JOINREL);
1491+
Assert(IS_SIMPLE_REL(foreignrel) || IS_JOIN_REL(foreignrel));
15001492

15011493
Assert(fpinfo->local_conds == NIL);
15021494

@@ -3097,15 +3089,13 @@ is_subquery_var(Var *node, RelOptInfo *foreignrel, int *relno, int *colno)
30973089
RelOptInfo *innerrel = fpinfo->innerrel;
30983090

30993091
/* Should only be called in these cases. */
3100-
Assert(foreignrel->reloptkind == RELOPT_BASEREL ||
3101-
foreignrel->reloptkind == RELOPT_JOINREL ||
3102-
foreignrel->reloptkind == RELOPT_OTHER_MEMBER_REL);
3092+
Assert(IS_SIMPLE_REL(foreignrel) || IS_JOIN_REL(foreignrel));
31033093

31043094
/*
31053095
* If the given relation isn't a join relation, it doesn't have any lower
31063096
* subqueries, so the Var isn't a subquery output column.
31073097
*/
3108-
if (foreignrel->reloptkind != RELOPT_JOINREL)
3098+
if (!IS_JOIN_REL(foreignrel))
31093099
return false;
31103100

31113101
/*

‎contrib/postgres_fdw/postgres_fdw.c

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -729,8 +729,11 @@ get_useful_ecs_for_relation(PlannerInfo *root, RelOptInfo *rel)
729729
return useful_eclass_list;
730730

731731
/* If this is a child rel, we must use the topmost parent rel to search. */
732-
if (rel->reloptkind == RELOPT_OTHER_MEMBER_REL)
733-
relids = find_childrel_top_parent(root, rel)->relids;
732+
if (IS_OTHER_REL(rel))
733+
{
734+
Assert(!bms_is_empty(rel->top_parent_relids));
735+
relids = rel->top_parent_relids;
736+
}
734737
else
735738
relids = rel->relids;
736739

@@ -1129,8 +1132,7 @@ postgresGetForeignPlan(PlannerInfo *root,
11291132
* For base relations, set scan_relid as the relid of the relation. For
11301133
* other kinds of relations set it to 0.
11311134
*/
1132-
if (foreignrel->reloptkind == RELOPT_BASEREL ||
1133-
foreignrel->reloptkind == RELOPT_OTHER_MEMBER_REL)
1135+
if (IS_SIMPLE_REL(foreignrel))
11341136
scan_relid = foreignrel->relid;
11351137
else
11361138
{
@@ -1189,8 +1191,7 @@ postgresGetForeignPlan(PlannerInfo *root,
11891191
local_exprs = lappend(local_exprs, rinfo->clause);
11901192
}
11911193

1192-
if (foreignrel->reloptkind == RELOPT_JOINREL ||
1193-
foreignrel->reloptkind == RELOPT_UPPER_REL)
1194+
if (IS_JOIN_REL(foreignrel) || IS_UPPER_REL(foreignrel))
11941195
{
11951196
/* For a join relation, get the conditions from fdw_private structure */
11961197
remote_conds = fpinfo->remote_conds;
@@ -1216,7 +1217,7 @@ postgresGetForeignPlan(PlannerInfo *root,
12161217
* joins. Queries involving aggregates or grouping do not require
12171218
* EPQ mechanism, hence should not have an outer plan here.
12181219
*/
1219-
Assert(foreignrel->reloptkind != RELOPT_UPPER_REL);
1220+
Assert(!IS_UPPER_REL(foreignrel));
12201221

12211222
outer_plan->targetlist = fdw_scan_tlist;
12221223

@@ -1255,8 +1256,7 @@ postgresGetForeignPlan(PlannerInfo *root,
12551256
remote_conds,
12561257
retrieved_attrs,
12571258
makeInteger(fpinfo->fetch_size));
1258-
if (foreignrel->reloptkind == RELOPT_JOINREL ||
1259-
foreignrel->reloptkind == RELOPT_UPPER_REL)
1259+
if (IS_JOIN_REL(foreignrel) || IS_UPPER_REL(foreignrel))
12601260
fdw_private = lappend(fdw_private,
12611261
makeString(fpinfo->relation_name->data));
12621262

@@ -2535,8 +2535,7 @@ estimate_path_cost_size(PlannerInfo *root,
25352535
&remote_param_join_conds, &local_param_join_conds);
25362536

25372537
/* Build the list of columns to be fetched from the foreign server. */
2538-
if (foreignrel->reloptkind == RELOPT_JOINREL ||
2539-
foreignrel->reloptkind == RELOPT_UPPER_REL)
2538+
if (IS_JOIN_REL(foreignrel) || IS_UPPER_REL(foreignrel))
25402539
fdw_scan_tlist = build_tlist_to_deparse(foreignrel);
25412540
else
25422541
fdw_scan_tlist = NIL;
@@ -2617,7 +2616,7 @@ estimate_path_cost_size(PlannerInfo *root,
26172616
startup_cost = fpinfo->rel_startup_cost;
26182617
run_cost = fpinfo->rel_total_cost - fpinfo->rel_startup_cost;
26192618
}
2620-
else if (foreignrel->reloptkind == RELOPT_JOINREL)
2619+
else if (IS_JOIN_REL(foreignrel))
26212620
{
26222621
PgFdwRelationInfo *fpinfo_i;
26232622
PgFdwRelationInfo *fpinfo_o;
@@ -2683,7 +2682,7 @@ estimate_path_cost_size(PlannerInfo *root,
26832682
run_cost += nrows * remote_conds_cost.per_tuple;
26842683
run_cost += fpinfo->local_conds_cost.per_tuple * retrieved_rows;
26852684
}
2686-
else if (foreignrel->reloptkind == RELOPT_UPPER_REL)
2685+
else if (IS_UPPER_REL(foreignrel))
26872686
{
26882687
PgFdwRelationInfo *ofpinfo;
26892688
PathTarget *ptarget = root->upper_targets[UPPERREL_GROUP_AGG];

‎src/backend/foreign/foreign.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -717,7 +717,7 @@ GetExistingLocalJoinPath(RelOptInfo *joinrel)
717717
{
718718
ListCell *lc;
719719

720-
Assert(joinrel->reloptkind == RELOPT_JOINREL);
720+
Assert(IS_JOIN_REL(joinrel));
721721

722722
foreach(lc, joinrel->pathlist)
723723
{
@@ -782,7 +782,7 @@ GetExistingLocalJoinPath(RelOptInfo *joinrel)
782782
ForeignPath *foreign_path;
783783

784784
foreign_path = (ForeignPath *) joinpath->outerjoinpath;
785-
if (foreign_path->path.parent->reloptkind == RELOPT_JOINREL)
785+
if (IS_JOIN_REL(foreign_path->path.parent))
786786
joinpath->outerjoinpath = foreign_path->fdw_outerpath;
787787
}
788788

@@ -791,7 +791,7 @@ GetExistingLocalJoinPath(RelOptInfo *joinrel)
791791
ForeignPath *foreign_path;
792792

793793
foreign_path = (ForeignPath *) joinpath->innerjoinpath;
794-
if (foreign_path->path.parent->reloptkind == RELOPT_JOINREL)
794+
if (IS_JOIN_REL(foreign_path->path.parent))
795795
joinpath->innerjoinpath = foreign_path->fdw_outerpath;
796796
}
797797

‎src/backend/optimizer/path/allpaths.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -539,8 +539,7 @@ set_rel_consider_parallel(PlannerInfo *root, RelOptInfo *rel,
539539
Assert(root->glob->parallelModeOK);
540540

541541
/* This should only be called for baserels and appendrel children. */
542-
Assert(rel->reloptkind == RELOPT_BASEREL ||
543-
rel->reloptkind == RELOPT_OTHER_MEMBER_REL);
542+
Assert(IS_SIMPLE_REL(rel));
544543

545544
/* Assorted checks based on rtekind. */
546545
switch (rte->rtekind)
@@ -846,7 +845,7 @@ set_foreign_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
846845

847846
/*
848847
* set_append_rel_size
849-
* Set size estimates for an "append relation"
848+
* Set size estimates for a simple "append relation"
850849
*
851850
* The passed-in rel and RTE represent the entire append relation. The
852851
* relation's contents are computed by appending together the output of
@@ -867,6 +866,8 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
867866
int nattrs;
868867
ListCell *l;
869868

869+
Assert(IS_SIMPLE_REL(rel));
870+
870871
/*
871872
* Initialize to compute size estimates for whole append relation.
872873
*

‎src/backend/optimizer/path/equivclass.c

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1060,10 +1060,12 @@ generate_join_implied_equalities_for_ecs(PlannerInfo *root,
10601060
ListCell *lc;
10611061

10621062
/* If inner rel is a child, extra setup work is needed */
1063-
if (inner_rel->reloptkind == RELOPT_OTHER_MEMBER_REL)
1063+
if (IS_OTHER_REL(inner_rel))
10641064
{
1065+
Assert(!bms_is_empty(inner_rel->top_parent_relids));
1066+
10651067
/* Fetch relid set for the topmost parent rel */
1066-
nominal_inner_relids = find_childrel_top_parent(root, inner_rel)->relids;
1068+
nominal_inner_relids = inner_rel->top_parent_relids;
10671069
/* ECs will be marked with the parent's relid, not the child's */
10681070
nominal_join_relids = bms_union(outer_relids, nominal_inner_relids);
10691071
}
@@ -1324,8 +1326,7 @@ generate_join_implied_equalities_broken(PlannerInfo *root,
13241326
* mentioned in the ec_sources clauses, we have to be prepared to apply
13251327
* multiple levels of Var translation.
13261328
*/
1327-
if (inner_rel->reloptkind == RELOPT_OTHER_MEMBER_REL &&
1328-
result != NIL)
1329+
if (IS_OTHER_REL(inner_rel) && result != NIL)
13291330
result = (List *) adjust_appendrel_attrs_multilevel(root,
13301331
(Node *) result,
13311332
inner_rel);
@@ -2180,6 +2181,9 @@ generate_implied_equalities_for_column(PlannerInfo *root,
21802181
Relids parent_relids;
21812182
ListCell *lc1;
21822183

2184+
/* Indexes are available only on base or "other" member relations. */
2185+
Assert(IS_SIMPLE_REL(rel));
2186+
21832187
/* If it's a child rel, we'll need to know what its parent(s) are */
21842188
if (is_child_rel)
21852189
parent_relids = find_childrel_parents(root, rel);
@@ -2413,8 +2417,11 @@ eclass_useful_for_merging(PlannerInfo *root,
24132417
*/
24142418

24152419
/* If specified rel is a child, we must consider the topmost parent rel */
2416-
if (rel->reloptkind == RELOPT_OTHER_MEMBER_REL)
2417-
relids = find_childrel_top_parent(root, rel)->relids;
2420+
if (IS_OTHER_REL(rel))
2421+
{
2422+
Assert(!bms_is_empty(rel->top_parent_relids));
2423+
relids = rel->top_parent_relids;
2424+
}
24182425
else
24192426
relids = rel->relids;
24202427

‎src/backend/optimizer/path/indxpath.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2779,6 +2779,9 @@ check_index_predicates(PlannerInfo *root, RelOptInfo *rel)
27792779
Relids otherrels;
27802780
ListCell *lc;
27812781

2782+
/* Indexes are available only on base or "other" member relations. */
2783+
Assert(IS_SIMPLE_REL(rel));
2784+
27822785
/*
27832786
* Initialize the indrestrictinfo lists to be identical to
27842787
* baserestrictinfo, and check whether there are any partial indexes. If

‎src/backend/optimizer/plan/createplan.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3461,7 +3461,7 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
34613461
* upper rel doesn't have relids set, but it covers all the base relations
34623462
* participating in the underlying scan, so use root's all_baserels.
34633463
*/
3464-
if (rel->reloptkind == RELOPT_UPPER_REL)
3464+
if (IS_UPPER_REL(rel))
34653465
scan_plan->fs_relids = root->all_baserels;
34663466
else
34673467
scan_plan->fs_relids = best_path->path.parent->relids;

‎src/backend/optimizer/plan/initsplan.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ add_base_rels_to_query(PlannerInfo *root, Node *jtnode)
109109
{
110110
int varno = ((RangeTblRef *) jtnode)->rtindex;
111111

112-
(void) build_simple_rel(root, varno, RELOPT_BASEREL);
112+
(void) build_simple_rel(root, varno, NULL);
113113
}
114114
else if (IsA(jtnode, FromExpr))
115115
{

‎src/backend/optimizer/plan/planmain.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,8 +242,7 @@ query_planner(PlannerInfo *root, List *tlist,
242242

243243
Assert(brel->relid == rti); /* sanity check on array */
244244

245-
if (brel->reloptkind == RELOPT_BASEREL ||
246-
brel->reloptkind == RELOPT_OTHER_MEMBER_REL)
245+
if (IS_SIMPLE_REL(brel))
247246
total_pages += (double) brel->pages;
248247
}
249248
root->total_table_pages = total_pages;

‎src/backend/optimizer/plan/planner.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6001,7 +6001,7 @@ plan_cluster_use_sort(Oid tableOid, Oid indexOid)
60016001
setup_simple_rel_arrays(root);
60026002

60036003
/* Build RelOptInfo */
6004-
rel = build_simple_rel(root, 1, RELOPT_BASEREL);
6004+
rel = build_simple_rel(root, 1, NULL);
60056005

60066006
/* Locate IndexOptInfo for the target index */
60076007
indexInfo = NULL;

‎src/backend/optimizer/prep/prepunion.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ recurse_set_operations(Node *setOp, PlannerInfo *root,
271271
* used for much here, but it carries the subroot data structures
272272
* forward to setrefs.c processing.
273273
*/
274-
rel = build_simple_rel(root, rtr->rtindex, RELOPT_BASEREL);
274+
rel = build_simple_rel(root, rtr->rtindex, NULL);
275275

276276
/* plan_params should not be in use in current query level */
277277
Assert(root->plan_params == NIL);
@@ -2143,7 +2143,7 @@ adjust_appendrel_attrs_multilevel(PlannerInfo *root, Node *node,
21432143
RelOptInfo *parent_rel = find_base_rel(root, appinfo->parent_relid);
21442144

21452145
/* If parent is also a child, first recurse to apply its translations */
2146-
if (parent_rel->reloptkind == RELOPT_OTHER_MEMBER_REL)
2146+
if (IS_OTHER_REL(parent_rel))
21472147
node = adjust_appendrel_attrs_multilevel(root, node, parent_rel);
21482148
else
21492149
Assert(parent_rel->reloptkind == RELOPT_BASEREL);

0 commit comments

Comments
 (0)