Skip to content

Commit

Permalink
[BACKPORT 2.18.5] [#19642, #19946] YSQL: Refactor batched/unbatched p…
Browse files Browse the repository at this point in the history
…ath param requirement computation

Summary:
Original commit: 8ac82f4 / D30216
PG computes the required parameters of a path via the macro PATH_REQ_OUTER to read the param_info->ppi_req_outer field of a path. This change refactors reads to param_info->yb_ppi_req_outer_batched to be computed by the macro YB_PATH_REQ_OUTER_BATCHED.

This change also gets rid of the field yb_ppi_req_outer_unbatched in ParamPathInfo and computes unbatched relids via the difference between the total required params of a path and its batched params. Doing so simplifies logic and fixes the following bug:
Consider a Nested Loop join between B and C where B is parameterized by A but A is not a batched parameter. If the access path on B was not generated by yb_get_batched_index_paths, the yb_ppi_req_outer_unbatched field of B's param_info would not record A as one might expect. As a result, the param_info of the NestedLoop join would fail to register in its yb_ppi_req_outer_unbatched field that A needs to not be a batched parameter.

With this change, we release the need to explicitly keep track of what relations cannot be batched parameters. As long as the set of batchable parameters is accurately maintained, we can easily determine its complement.

NOTE: Original commit had tests that involved the usage of YbBatchedNL/NoYbBatchedNL hints that are unavailable in 2.18.5. Those tests have been replaced in this backport.

Jira: DB-8424, DB-8910

Test Plan:
Jenkins: Urgent

./yb_build.sh --java-test 'org.yb.pgsql.TestPgRegressJoin'

Reviewers: mtakahara

Reviewed By: mtakahara

Subscribers: yql

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D31396
  • Loading branch information
tanujnay112 committed Dec 29, 2023
1 parent b24a9f5 commit f0bc7c5
Show file tree
Hide file tree
Showing 10 changed files with 161 additions and 144 deletions.
4 changes: 4 additions & 0 deletions src/postgres/src/backend/optimizer/path/allpaths.c
Original file line number Diff line number Diff line change
Expand Up @@ -1848,11 +1848,15 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
accumulate_append_subpath(subpath, &subpaths, NULL);
}

subpaths_valid &= yb_has_same_batching_reqs(subpaths);

if (subpaths_valid)
add_path(rel, (Path *)
create_append_path(root, rel, subpaths, NIL,
required_outer, 0, false,
partitioned_rels, -1));
else
break;
}
}

Expand Down
3 changes: 0 additions & 3 deletions src/postgres/src/backend/optimizer/path/indxpath.c
Original file line number Diff line number Diff line change
Expand Up @@ -716,9 +716,7 @@ yb_get_batched_index_paths(PlannerInfo *root, RelOptInfo *rel,
batchedrelids = bms_difference(batchedrelids, unbatchablerelids);

Assert(!root->yb_cur_batched_relids);
Assert(!root->yb_cur_unbatched_relids);
root->yb_cur_batched_relids = batchedrelids;
root->yb_cur_unbatched_relids = unbatchablerelids;

/*
* Build simple index paths using the clauses. Allow ScalarArrayOpExpr
Expand Down Expand Up @@ -775,7 +773,6 @@ yb_get_batched_index_paths(PlannerInfo *root, RelOptInfo *rel,
}

root->yb_cur_batched_relids = NULL;
root->yb_cur_unbatched_relids = NULL;
}

/*
Expand Down
57 changes: 17 additions & 40 deletions src/postgres/src/backend/optimizer/path/joinpath.c
Original file line number Diff line number Diff line change
Expand Up @@ -451,17 +451,16 @@ try_nestloop_path(PlannerInfo *root,
}
}

if (inner_path->param_info &&
inner_path->param_info->yb_ppi_req_outer_batched)
if (IsYugaByteEnabled() && YB_PATH_NEEDS_BATCHED_RELS(inner_path))
{

if (yb_has_non_evaluable_bnl_clauses(outer_path,
inner_path,
extra->restrictlist) ||
(yb_has_non_evaluable_bnl_clauses(outer_path,
inner_path,
inner_path->param_info
->ppi_clauses)))
inner_path,
extra->restrictlist) ||
(yb_has_non_evaluable_bnl_clauses(outer_path,
inner_path,
inner_path->param_info
->ppi_clauses)))
{
bms_free(required_outer);
return;
Expand Down Expand Up @@ -1328,52 +1327,30 @@ generate_mergejoin_paths(PlannerInfo *root,
Relids
yb_get_batched_relids(NestPath *nest)
{
ParamPathInfo *innerppi = nest->innerjoinpath->param_info;
ParamPathInfo *outerppi = nest->outerjoinpath->param_info;
Relids inner_batched = YB_PATH_REQ_OUTER_BATCHED(nest->innerjoinpath);

Relids outer_unbatched =
outerppi ? outerppi->yb_ppi_req_outer_unbatched : NULL;
Relids inner_batched = innerppi ? innerppi->yb_ppi_req_outer_batched : NULL;

/* Rels not in this join that can't be batched. */
Relids param_unbatched =
nest->path.param_info ?
nest->path.param_info->yb_ppi_req_outer_unbatched : NULL;

return bms_difference(bms_difference(inner_batched, outer_unbatched),
param_unbatched);
Relids outerrels = nest->outerjoinpath->parent->relids;
return bms_intersect(inner_batched, outerrels);
}

Relids
yb_get_unbatched_relids(NestPath *nest)
{
ParamPathInfo *innerppi = nest->innerjoinpath->param_info;
ParamPathInfo *outerppi = nest->outerjoinpath->param_info;

Relids outer_unbatched =
outerppi ? outerppi->yb_ppi_req_outer_unbatched : NULL;
Relids inner_unbatched =
innerppi ? innerppi->yb_ppi_req_outer_unbatched : NULL;
Relids outer_unbatched = YB_PATH_REQ_OUTER_UNBATCHED(nest->outerjoinpath);
Relids inner_unbatched = YB_PATH_REQ_OUTER_UNBATCHED(nest->innerjoinpath);

/* Rels not in this join that can't be batched. */
Relids param_unbatched =
nest->path.param_info ?
nest->path.param_info->yb_ppi_req_outer_unbatched : NULL;
Relids param_unbatched = YB_PATH_REQ_OUTER_UNBATCHED(&nest->path);

return bms_union(outer_unbatched,
bms_union(inner_unbatched, param_unbatched));
bms_union(inner_unbatched, param_unbatched));
}

bool
yb_is_outer_inner_batched(Path *outer, Path *inner)
{
ParamPathInfo *innerppi = inner->param_info;
ParamPathInfo *outerppi = outer->param_info;

Relids outer_unbatched =
outerppi ? outerppi->yb_ppi_req_outer_unbatched : NULL;
Relids inner_batched =
innerppi ? innerppi->yb_ppi_req_outer_batched : NULL;
Relids outer_unbatched = YB_PATH_REQ_OUTER_UNBATCHED(outer);
Relids inner_batched = YB_PATH_REQ_OUTER_BATCHED(inner);

return bms_overlap(outer->parent->relids,
bms_difference(inner_batched, outer_unbatched));
Expand Down Expand Up @@ -2127,7 +2104,7 @@ yb_has_non_evaluable_bnl_clauses(Path *outer_path, Path *inner_path,
List *rinfos)
{
ListCell *lc;
Relids req_batched_rels = inner_path->param_info->yb_ppi_req_outer_batched;
Relids req_batched_rels = YB_PATH_REQ_OUTER_BATCHED(inner_path);
Relids outer_relids = outer_path->parent->relids;
Relids inner_relids = inner_path->parent->relids;

Expand Down
13 changes: 9 additions & 4 deletions src/postgres/src/backend/optimizer/plan/createplan.c
Original file line number Diff line number Diff line change
Expand Up @@ -4871,6 +4871,11 @@ create_nestloop_plan(PlannerInfo *root,

if (yb_is_batched)
{
/* No rels supplied to inner from outer should be unbatched. */
Relids inner_unbatched =
YB_PATH_REQ_OUTER_UNBATCHED(best_path->innerjoinpath);
Assert(!bms_overlap(inner_unbatched, outerrelids));
(void)inner_unbatched;
/* Add the available batched outer rels. */
root->yb_availBatchedRelids =
lcons(outerrelids, root->yb_availBatchedRelids);
Expand Down Expand Up @@ -4906,14 +4911,14 @@ create_nestloop_plan(PlannerInfo *root,
}

if (rinfo->can_join &&
OidIsValid(rinfo->hashjoinoperator) &&
yb_can_batch_rinfo(rinfo, batched_outerrelids, inner_relids))
OidIsValid(rinfo->hashjoinoperator) &&
yb_can_batch_rinfo(rinfo, batched_outerrelids, inner_relids))
{
/* if nlhash can process this */
Assert(is_opclause(rinfo->clause));
RestrictInfo *batched_rinfo =
yb_get_batched_restrictinfo(rinfo,batched_outerrelids,
inner_relids);
yb_get_batched_restrictinfo(rinfo, batched_outerrelids,
inner_relids);

hashOpno = ((OpExpr *) batched_rinfo->clause)->opno;
}
Expand Down
22 changes: 9 additions & 13 deletions src/postgres/src/backend/optimizer/util/pathnode.c
Original file line number Diff line number Diff line change
Expand Up @@ -1295,14 +1295,14 @@ create_append_path(PlannerInfo *root,
if (partitioned_rels != NIL && root && rel->reloptkind == RELOPT_BASEREL)
{
/* YB: Accumulate batching info from subpaths for this "baserel". */
Assert(root->yb_cur_batched_relids == NULL);
yb_accumulate_batching_info(subpaths,
&root->yb_cur_batched_relids, &root->yb_cur_unbatched_relids);
Assert(yb_has_same_batching_reqs(subpaths));

root->yb_cur_batched_relids =
YB_PATH_REQ_OUTER_BATCHED((Path *) linitial(subpaths));
pathnode->path.param_info = get_baserel_parampathinfo(root,
rel,
required_outer);
root->yb_cur_batched_relids = NULL;
root->yb_cur_unbatched_relids = NULL;
}
else
pathnode->path.param_info = get_appendrel_parampathinfo(rel,
Expand Down Expand Up @@ -2263,17 +2263,13 @@ create_nestloop_path(PlannerInfo *root,
* because the restrict_clauses list can affect the size and cost
* estimates for this path.
*/
ParamPathInfo *param_info = inner_path->param_info;
Relids inner_req_batched = param_info == NULL
? NULL : param_info->yb_ppi_req_outer_batched;

Relids outer_req_unbatched = outer_path->param_info ?
outer_path->param_info->yb_ppi_req_outer_unbatched :
NULL;
Relids inner_req_batched = YB_PATH_REQ_OUTER_BATCHED(inner_path);

Relids outer_req_unbatched = YB_PATH_REQ_OUTER_UNBATCHED(outer_path);

bool is_batched = bms_overlap(inner_req_batched,
outer_path->parent->relids) &&
!bms_overlap(outer_req_unbatched, inner_req_batched);
outer_path->parent->relids) &&
!bms_overlap(outer_req_unbatched, inner_req_batched);
if (!is_batched && bms_overlap(inner_req_outer, outer_path->parent->relids))
{
Relids inner_and_outer = bms_union(inner_path->parent->relids,
Expand Down
Loading

0 comments on commit f0bc7c5

Please sign in to comment.