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

Commit3753982

Browse files
committed
Fix planner failure in some cases of sorting by an aggregate.
An oversight introduced by the incremental-sort patches caused"could not find pathkey item to sort" errors in some situationswhere a sort key involves an aggregate or window function.The basic problem here is that find_em_expr_usable_for_sorting_relisn't properly modeling what prepare_sort_from_pathkeys will dolater. Rather than hoping we can keep those functions in sync,let's refactor so that they actually share the code foridentifying a suitable sort expression.With this refactoring, tlist.c's tlist_member_ignore_relabelis unused. I removed it in HEAD but left it in place in v13,in case any extensions are using it.Per report from Luc Vlaming. Back-patch to v13 where theproblem arose.James Coleman and Tom LaneDiscussion:https://postgr.es/m/91f3ec99-85a4-fa55-ea74-33f85a5c651f@swarm64.com
1 parent95c3a19 commit3753982

File tree

7 files changed

+262
-181
lines changed

7 files changed

+262
-181
lines changed

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

Lines changed: 212 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
staticEquivalenceMember*add_eq_member(EquivalenceClass*ec,
3636
Expr*expr,Relidsrelids,Relidsnullable_relids,
3737
boolis_child,Oiddatatype);
38+
staticboolis_exprlist_member(Expr*node,List*exprs);
3839
staticvoidgenerate_base_implied_equalities_const(PlannerInfo*root,
3940
EquivalenceClass*ec);
4041
staticvoidgenerate_base_implied_equalities_no_const(PlannerInfo*root,
@@ -769,6 +770,167 @@ get_eclass_for_sort_expr(PlannerInfo *root,
769770
returnnewec;
770771
}
771772

773+
/*
774+
* find_ec_member_matching_expr
775+
*Locate an EquivalenceClass member matching the given expr, if any;
776+
*return NULL if no match.
777+
*
778+
* "Matching" is defined as "equal after stripping RelabelTypes".
779+
* This is used for identifying sort expressions, and we need to allow
780+
* binary-compatible relabeling for some cases involving binary-compatible
781+
* sort operators.
782+
*
783+
* Child EC members are ignored unless they belong to given 'relids'.
784+
*/
785+
EquivalenceMember*
786+
find_ec_member_matching_expr(EquivalenceClass*ec,
787+
Expr*expr,
788+
Relidsrelids)
789+
{
790+
ListCell*lc;
791+
792+
/* We ignore binary-compatible relabeling on both ends */
793+
while (expr&&IsA(expr,RelabelType))
794+
expr= ((RelabelType*)expr)->arg;
795+
796+
foreach(lc,ec->ec_members)
797+
{
798+
EquivalenceMember*em= (EquivalenceMember*)lfirst(lc);
799+
Expr*emexpr;
800+
801+
/*
802+
* We shouldn't be trying to sort by an equivalence class that
803+
* contains a constant, so no need to consider such cases any further.
804+
*/
805+
if (em->em_is_const)
806+
continue;
807+
808+
/*
809+
* Ignore child members unless they belong to the requested rel.
810+
*/
811+
if (em->em_is_child&&
812+
!bms_is_subset(em->em_relids,relids))
813+
continue;
814+
815+
/*
816+
* Match if same expression (after stripping relabel).
817+
*/
818+
emexpr=em->em_expr;
819+
while (emexpr&&IsA(emexpr,RelabelType))
820+
emexpr= ((RelabelType*)emexpr)->arg;
821+
822+
if (equal(emexpr,expr))
823+
returnem;
824+
}
825+
826+
returnNULL;
827+
}
828+
829+
/*
830+
* find_computable_ec_member
831+
*Locate an EquivalenceClass member that can be computed from the
832+
*expressions appearing in "exprs"; return NULL if no match.
833+
*
834+
* "exprs" can be either a list of bare expression trees, or a list of
835+
* TargetEntry nodes. Either way, it should contain Vars and possibly
836+
* Aggrefs and WindowFuncs, which are matched to the corresponding elements
837+
* of the EquivalenceClass's expressions.
838+
*
839+
* Unlike find_ec_member_matching_expr, there's no special provision here
840+
* for binary-compatible relabeling. This is intentional: if we have to
841+
* compute an expression in this way, setrefs.c is going to insist on exact
842+
* matches of Vars to the source tlist.
843+
*
844+
* Child EC members are ignored unless they belong to given 'relids'.
845+
* Also, non-parallel-safe expressions are ignored if 'require_parallel_safe'.
846+
*
847+
* Note: some callers pass root == NULL for notational reasons. This is OK
848+
* when require_parallel_safe is false.
849+
*/
850+
EquivalenceMember*
851+
find_computable_ec_member(PlannerInfo*root,
852+
EquivalenceClass*ec,
853+
List*exprs,
854+
Relidsrelids,
855+
boolrequire_parallel_safe)
856+
{
857+
ListCell*lc;
858+
859+
foreach(lc,ec->ec_members)
860+
{
861+
EquivalenceMember*em= (EquivalenceMember*)lfirst(lc);
862+
List*exprvars;
863+
ListCell*lc2;
864+
865+
/*
866+
* We shouldn't be trying to sort by an equivalence class that
867+
* contains a constant, so no need to consider such cases any further.
868+
*/
869+
if (em->em_is_const)
870+
continue;
871+
872+
/*
873+
* Ignore child members unless they belong to the requested rel.
874+
*/
875+
if (em->em_is_child&&
876+
!bms_is_subset(em->em_relids,relids))
877+
continue;
878+
879+
/*
880+
* Match if all Vars and quasi-Vars are available in "exprs".
881+
*/
882+
exprvars=pull_var_clause((Node*)em->em_expr,
883+
PVC_INCLUDE_AGGREGATES |
884+
PVC_INCLUDE_WINDOWFUNCS |
885+
PVC_INCLUDE_PLACEHOLDERS);
886+
foreach(lc2,exprvars)
887+
{
888+
if (!is_exprlist_member(lfirst(lc2),exprs))
889+
break;
890+
}
891+
list_free(exprvars);
892+
if (lc2)
893+
continue;/* we hit a non-available Var */
894+
895+
/*
896+
* If requested, reject expressions that are not parallel-safe. We
897+
* check this last because it's a rather expensive test.
898+
*/
899+
if (require_parallel_safe&&
900+
!is_parallel_safe(root, (Node*)em->em_expr))
901+
continue;
902+
903+
returnem;/* found usable expression */
904+
}
905+
906+
returnNULL;
907+
}
908+
909+
/*
910+
* is_exprlist_member
911+
* Subroutine for find_computable_ec_member: is "node" in "exprs"?
912+
*
913+
* Per the requirements of that function, "exprs" might or might not have
914+
* TargetEntry superstructure.
915+
*/
916+
staticbool
917+
is_exprlist_member(Expr*node,List*exprs)
918+
{
919+
ListCell*lc;
920+
921+
foreach(lc,exprs)
922+
{
923+
Expr*expr= (Expr*)lfirst(lc);
924+
925+
if (expr&&IsA(expr,TargetEntry))
926+
expr= ((TargetEntry*)expr)->expr;
927+
928+
if (equal(node,expr))
929+
return true;
930+
}
931+
return false;
932+
}
933+
772934
/*
773935
* Find an equivalence class member expression, all of whose Vars, come from
774936
* the indicated relation.
@@ -799,71 +961,78 @@ find_em_expr_for_rel(EquivalenceClass *ec, RelOptInfo *rel)
799961
}
800962

801963
/*
802-
* Find an equivalence class member expression that can be safely used to build
803-
* a sort node using the provided relation. The rules are a subset of those
804-
* applied in prepare_sort_from_pathkeys since that function deals with sorts
805-
* that must be delayed until the last stages of query execution, while here
806-
* we only care about proactive sorts.
964+
* Find an equivalence class member expression that can be used to build
965+
* a sort node using the provided relation; return NULL if no candidate.
966+
*
967+
* To succeed, we must find an EC member that prepare_sort_from_pathkeys knows
968+
* how to sort on, given the rel's reltarget as input. There are also a few
969+
* additional constraints based on the fact that the desired sort will be done
970+
* within the scan/join part of the plan. Also, non-parallel-safe expressions
971+
* are ignored if 'require_parallel_safe'.
807972
*/
808973
Expr*
809974
find_em_expr_usable_for_sorting_rel(PlannerInfo*root,EquivalenceClass*ec,
810975
RelOptInfo*rel,boolrequire_parallel_safe)
811976
{
812-
ListCell*lc_em;
977+
PathTarget*target=rel->reltarget;
978+
EquivalenceMember*em;
979+
ListCell*lc;
813980

814981
/*
815-
* If there is more than one equivalence member matching these
816-
* requirements we'll be content to choose any one of them.
982+
* Reject volatile ECs immediately; such sorts must always be postponed.
817983
*/
818-
foreach(lc_em,ec->ec_members)
819-
{
820-
EquivalenceMember*em=lfirst(lc_em);
821-
Expr*em_expr=em->em_expr;
984+
if (ec->ec_has_volatile)
985+
returnNULL;
822986

823-
/*
824-
*We shouldn't be trying to sort by an equivalence class that
825-
* contains a constant, so no need to consider such cases any further.
826-
*/
827-
if (em->em_is_const)
828-
continue;
987+
/*
988+
*Try to find an EM directly matching some reltarget member.
989+
*/
990+
foreach(lc,target->exprs)
991+
{
992+
Expr*targetexpr= (Expr*)lfirst(lc);
829993

830-
/*
831-
* Any Vars in the equivalence class member need to come from this
832-
* relation. This is a superset of prepare_sort_from_pathkeys ignoring
833-
* child members unless they belong to the rel being sorted.
834-
*/
835-
if (!bms_is_subset(em->em_relids,rel->relids))
994+
em=find_ec_member_matching_expr(ec,targetexpr,rel->relids);
995+
if (!em)
836996
continue;
837997

838998
/*
839-
* If requested, reject expressions that are not parallel-safe.
999+
* Reject expressions involving set-returning functions, as those
1000+
* can't be computed early either. (Note: this test and the following
1001+
* one are effectively checking properties of targetexpr, so there's
1002+
* no point in asking whether some other EC member would be better.)
8401003
*/
841-
if (require_parallel_safe&& !is_parallel_safe(root,(Node*)em_expr))
1004+
if (IS_SRF_CALL((Node*)em->em_expr))
8421005
continue;
8431006

8441007
/*
845-
*Disallow SRFs sothatall of them can be evaluated at the correct
846-
*time as determined by make_sort_input_target.
1008+
*If requested, reject expressionsthatare not parallel-safe. We
1009+
*check this last because it's a rather expensive test.
8471010
*/
848-
if (IS_SRF_CALL((Node*)em_expr))
1011+
if (require_parallel_safe&&
1012+
!is_parallel_safe(root, (Node*)em->em_expr))
8491013
continue;
8501014

851-
/*
852-
* As long as the expression isn't volatile then
853-
* prepare_sort_from_pathkeys is able to generate a new target entry,
854-
* so there's no need to verify that one already exists.
855-
*
856-
* While prepare_sort_from_pathkeys has to be concerned about matching
857-
* up a volatile expression to the proper tlist entry, it doesn't seem
858-
* valuable here to expend the work trying to find a match in the
859-
* target's exprs since such a sort will have to be postponed anyway.
860-
*/
861-
if (!ec->ec_has_volatile)
862-
returnem->em_expr;
1015+
returnem->em_expr;
8631016
}
8641017

865-
/* We didn't find any suitable equivalence class expression */
866-
returnNULL;
1018+
/*
1019+
* Try to find a expression computable from the reltarget.
1020+
*/
1021+
em=find_computable_ec_member(root,ec,target->exprs,rel->relids,
1022+
require_parallel_safe);
1023+
if (!em)
1024+
returnNULL;
1025+
1026+
/*
1027+
* Reject expressions involving set-returning functions, as those can't be
1028+
* computed early either. (There's no point in looking for another EC
1029+
* member in this case; since SRFs can't appear in WHERE, they cannot
1030+
* belong to multi-member ECs.)
1031+
*/
1032+
if (IS_SRF_CALL((Node*)em->em_expr))
1033+
returnNULL;
1034+
1035+
returnem->em_expr;
8671036
}
8681037

8691038
/*

0 commit comments

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