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

Commit141225b

Browse files
committed
Mop up some undue familiarity with the innards of Bitmapsets.
nodeAppend.c used non-nullness of appendstate->as_valid_subplans asa state flag to indicate whether it'd done ExecFindMatchingSubPlans(or some sufficient approximation to that). This was prettyquestionable even in the beginning, since it wouldn't really workright if there are no valid subplans. It got more questionableafter commit27e1f14 added logic that could reduce as_valid_subplansto an empty set: at that point we were depending on unspecifiedbehavior of bms_del_members, namely that it'd not return an emptyset as NULL. It's about to start doing that, which breaks thislogic entirely. Hence, add a separate boolean flag to signalwhether as_valid_subplans has been computed.Also fix a previously-cosmetic bug in nodeAgg.c, wherein it ignoredthe return value of bms_del_member instead of updating its pointer.Patch by me; thanks to Nathan Bossart and Richard Guo for review.Discussion:https://postgr.es/m/1159933.1677621588@sss.pgh.pa.us
1 parent462bb7f commit141225b

File tree

3 files changed

+25
-15
lines changed

3 files changed

+25
-15
lines changed

‎src/backend/executor/nodeAgg.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1642,7 +1642,7 @@ find_hash_columns(AggState *aggstate)
16421642
perhash->hashGrpColIdxHash[i]=i+1;
16431643
perhash->numhashGrpCols++;
16441644
/* delete already mapped columns */
1645-
bms_del_member(colnos,grpColIdx[i]);
1645+
colnos=bms_del_member(colnos,grpColIdx[i]);
16461646
}
16471647

16481648
/* and add the remaining columns */

‎src/backend/executor/nodeAppend.c

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,10 @@ ExecInitAppend(Append *node, EState *estate, int eflags)
157157
* later calls to ExecFindMatchingSubPlans.
158158
*/
159159
if (!prunestate->do_exec_prune&&nplans>0)
160+
{
160161
appendstate->as_valid_subplans=bms_add_range(NULL,0,nplans-1);
162+
appendstate->as_valid_subplans_identified= true;
163+
}
161164
}
162165
else
163166
{
@@ -170,6 +173,7 @@ ExecInitAppend(Append *node, EState *estate, int eflags)
170173
Assert(nplans>0);
171174
appendstate->as_valid_subplans=validsubplans=
172175
bms_add_range(NULL,0,nplans-1);
176+
appendstate->as_valid_subplans_identified= true;
173177
appendstate->as_prune_state=NULL;
174178
}
175179

@@ -259,7 +263,7 @@ ExecInitAppend(Append *node, EState *estate, int eflags)
259263
appendstate->as_asyncresults= (TupleTableSlot**)
260264
palloc0(nasyncplans*sizeof(TupleTableSlot*));
261265

262-
if (appendstate->as_valid_subplans!=NULL)
266+
if (appendstate->as_valid_subplans_identified)
263267
classify_matching_subplans(appendstate);
264268
}
265269

@@ -414,13 +418,11 @@ ExecReScanAppend(AppendState *node)
414418
bms_overlap(node->ps.chgParam,
415419
node->as_prune_state->execparamids))
416420
{
421+
node->as_valid_subplans_identified= false;
417422
bms_free(node->as_valid_subplans);
418423
node->as_valid_subplans=NULL;
419-
if (nasyncplans>0)
420-
{
421-
bms_free(node->as_valid_asyncplans);
422-
node->as_valid_asyncplans=NULL;
423-
}
424+
bms_free(node->as_valid_asyncplans);
425+
node->as_valid_asyncplans=NULL;
424426
}
425427

426428
for (i=0;i<node->as_nplans;i++)
@@ -574,11 +576,14 @@ choose_next_subplan_locally(AppendState *node)
574576
if (node->as_nasyncplans>0)
575577
{
576578
/* We'd have filled as_valid_subplans already */
577-
Assert(node->as_valid_subplans);
579+
Assert(node->as_valid_subplans_identified);
578580
}
579-
elseif (node->as_valid_subplans==NULL)
581+
elseif (!node->as_valid_subplans_identified)
582+
{
580583
node->as_valid_subplans=
581584
ExecFindMatchingSubPlans(node->as_prune_state, false);
585+
node->as_valid_subplans_identified= true;
586+
}
582587

583588
whichplan=-1;
584589
}
@@ -640,10 +645,11 @@ choose_next_subplan_for_leader(AppendState *node)
640645
* run-time pruning is disabled then the valid subplans will always be
641646
* set to all subplans.
642647
*/
643-
if (node->as_valid_subplans==NULL)
648+
if (!node->as_valid_subplans_identified)
644649
{
645650
node->as_valid_subplans=
646651
ExecFindMatchingSubPlans(node->as_prune_state, false);
652+
node->as_valid_subplans_identified= true;
647653

648654
/*
649655
* Mark each invalid plan as finished to allow the loop below to
@@ -715,10 +721,12 @@ choose_next_subplan_for_worker(AppendState *node)
715721
* run-time pruning is disabled then the valid subplans will always be set
716722
* to all subplans.
717723
*/
718-
elseif (node->as_valid_subplans==NULL)
724+
elseif (!node->as_valid_subplans_identified)
719725
{
720726
node->as_valid_subplans=
721727
ExecFindMatchingSubPlans(node->as_prune_state, false);
728+
node->as_valid_subplans_identified= true;
729+
722730
mark_invalid_subplans_as_finished(node);
723731
}
724732

@@ -866,10 +874,11 @@ ExecAppendAsyncBegin(AppendState *node)
866874
Assert(node->as_nasyncplans>0);
867875

868876
/* If we've yet to determine the valid subplans then do so now. */
869-
if (node->as_valid_subplans==NULL)
877+
if (!node->as_valid_subplans_identified)
870878
{
871879
node->as_valid_subplans=
872880
ExecFindMatchingSubPlans(node->as_prune_state, false);
881+
node->as_valid_subplans_identified= true;
873882

874883
classify_matching_subplans(node);
875884
}
@@ -1143,6 +1152,7 @@ classify_matching_subplans(AppendState *node)
11431152
{
11441153
Bitmapset*valid_asyncplans;
11451154

1155+
Assert(node->as_valid_subplans_identified);
11461156
Assert(node->as_valid_asyncplans==NULL);
11471157

11481158
/* Nothing to do if there are no valid subplans. */
@@ -1161,9 +1171,8 @@ classify_matching_subplans(AppendState *node)
11611171
}
11621172

11631173
/* Get valid async subplans. */
1164-
valid_asyncplans=bms_copy(node->as_asyncplans);
1165-
valid_asyncplans=bms_int_members(valid_asyncplans,
1166-
node->as_valid_subplans);
1174+
valid_asyncplans=bms_intersect(node->as_asyncplans,
1175+
node->as_valid_subplans);
11671176

11681177
/* Adjust the valid subplans to contain sync subplans only. */
11691178
node->as_valid_subplans=bms_del_members(node->as_valid_subplans,

‎src/include/nodes/execnodes.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1350,6 +1350,7 @@ struct AppendState
13501350
ParallelAppendState*as_pstate;/* parallel coordination info */
13511351
Sizepstate_len;/* size of parallel coordination info */
13521352
structPartitionPruneState*as_prune_state;
1353+
boolas_valid_subplans_identified;/* is as_valid_subplans valid? */
13531354
Bitmapset*as_valid_subplans;
13541355
Bitmapset*as_valid_asyncplans;/* valid asynchronous plans indexes */
13551356
bool(*choose_next_subplan) (AppendState*);

0 commit comments

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