Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit b7e2121

Browse files
committed
Postpone reparameterization of paths until create_plan().
When considering nestloop paths for individual partitions within a partitionwise join, if the inner path is parameterized, it is parameterized by the topmost parent of the outer rel, not the corresponding outer rel itself. Therefore, we need to translate the parameterization so that the inner path is parameterized by the corresponding outer rel. Up to now, we did this while generating join paths. However, that's problematic because we must also translate some expressions that are shared across all paths for a relation, such as restriction clauses (kept in the RelOptInfo and/or IndexOptInfo) and TableSampleClauses (kept in the RangeTblEntry). The existing code fails to translate these at all, leading to wrong answers, odd failures such as "variable not found in subplan target list", or executor crashes. But we can't modify them during path generation, because that would break things if we end up choosing some non-partitioned-join path. So this patch postpones reparameterization of the inner path until createplan.c, where it is safe to modify the referenced RangeTblEntry, RelOptInfo or IndexOptInfo, because we have made a final choice of which Path to use. We do still have to check during path generation that the reparameterization will be possible. So we introduce a new function path_is_reparameterizable_by_child() to detect that. The duplication between path_is_reparameterizable_by_child() and reparameterize_path_by_child() is a bit annoying, but there seems no other good answer. A small benefit is that we can avoid building useless reparameterized trees in cases where a non-partitioned join is ultimately chosen. Also, reparameterize_path_by_child() can now be allowed to scribble on the input paths, saving a few cycles. This fix repairs the same problems previously addressed in the back branches by commits 62f1202 et al. Richard Guo, reviewed at various times by Ashutosh Bapat, Andrei Lepikhov, Alena Rybakina, Robert Haas, and myself Discussion: https://postgr.es/m/CAMbWs496+N=UAjOc=rcD3P7B6oJe4rZw08e_TZRUsWbPxZW3Tw@mail.gmail.com
1 parent 605721f commit b7e2121

File tree

6 files changed

+485
-101
lines changed

6 files changed

+485
-101
lines changed

src/backend/optimizer/path/joinpath.c

Lines changed: 28 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,9 @@
3030
set_join_pathlist_hook_type set_join_pathlist_hook = NULL;
3131

3232
/*
33-
* Paths parameterized by the parent can be considered to be parameterized by
34-
* any of its child.
33+
* Paths parameterized by a parent rel can be considered to be parameterized
34+
* by any of its children, when we are performing partitionwise joins. These
35+
* macros simplify checking for such cases. Beware multiple eval of args.
3536
*/
3637
#define PATH_PARAM_BY_PARENT(path, rel) \
3738
((path)->param_info && bms_overlap(PATH_REQ_OUTER(path), \
@@ -784,6 +785,20 @@ try_nestloop_path(PlannerInfo *root,
784785
/* If we got past that, we shouldn't have any unsafe outer-join refs */
785786
Assert(!have_unsafe_outer_join_ref(root, outerrelids, inner_paramrels));
786787

788+
/*
789+
* If the inner path is parameterized, it is parameterized by the topmost
790+
* parent of the outer rel, not the outer rel itself. We will need to
791+
* translate the parameterization, if this path is chosen, during
792+
* create_plan(). Here we just check whether we will be able to perform
793+
* the translation, and if not avoid creating a nestloop path.
794+
*/
795+
if (PATH_PARAM_BY_PARENT(inner_path, outer_path->parent) &&
796+
!path_is_reparameterizable_by_child(inner_path, outer_path->parent))
797+
{
798+
bms_free(required_outer);
799+
return;
800+
}
801+
787802
/*
788803
* Do a precheck to quickly eliminate obviously-inferior paths. We
789804
* calculate a cheap lower bound on the path's cost and then use
@@ -800,27 +815,6 @@ try_nestloop_path(PlannerInfo *root,
800815
workspace.startup_cost, workspace.total_cost,
801816
pathkeys, required_outer))
802817
{
803-
/*
804-
* If the inner path is parameterized, it is parameterized by the
805-
* topmost parent of the outer rel, not the outer rel itself. Fix
806-
* that.
807-
*/
808-
if (PATH_PARAM_BY_PARENT(inner_path, outer_path->parent))
809-
{
810-
inner_path = reparameterize_path_by_child(root, inner_path,
811-
outer_path->parent);
812-
813-
/*
814-
* If we could not translate the path, we can't create nest loop
815-
* path.
816-
*/
817-
if (!inner_path)
818-
{
819-
bms_free(required_outer);
820-
return;
821-
}
822-
}
823-
824818
add_path(joinrel, (Path *)
825819
create_nestloop_path(root,
826820
joinrel,
@@ -883,6 +877,17 @@ try_partial_nestloop_path(PlannerInfo *root,
883877
return;
884878
}
885879

880+
/*
881+
* If the inner path is parameterized, it is parameterized by the topmost
882+
* parent of the outer rel, not the outer rel itself. We will need to
883+
* translate the parameterization, if this path is chosen, during
884+
* create_plan(). Here we just check whether we will be able to perform
885+
* the translation, and if not avoid creating a nestloop path.
886+
*/
887+
if (PATH_PARAM_BY_PARENT(inner_path, outer_path->parent) &&
888+
!path_is_reparameterizable_by_child(inner_path, outer_path->parent))
889+
return;
890+
886891
/*
887892
* Before creating a path, get a quick lower bound on what it is likely to
888893
* cost. Bail out right away if it looks terrible.
@@ -892,22 +897,6 @@ try_partial_nestloop_path(PlannerInfo *root,
892897
if (!add_partial_path_precheck(joinrel, workspace.total_cost, pathkeys))
893898
return;
894899

895-
/*
896-
* If the inner path is parameterized, it is parameterized by the topmost
897-
* parent of the outer rel, not the outer rel itself. Fix that.
898-
*/
899-
if (PATH_PARAM_BY_PARENT(inner_path, outer_path->parent))
900-
{
901-
inner_path = reparameterize_path_by_child(root, inner_path,
902-
outer_path->parent);
903-
904-
/*
905-
* If we could not translate the path, we can't create nest loop path.
906-
*/
907-
if (!inner_path)
908-
return;
909-
}
910-
911900
/* Might be good enough to be worth trying, so let's try it. */
912901
add_partial_path(joinrel, (Path *)
913902
create_nestloop_path(root,

src/backend/optimizer/plan/createplan.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "optimizer/cost.h"
3030
#include "optimizer/optimizer.h"
3131
#include "optimizer/paramassign.h"
32+
#include "optimizer/pathnode.h"
3233
#include "optimizer/paths.h"
3334
#include "optimizer/placeholder.h"
3435
#include "optimizer/plancat.h"
@@ -4355,6 +4356,22 @@ create_nestloop_plan(PlannerInfo *root,
43554356
List *nestParams;
43564357
Relids saveOuterRels = root->curOuterRels;
43574358

4359+
/*
4360+
* If the inner path is parameterized by the topmost parent of the outer
4361+
* rel rather than the outer rel itself, fix that. (Nothing happens here
4362+
* if it is not so parameterized.)
4363+
*/
4364+
best_path->jpath.innerjoinpath =
4365+
reparameterize_path_by_child(root,
4366+
best_path->jpath.innerjoinpath,
4367+
best_path->jpath.outerjoinpath->parent);
4368+
4369+
/*
4370+
* Failure here probably means that reparameterize_path_by_child() is not
4371+
* in sync with path_is_reparameterizable_by_child().
4372+
*/
4373+
Assert(best_path->jpath.innerjoinpath != NULL);
4374+
43584375
/* NestLoop can project, so no need to be picky about child tlists */
43594376
outer_plan = create_plan_recurse(root, best_path->jpath.outerjoinpath, 0);
43604377

0 commit comments

Comments
 (0)