Sort is actually PlanState?

Started by Hitoshi Haradaabout 15 years ago3 messages
#1Hitoshi Harada
umi.tanuki@gmail.com
1 attachment(s)

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 */
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Hitoshi Harada (#1)
Re: Sort is actually PlanState?

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

#3Hitoshi Harada
umi.tanuki@gmail.com
In reply to: Tom Lane (#2)
Re: Sort is actually PlanState?

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