Sort is actually PlanState?

Started by Hitoshi Haradaover 15 years ago3 messageshackers
Jump to latest
#1Hitoshi Harada
umi.tanuki@gmail.com

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+16-21
#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