Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit1cce024

Browse files
committed
Fix pull_varnos' miscomputation of relids set for a PlaceHolderVar.
Previously, pull_varnos() took the relids of a PlaceHolderVar as beingequal to the relids in its contents, but that fails to account for thepossibility that we have to postpone evaluation of the PHV due to outerjoins. This could result in a malformed plan. The known cases end uptriggering the "failed to assign all NestLoopParams to plan nodes"sanity check in createplan.c, but other symptoms may be possible.The right value to use is the join level we actually intend to evaluatethe PHV at. We can get that from the ph_eval_at field of the associatedPlaceHolderInfo. However, there are some places that call pull_varnos()before the PlaceHolderInfos have been created; in that case, fall backto the conservative assumption that the PHV will be evaluated at itssyntactic level. (In principle this might result in missing some legaloptimization, but I'm not aware of any cases where it's an issue inpractice.) Things are also a bit ticklish for calls occurring duringdeconstruct_jointree(), but AFAICS the ph_eval_at fields should havereached their final values by the time we need them.The main problem in making this work is that pull_varnos() has noway to get at the PlaceHolderInfos. We can fix that easily, if abit tediously, in HEAD by passing it the planner "root" pointer.In the back branches that'd cause an unacceptable API/ABI break forextensions, so leave the existing entry points alone and add new oneswith the additional parameter. (If an old entry point is called andencounters a PHV, it'll fall back to using the syntactic level,again possibly missing some valid optimization.)Back-patch to v12. The computation is surely also wrong before that,but it appears that we cannot reach a bad plan thanks to join orderrestrictions imposed on the subquery that the PlaceHolderVar came from.The error only became reachable when commit4be058f allowed trivialsubqueries to be collapsed out completely, eliminating their join orderrestrictions.Per report from Stephan Springl.Discussion:https://postgr.es/m/171041.1610849523@sss.pgh.pa.us
1 parent561dd8d commit1cce024

File tree

24 files changed

+354
-116
lines changed

24 files changed

+354
-116
lines changed

‎contrib/postgres_fdw/postgres_fdw.c‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,9 @@
4444
#include"utils/sampling.h"
4545
#include"utils/selfuncs.h"
4646

47+
/* source-code-compatibility hacks for pull_varnos() API change */
48+
#definemake_restrictinfo(a,b,c,d,e,f,g,h,i) make_restrictinfo_new(a,b,c,d,e,f,g,h,i)
49+
4750
PG_MODULE_MAGIC;
4851

4952
/* Default CPU cost to start up a foreign query. */
@@ -5665,7 +5668,8 @@ foreign_grouping_ok(PlannerInfo *root, RelOptInfo *grouped_rel,
56655668
* RestrictInfos, so we must make our own.
56665669
*/
56675670
Assert(!IsA(expr,RestrictInfo));
5668-
rinfo=make_restrictinfo(expr,
5671+
rinfo=make_restrictinfo(root,
5672+
expr,
56695673
true,
56705674
false,
56715675
false,

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@
2727
#include"statistics/statistics.h"
2828

2929

30+
/* source-code-compatibility hacks for pull_varnos() API change */
31+
#defineNumRelids(a,b) NumRelids_new(a,b)
32+
3033
/*
3134
* Data structure for accumulating info about possible range-query
3235
* clause pairs in clauselist_selectivity.
@@ -236,7 +239,7 @@ clauselist_selectivity_simple(PlannerInfo *root,
236239
}
237240
else
238241
{
239-
ok= (NumRelids(clause)==1)&&
242+
ok= (NumRelids(root,clause)==1)&&
240243
(is_pseudo_constant_clause(lsecond(expr->args))||
241244
(varonleft= false,
242245
is_pseudo_constant_clause(linitial(expr->args))));
@@ -520,7 +523,7 @@ bms_is_subset_singleton(const Bitmapset *s, int x)
520523
* restriction or join estimator. Subroutine for clause_selectivity().
521524
*/
522525
staticinlinebool
523-
treat_as_join_clause(Node*clause,RestrictInfo*rinfo,
526+
treat_as_join_clause(PlannerInfo*root,Node*clause,RestrictInfo*rinfo,
524527
intvarRelid,SpecialJoinInfo*sjinfo)
525528
{
526529
if (varRelid!=0)
@@ -554,7 +557,7 @@ treat_as_join_clause(Node *clause, RestrictInfo *rinfo,
554557
if (rinfo)
555558
return (bms_membership(rinfo->clause_relids)==BMS_MULTIPLE);
556559
else
557-
return (NumRelids(clause)>1);
560+
return (NumRelids(root,clause)>1);
558561
}
559562
}
560563

@@ -760,7 +763,7 @@ clause_selectivity(PlannerInfo *root,
760763
OpExpr*opclause= (OpExpr*)clause;
761764
Oidopno=opclause->opno;
762765

763-
if (treat_as_join_clause(clause,rinfo,varRelid,sjinfo))
766+
if (treat_as_join_clause(root,clause,rinfo,varRelid,sjinfo))
764767
{
765768
/* Estimate selectivity for a join clause. */
766769
s1=join_selectivity(root,opno,
@@ -796,7 +799,7 @@ clause_selectivity(PlannerInfo *root,
796799
funcclause->funcid,
797800
funcclause->args,
798801
funcclause->inputcollid,
799-
treat_as_join_clause(clause,rinfo,
802+
treat_as_join_clause(root,clause,rinfo,
800803
varRelid,sjinfo),
801804
varRelid,
802805
jointype,
@@ -807,7 +810,7 @@ clause_selectivity(PlannerInfo *root,
807810
/* Use node specific selectivity calculation function */
808811
s1=scalararraysel(root,
809812
(ScalarArrayOpExpr*)clause,
810-
treat_as_join_clause(clause,rinfo,
813+
treat_as_join_clause(root,clause,rinfo,
811814
varRelid,sjinfo),
812815
varRelid,
813816
jointype,

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

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@
3232
#include"utils/lsyscache.h"
3333

3434

35+
/* source-code-compatibility hacks for pull_varnos() API change */
36+
#definepull_varnos(a,b) pull_varnos_new(a,b)
37+
#definemake_restrictinfo(a,b,c,d,e,f,g,h,i) make_restrictinfo_new(a,b,c,d,e,f,g,h,i)
38+
3539
staticEquivalenceMember*add_eq_member(EquivalenceClass*ec,
3640
Expr*expr,Relidsrelids,Relidsnullable_relids,
3741
boolis_child,Oiddatatype);
@@ -191,7 +195,8 @@ process_equivalence(PlannerInfo *root,
191195
ntest->location=-1;
192196

193197
*p_restrictinfo=
194-
make_restrictinfo((Expr*)ntest,
198+
make_restrictinfo(root,
199+
(Expr*)ntest,
195200
restrictinfo->is_pushed_down,
196201
restrictinfo->outerjoin_delayed,
197202
restrictinfo->pseudoconstant,
@@ -708,7 +713,7 @@ get_eclass_for_sort_expr(PlannerInfo *root,
708713
/*
709714
* Get the precise set of nullable relids appearing in the expression.
710715
*/
711-
expr_relids=pull_varnos((Node*)expr);
716+
expr_relids=pull_varnos(root,(Node*)expr);
712717
nullable_relids=bms_intersect(nullable_relids,expr_relids);
713718

714719
newem=add_eq_member(newec,copyObject(expr),expr_relids,
@@ -1449,7 +1454,8 @@ create_join_clause(PlannerInfo *root,
14491454
*/
14501455
oldcontext=MemoryContextSwitchTo(root->planner_cxt);
14511456

1452-
rinfo=build_implied_join_equality(opno,
1457+
rinfo=build_implied_join_equality(root,
1458+
opno,
14531459
ec->ec_collation,
14541460
leftem->em_expr,
14551461
rightem->em_expr,
@@ -1763,7 +1769,8 @@ reconsider_outer_join_clause(PlannerInfo *root, RestrictInfo *rinfo,
17631769
cur_em->em_datatype);
17641770
if (!OidIsValid(eq_op))
17651771
continue;/* can't generate equality */
1766-
newrinfo=build_implied_join_equality(eq_op,
1772+
newrinfo=build_implied_join_equality(root,
1773+
eq_op,
17671774
cur_ec->ec_collation,
17681775
innervar,
17691776
cur_em->em_expr,
@@ -1906,7 +1913,8 @@ reconsider_full_join_clause(PlannerInfo *root, RestrictInfo *rinfo)
19061913
cur_em->em_datatype);
19071914
if (OidIsValid(eq_op))
19081915
{
1909-
newrinfo=build_implied_join_equality(eq_op,
1916+
newrinfo=build_implied_join_equality(root,
1917+
eq_op,
19101918
cur_ec->ec_collation,
19111919
leftvar,
19121920
cur_em->em_expr,
@@ -1921,7 +1929,8 @@ reconsider_full_join_clause(PlannerInfo *root, RestrictInfo *rinfo)
19211929
cur_em->em_datatype);
19221930
if (OidIsValid(eq_op))
19231931
{
1924-
newrinfo=build_implied_join_equality(eq_op,
1932+
newrinfo=build_implied_join_equality(root,
1933+
eq_op,
19251934
cur_ec->ec_collation,
19261935
rightvar,
19271936
cur_em->em_expr,

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

Lines changed: 48 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,12 @@
3636
#include"utils/selfuncs.h"
3737

3838

39+
/* source-code-compatibility hacks for pull_varnos() API change */
40+
#definepull_varnos(a,b) pull_varnos_new(a,b)
41+
#undef make_simple_restrictinfo
42+
#definemake_simple_restrictinfo(root,clause) \
43+
make_restrictinfo_new(root, clause, true, false, false, 0, NULL, NULL, NULL)
44+
3945
/* XXX see PartCollMatchesExprColl */
4046
#defineIndexCollMatchesExprColl(idxcollation,exprcollation) \
4147
((idxcollation) == InvalidOid || (idxcollation) == (exprcollation))
@@ -153,7 +159,8 @@ static IndexClause *match_clause_to_indexcol(PlannerInfo *root,
153159
RestrictInfo*rinfo,
154160
intindexcol,
155161
IndexOptInfo*index);
156-
staticIndexClause*match_boolean_index_clause(RestrictInfo*rinfo,
162+
staticIndexClause*match_boolean_index_clause(PlannerInfo*root,
163+
RestrictInfo*rinfo,
157164
intindexcol,IndexOptInfo*index);
158165
staticIndexClause*match_opclause_to_indexcol(PlannerInfo*root,
159166
RestrictInfo*rinfo,
@@ -169,13 +176,16 @@ static IndexClause *get_index_clause_from_support(PlannerInfo *root,
169176
intindexarg,
170177
intindexcol,
171178
IndexOptInfo*index);
172-
staticIndexClause*match_saopclause_to_indexcol(RestrictInfo*rinfo,
179+
staticIndexClause*match_saopclause_to_indexcol(PlannerInfo*root,
180+
RestrictInfo*rinfo,
173181
intindexcol,
174182
IndexOptInfo*index);
175-
staticIndexClause*match_rowcompare_to_indexcol(RestrictInfo*rinfo,
183+
staticIndexClause*match_rowcompare_to_indexcol(PlannerInfo*root,
184+
RestrictInfo*rinfo,
176185
intindexcol,
177186
IndexOptInfo*index);
178-
staticIndexClause*expand_indexqual_rowcompare(RestrictInfo*rinfo,
187+
staticIndexClause*expand_indexqual_rowcompare(PlannerInfo*root,
188+
RestrictInfo*rinfo,
179189
intindexcol,
180190
IndexOptInfo*index,
181191
Oidexpr_op,
@@ -2313,7 +2323,7 @@ match_clause_to_indexcol(PlannerInfo *root,
23132323
opfamily=index->opfamily[indexcol];
23142324
if (IsBooleanOpfamily(opfamily))
23152325
{
2316-
iclause=match_boolean_index_clause(rinfo,indexcol,index);
2326+
iclause=match_boolean_index_clause(root,rinfo,indexcol,index);
23172327
if (iclause)
23182328
returniclause;
23192329
}
@@ -2333,11 +2343,11 @@ match_clause_to_indexcol(PlannerInfo *root,
23332343
}
23342344
elseif (IsA(clause,ScalarArrayOpExpr))
23352345
{
2336-
returnmatch_saopclause_to_indexcol(rinfo,indexcol,index);
2346+
returnmatch_saopclause_to_indexcol(root,rinfo,indexcol,index);
23372347
}
23382348
elseif (IsA(clause,RowCompareExpr))
23392349
{
2340-
returnmatch_rowcompare_to_indexcol(rinfo,indexcol,index);
2350+
returnmatch_rowcompare_to_indexcol(root,rinfo,indexcol,index);
23412351
}
23422352
elseif (index->amsearchnulls&&IsA(clause,NullTest))
23432353
{
@@ -2376,7 +2386,8 @@ match_clause_to_indexcol(PlannerInfo *root,
23762386
* index's key, and if so, build a suitable IndexClause.
23772387
*/
23782388
staticIndexClause*
2379-
match_boolean_index_clause(RestrictInfo*rinfo,
2389+
match_boolean_index_clause(PlannerInfo*root,
2390+
RestrictInfo*rinfo,
23802391
intindexcol,
23812392
IndexOptInfo*index)
23822393
{
@@ -2446,7 +2457,7 @@ match_boolean_index_clause(RestrictInfo *rinfo,
24462457
IndexClause*iclause=makeNode(IndexClause);
24472458

24482459
iclause->rinfo=rinfo;
2449-
iclause->indexquals=list_make1(make_simple_restrictinfo(op));
2460+
iclause->indexquals=list_make1(make_simple_restrictinfo(root,op));
24502461
iclause->lossy= false;
24512462
iclause->indexcol=indexcol;
24522463
iclause->indexcols=NIL;
@@ -2671,7 +2682,8 @@ get_index_clause_from_support(PlannerInfo *root,
26712682
{
26722683
Expr*clause= (Expr*)lfirst(lc);
26732684

2674-
indexquals=lappend(indexquals,make_simple_restrictinfo(clause));
2685+
indexquals=lappend(indexquals,
2686+
make_simple_restrictinfo(root,clause));
26752687
}
26762688

26772689
iclause->rinfo=rinfo;
@@ -2692,7 +2704,8 @@ get_index_clause_from_support(PlannerInfo *root,
26922704
* which see for comments.
26932705
*/
26942706
staticIndexClause*
2695-
match_saopclause_to_indexcol(RestrictInfo*rinfo,
2707+
match_saopclause_to_indexcol(PlannerInfo*root,
2708+
RestrictInfo*rinfo,
26962709
intindexcol,
26972710
IndexOptInfo*index)
26982711
{
@@ -2711,7 +2724,7 @@ match_saopclause_to_indexcol(RestrictInfo *rinfo,
27112724
returnNULL;
27122725
leftop= (Node*)linitial(saop->args);
27132726
rightop= (Node*)lsecond(saop->args);
2714-
right_relids=pull_varnos(rightop);
2727+
right_relids=pull_varnos(root,rightop);
27152728
expr_op=saop->opno;
27162729
expr_coll=saop->inputcollid;
27172730

@@ -2759,7 +2772,8 @@ match_saopclause_to_indexcol(RestrictInfo *rinfo,
27592772
* is handled by expand_indexqual_rowcompare().
27602773
*/
27612774
staticIndexClause*
2762-
match_rowcompare_to_indexcol(RestrictInfo*rinfo,
2775+
match_rowcompare_to_indexcol(PlannerInfo*root,
2776+
RestrictInfo*rinfo,
27632777
intindexcol,
27642778
IndexOptInfo*index)
27652779
{
@@ -2804,14 +2818,14 @@ match_rowcompare_to_indexcol(RestrictInfo *rinfo,
28042818
* These syntactic tests are the same as in match_opclause_to_indexcol()
28052819
*/
28062820
if (match_index_to_operand(leftop,indexcol,index)&&
2807-
!bms_is_member(index_relid,pull_varnos(rightop))&&
2821+
!bms_is_member(index_relid,pull_varnos(root,rightop))&&
28082822
!contain_volatile_functions(rightop))
28092823
{
28102824
/* OK, indexkey is on left */
28112825
var_on_left= true;
28122826
}
28132827
elseif (match_index_to_operand(rightop,indexcol,index)&&
2814-
!bms_is_member(index_relid,pull_varnos(leftop))&&
2828+
!bms_is_member(index_relid,pull_varnos(root,leftop))&&
28152829
!contain_volatile_functions(leftop))
28162830
{
28172831
/* indexkey is on right, so commute the operator */
@@ -2830,7 +2844,8 @@ match_rowcompare_to_indexcol(RestrictInfo *rinfo,
28302844
caseBTLessEqualStrategyNumber:
28312845
caseBTGreaterEqualStrategyNumber:
28322846
caseBTGreaterStrategyNumber:
2833-
returnexpand_indexqual_rowcompare(rinfo,
2847+
returnexpand_indexqual_rowcompare(root,
2848+
rinfo,
28342849
indexcol,
28352850
index,
28362851
expr_op,
@@ -2864,7 +2879,8 @@ match_rowcompare_to_indexcol(RestrictInfo *rinfo,
28642879
* but we split it out for comprehensibility.
28652880
*/
28662881
staticIndexClause*
2867-
expand_indexqual_rowcompare(RestrictInfo*rinfo,
2882+
expand_indexqual_rowcompare(PlannerInfo*root,
2883+
RestrictInfo*rinfo,
28682884
intindexcol,
28692885
IndexOptInfo*index,
28702886
Oidexpr_op,
@@ -2942,7 +2958,7 @@ expand_indexqual_rowcompare(RestrictInfo *rinfo,
29422958
if (expr_op==InvalidOid)
29432959
break;/* operator is not usable */
29442960
}
2945-
if (bms_is_member(index->rel->relid,pull_varnos(constop)))
2961+
if (bms_is_member(index->rel->relid,pull_varnos(root,constop)))
29462962
break;/* no good, Var on wrong side */
29472963
if (contain_volatile_functions(constop))
29482964
break;/* no good, volatile comparison value */
@@ -3055,7 +3071,8 @@ expand_indexqual_rowcompare(RestrictInfo *rinfo,
30553071
matching_cols);
30563072
rc->rargs=list_truncate(copyObject(non_var_args),
30573073
matching_cols);
3058-
iclause->indexquals=list_make1(make_simple_restrictinfo((Expr*)rc));
3074+
iclause->indexquals=list_make1(make_simple_restrictinfo(root,
3075+
(Expr*)rc));
30593076
}
30603077
else
30613078
{
@@ -3069,7 +3086,7 @@ expand_indexqual_rowcompare(RestrictInfo *rinfo,
30693086
copyObject(linitial(non_var_args)),
30703087
InvalidOid,
30713088
linitial_oid(clause->inputcollids));
3072-
iclause->indexquals=list_make1(make_simple_restrictinfo(op));
3089+
iclause->indexquals=list_make1(make_simple_restrictinfo(root,op));
30733090
}
30743091
}
30753092

@@ -3686,7 +3703,9 @@ relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel,
36863703
* specified index column matches a boolean restriction clause.
36873704
*/
36883705
bool
3689-
indexcol_is_bool_constant_for_query(IndexOptInfo*index,intindexcol)
3706+
indexcol_is_bool_constant_for_query(PlannerInfo*root,
3707+
IndexOptInfo*index,
3708+
intindexcol)
36903709
{
36913710
ListCell*lc;
36923711

@@ -3708,7 +3727,7 @@ indexcol_is_bool_constant_for_query(IndexOptInfo *index, int indexcol)
37083727
continue;
37093728

37103729
/* See if we can match the clause's expression to the index column */
3711-
if (match_boolean_index_clause(rinfo,indexcol,index))
3730+
if (match_boolean_index_clause(root,rinfo,indexcol,index))
37123731
return true;
37133732
}
37143733

@@ -3821,9 +3840,15 @@ match_index_to_operand(Node *operand,
38213840
*/
38223841
bool
38233842
is_pseudo_constant_for_index(Node*expr,IndexOptInfo*index)
3843+
{
3844+
returnis_pseudo_constant_for_index_new(NULL,expr,index);
3845+
}
3846+
3847+
bool
3848+
is_pseudo_constant_for_index_new(PlannerInfo*root,Node*expr,IndexOptInfo*index)
38243849
{
38253850
/* pull_varnos is cheaper than volatility check, so do that first */
3826-
if (bms_is_member(index->rel->relid,pull_varnos(expr)))
3851+
if (bms_is_member(index->rel->relid,pull_varnos(root,expr)))
38273852
return false;/* no good, contains Var of table */
38283853
if (contain_volatile_functions(expr))
38293854
return false;/* no good, volatile comparison value */

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,7 @@ build_index_pathkeys(PlannerInfo *root,
542542
* should stop considering index columns; any lower-order sort
543543
* keys won't be useful either.
544544
*/
545-
if (!indexcol_is_bool_constant_for_query(index,i))
545+
if (!indexcol_is_bool_constant_for_query(root,index,i))
546546
break;
547547
}
548548

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp