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

Commitdb9f0e1

Browse files
committed
Postpone creation of pathkeys lists to fix bug #8049.
This patch gets rid of the concept of, and infrastructure for,non-canonical PathKeys; we now only ever create canonical pathkey lists.The need for non-canonical pathkeys came from the desire to havegrouping_planner initialize query_pathkeys and related pathkey lists beforecalling query_planner. However, since query_planner didn't actually *do*anything with those lists before they'd been made canonical, we can get ridof the whole mess by just not creating the lists at all until the pointwhere we formerly canonicalized them.There are several ways in which we could implement that without makingquery_planner itself deal with grouping/sorting features (which aresupposed to be the province of grouping_planner). I chose to add acallback function to query_planner's API; other alternatives would haverequired adding more fields to PlannerInfo, which while not bad in itselfwould create an ABI break for planner-related plugins in the 9.2 releaseseries. This still breaks ABI for anything that calls query_plannerdirectly, but it seems somewhat unlikely that there are any such plugins.I had originally conceived of this change as merely a step on the way tofixing bug #8049 from Teun Hoogendoorn; but it turns out that this fixesthat bug all by itself, as per the added regression test. The reason isthat now get_eclass_for_sort_expr is adding the ORDER BY expression at theend of EquivalenceClass creation not the start, and so anything that is ina multi-member EquivalenceClass has already been created with correctem_nullable_relids. I am suspicious that there are related scenarios inwhich we still need to teach get_eclass_for_sort_expr to compute correctnullable_relids, but am not eager to risk destabilizing either 9.2 or 9.3to fix bugs that are only hypothetical. So for the moment, do this andstop here.Back-patch to 9.2 but not to earlier branches, since they don't exhibitthis bug for lack of join-clause-movement logic that depends onem_nullable_relids being correct. (We might have to revisit that choiceif any related bugs turn up.) In 9.2, don't change the signature ofmake_pathkeys_for_sortclauses nor remove canonicalize_pathkeys, so asnot to risk more plugin breakage than we have to.
1 parent5fc8937 commitdb9f0e1

File tree

12 files changed

+246
-314
lines changed

12 files changed

+246
-314
lines changed

‎src/backend/nodes/equalfuncs.c

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -728,22 +728,8 @@ _equalFromExpr(const FromExpr *a, const FromExpr *b)
728728
staticbool
729729
_equalPathKey(constPathKey*a,constPathKey*b)
730730
{
731-
/*
732-
* This is normally used on non-canonicalized PathKeys, so must chase up
733-
* to the topmost merged EquivalenceClass and see if those are the same
734-
* (by pointer equality).
735-
*/
736-
EquivalenceClass*a_eclass;
737-
EquivalenceClass*b_eclass;
738-
739-
a_eclass=a->pk_eclass;
740-
while (a_eclass->ec_merged)
741-
a_eclass=a_eclass->ec_merged;
742-
b_eclass=b->pk_eclass;
743-
while (b_eclass->ec_merged)
744-
b_eclass=b_eclass->ec_merged;
745-
if (a_eclass!=b_eclass)
746-
return false;
731+
/* We assume pointer equality is sufficient to compare the eclasses */
732+
COMPARE_SCALAR_FIELD(pk_eclass);
747733
COMPARE_SCALAR_FIELD(pk_opfamily);
748734
COMPARE_SCALAR_FIELD(pk_strategy);
749735
COMPARE_SCALAR_FIELD(pk_nulls_first);

‎src/backend/optimizer/README

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -589,16 +589,6 @@ since they are easily compared to the pathkeys of a potential candidate
589589
path. So, SortGroupClause lists are turned into pathkeys lists for use
590590
inside the optimizer.
591591

592-
Because we have to generate pathkeys lists from the sort clauses before
593-
we've finished EquivalenceClass merging, we cannot use the pointer-equality
594-
method of comparing PathKeys in the earliest stages of the planning
595-
process. Instead, we generate "non canonical" PathKeys that reference
596-
single-element EquivalenceClasses that might get merged later. After we
597-
complete EquivalenceClass merging, we replace these with "canonical"
598-
PathKeys that reference only fully-merged classes, and after that we make
599-
sure we don't generate more than one copy of each "canonical" PathKey.
600-
Then it is safe to use pointer comparison on canonical PathKeys.
601-
602592
An additional refinement we can make is to insist that canonical pathkey
603593
lists (sort orderings) do not mention the same EquivalenceClass more than
604594
once. For example, in all these cases the second sort column is redundant,
@@ -651,12 +641,11 @@ mergejoinable clauses found in the quals. At the end of this process,
651641
we know all we can know about equivalence of different variables, so
652642
subsequently there will be no further merging of EquivalenceClasses.
653643
At that point it is possible to consider the EquivalenceClasses as
654-
"canonical" and build canonical PathKeys that reference them. Before
655-
we reach that point (actually, before entering query_planner at all)
656-
we also ensure that we have constructed EquivalenceClasses for all the
657-
expressions used in the query's ORDER BY and related clauses. These
658-
classes might or might not get merged together, depending on what we
659-
find in the quals.
644+
"canonical" and build canonical PathKeys that reference them. At this
645+
time we construct PathKeys for the query's ORDER BY and related clauses.
646+
(Any ordering expressions that do not appear elsewhere will result in
647+
the creation of new EquivalenceClasses, but this cannot result in merging
648+
existing classes, so canonical-ness is not lost.)
660649

661650
Because all the EquivalenceClasses are known before we begin path
662651
generation, we can use them as a guide to which indexes are of interest:

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -285,11 +285,19 @@ process_equivalence(PlannerInfo *root, RestrictInfo *restrictinfo,
285285
}
286286

287287
/*
288-
* Case 2: need to merge ec1 and ec2. We add ec2's items to ec1, then
289-
* set ec2's ec_merged link to point to ec1 and remove ec2 from the
290-
* eq_classes list. We cannot simply delete ec2 because that could
291-
* leave dangling pointers in existing PathKeys. We leave it behind
292-
* with a link so that the merged EC can be found.
288+
* Case 2: need to merge ec1 and ec2. This should never happen after
289+
* we've built any canonical pathkeys; if it did, those pathkeys might
290+
* be rendered non-canonical by the merge.
291+
*/
292+
if (root->canon_pathkeys!=NIL)
293+
elog(ERROR,"too late to merge equivalence classes");
294+
295+
/*
296+
* We add ec2's items to ec1, then set ec2's ec_merged link to point
297+
* to ec1 and remove ec2 from the eq_classes list. We cannot simply
298+
* delete ec2 because that could leave dangling pointers in existing
299+
* PathKeys. We leave it behind with a link so that the merged EC can
300+
* be found.
293301
*/
294302
ec1->ec_members=list_concat(ec1->ec_members,ec2->ec_members);
295303
ec1->ec_sources=list_concat(ec1->ec_sources,ec2->ec_sources);

0 commit comments

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