Sort is actually PlanState?
I wonder why SortState is a ScanState. As far as I know ScanState
means the node may need projection and/or qualification, or it scans
some relation, but Sort actually doesn't do such things. I also tried
to modify SortState as PlanState as in the attached patch and
regression test passed. Do I misunderstand ScanState?
Regards,
--
Hitoshi Harada
Attachments:
sort-plan.20101102.patchapplication/octet-stream; name=sort-plan.20101102.patchDownload
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index f494ec9..ac1bb44 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -1388,7 +1388,7 @@ show_upper_qual(List *qual, const char *qlabel,
static void
show_sort_keys(SortState *sortstate, List *ancestors, ExplainState *es)
{
- Sort *plan = (Sort *) sortstate->ss.ps.plan;
+ Sort *plan = (Sort *) sortstate->ps.plan;
show_sort_keys_common((PlanState *) sortstate,
plan->numCols, plan->sortColIdx,
diff --git a/src/backend/executor/nodeSort.c b/src/backend/executor/nodeSort.c
index 1f5ae98..9d9afbf 100644
--- a/src/backend/executor/nodeSort.c
+++ b/src/backend/executor/nodeSort.c
@@ -49,7 +49,7 @@ ExecSort(SortState *node)
SO1_printf("ExecSort: %s\n",
"entering routine");
- estate = node->ss.ps.state;
+ estate = node->ps.state;
dir = estate->es_direction;
tuplesortstate = (Tuplesortstate *) node->tuplesortstate;
@@ -60,7 +60,7 @@ ExecSort(SortState *node)
if (!node->sort_Done)
{
- Sort *plannode = (Sort *) node->ss.ps.plan;
+ Sort *plannode = (Sort *) node->ps.plan;
PlanState *outerNode;
TupleDesc tupDesc;
@@ -133,7 +133,7 @@ ExecSort(SortState *node)
* Get the first or next tuple from tuplesort. Returns NULL if no more
* tuples.
*/
- slot = node->ss.ps.ps_ResultTupleSlot;
+ slot = node->ps.ps_ResultTupleSlot;
(void) tuplesort_gettupleslot(tuplesortstate,
ScanDirectionIsForward(dir),
slot);
@@ -159,8 +159,8 @@ ExecInitSort(Sort *node, EState *estate, int eflags)
* create state structure
*/
sortstate = makeNode(SortState);
- sortstate->ss.ps.plan = (Plan *) node;
- sortstate->ss.ps.state = estate;
+ sortstate->ps.plan = (Plan *) node;
+ sortstate->ps.state = estate;
/*
* We must have random access to the sort output to do backward scan or
@@ -187,8 +187,7 @@ ExecInitSort(Sort *node, EState *estate, int eflags)
*
* sort nodes only return scan tuples from their sorted relation.
*/
- ExecInitResultTupleSlot(estate, &sortstate->ss.ps);
- ExecInitScanTupleSlot(estate, &sortstate->ss);
+ ExecInitResultTupleSlot(estate, &sortstate->ps);
/*
* initialize child nodes
@@ -204,9 +203,8 @@ ExecInitSort(Sort *node, EState *estate, int eflags)
* initialize tuple type. no need to initialize projection info because
* this node doesn't do projections.
*/
- ExecAssignResultTypeFromTL(&sortstate->ss.ps);
- ExecAssignScanTypeFromOuterPlan(&sortstate->ss);
- sortstate->ss.ps.ps_ProjInfo = NULL;
+ ExecAssignResultTypeFromTL(&sortstate->ps);
+ sortstate->ps.ps_ProjInfo = NULL;
SO1_printf("ExecInitSort: %s\n",
"sort node initialized");
@@ -224,12 +222,8 @@ ExecEndSort(SortState *node)
SO1_printf("ExecEndSort: %s\n",
"shutting down sort node");
- /*
- * clean out the tuple table
- */
- ExecClearTuple(node->ss.ss_ScanTupleSlot);
/* must drop pointer to sort result tuple */
- ExecClearTuple(node->ss.ps.ps_ResultTupleSlot);
+ ExecClearTuple(node->ps.ps_ResultTupleSlot);
/*
* Release tuplesort resources
@@ -298,7 +292,7 @@ ExecReScanSort(SortState *node)
return;
/* must drop pointer to sort result tuple */
- ExecClearTuple(node->ss.ps.ps_ResultTupleSlot);
+ ExecClearTuple(node->ps.ps_ResultTupleSlot);
/*
* If subnode is to be rescanned then we forget previous sort results; we
@@ -307,7 +301,7 @@ ExecReScanSort(SortState *node)
*
* Otherwise we can just rewind and rescan the sorted output.
*/
- if (node->ss.ps.lefttree->chgParam != NULL ||
+ if (node->ps.lefttree->chgParam != NULL ||
node->bounded != node->bounded_Done ||
node->bound != node->bound_Done ||
!node->randomAccess)
@@ -320,8 +314,8 @@ ExecReScanSort(SortState *node)
* if chgParam of subnode is not null then plan will be re-scanned by
* first ExecProcNode.
*/
- if (node->ss.ps.lefttree->chgParam == NULL)
- ExecReScan(node->ss.ps.lefttree);
+ if (node->ps.lefttree->chgParam == NULL)
+ ExecReScan(node->ps.lefttree);
}
else
tuplesort_rescan((Tuplesortstate *) node->tuplesortstate);
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index b78ee35..255a3b8 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1541,7 +1541,8 @@ typedef struct MaterialState
*/
typedef struct SortState
{
- ScanState ss; /* its first field is NodeTag */
+// ScanState ss; /* its first field is NodeTag */
+ PlanState ps; /* its first field is NodeTag */
bool randomAccess; /* need random access to sort output? */
bool bounded; /* is the result set bounded? */
int64 bound; /* if bounded, how many tuples are needed */
Hitoshi Harada <umi.tanuki@gmail.com> writes:
I wonder why SortState is a ScanState. As far as I know ScanState
means the node may need projection and/or qualification, or it scans
some relation, but Sort actually doesn't do such things.
No, not really. Per the comment for ScanState:
* ScanState extends PlanState for node types that represent
* scans of an underlying relation. It can also be used for nodes
* that scan the output of an underlying plan node --- in that case,
* only ScanTupleSlot is actually useful, and it refers to the tuple
* retrieved from the subplan.
It might be that we don't actually need ScanTupleSlot right now in the
implementation of Sort, but I don't see a good reason to remove the
field. We might just have to put it back later.
BTW, Sort is not the only node type like this --- I see at least
Material that's not projection-capable but has a ScanState.
regards, tom lane
2010/11/2 Tom Lane <tgl@sss.pgh.pa.us>:
Hitoshi Harada <umi.tanuki@gmail.com> writes:
I wonder why SortState is a ScanState. As far as I know ScanState
means the node may need projection and/or qualification, or it scans
some relation, but Sort actually doesn't do such things.No, not really. Per the comment for ScanState:
* ScanState extends PlanState for node types that represent
* scans of an underlying relation. It can also be used for nodes
* that scan the output of an underlying plan node --- in that case,
* only ScanTupleSlot is actually useful, and it refers to the tuple
* retrieved from the subplan.It might be that we don't actually need ScanTupleSlot right now in the
implementation of Sort, but I don't see a good reason to remove the
field. We might just have to put it back later.
It might reduce a few cycle used in initializing and cleaning of
ScanTupleSlot, but I basically agree it's not good reason to do it.
BTW, Sort is not the only node type like this --- I see at least
Material that's not projection-capable but has a ScanState.
Yes, during designing DtScan which is coming in the writeable CTEs I
came up with the question.
Regards,
--
Hitoshi Harada