Re: Size of Path nodes

On Sat, Dec 5, 2015 at 12:35 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Amit Kapila <amit.kapila16@gmail.com> writes:
>> To add to whatever has been said above, intention of adding that flag
>> was also to avoid adding new nodes for parallelism.  Basically modify
>> the existing nodes (like SeqScan) to take care of whatever is needed
>> for parallel execution.
>
> TBH, I would say that that's a damn-fool idea.  I think you should instead
> create a separate ParallelSeqScan path type and plan type, and the same
> for every other thing that becomes parallel-aware.  The planner does not
> normally[1] use the same path type to represent two fundamentally different
> execution plans with enormously different cost estimates, but that is the
> direction you want to push in for parallel query.  I think it will lead to
> a mess: lots of unreadable code that has to do things in a way unlike the
> code around it, and lots of bugs-of-omission in places that should have
> distinguished seq and parallel cases but didn't.

Maybe.  But if we go down that path, we're eventually going to have
ParallelSeqScan, ParallelIndexScan, ParallelBitmapHeapScan,
ParallelForeignScan, ParallelAppend, ParallelHashJoin, and probably a
bunch of others.  That could lead to a lot of code duplication.  Even
for ParallelSeqScan:
src/backend/executor/nodeSeqscan.c | 136
++++++++++++++++++++++++++++++++++++++++++++++++++-----------------1 file changed, 103 insertions(+), 33 deletions(-)

Now that file has 344 lines today, but only 33 of those lines are
actual changes to pre-existing code, and most of those are mechanical.
So the effect of making that a whole separate node type would have
been to copy about 200 lines of code and comments.  That didn't seem
like a good idea.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company