Обсуждение: 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
Вложения
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 alsobe used for nodes* that scan the output of an underlying plan node --- in that case,* only ScanTupleSlotis 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