Thread

1. Sort is actually PlanState?

   Hitoshi Harada <umi.tanuki@gmail.com> — 2010-11-02T00:55:47Z

   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
   

2. Re: Sort is actually PlanState?

   Tom Lane <tgl@sss.pgh.pa.us> — 2010-11-02T01:06:22Z

   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
   
   

3. Re: Sort is actually PlanState?

   Hitoshi Harada <umi.tanuki@gmail.com> — 2010-11-02T02:49:13Z

   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