Re: Common Table Expressions (WITH RECURSIVE) patch

Tatsuo Ishii <ishii@postgresql.org> writes:
>> PlanState.has_recursivescan seems like a complete kluge.  Can't it just be
>> removed?  It looks to me like it is working around bugs that hopefully aren't
>> there anymore.  There is certainly no reason why a recursive CTE should be
>> more in need of rescanning than any other kind of plan.

> I don't think so. Recursion plan needs the hash table used by sublan
> be re-created at each recursion loop stage. Remember that in each
> evaluation of recursive plan, the recursive name is replaced by a
> working table which is holding previous evalution result of recursion
> stage. Thus the hash table corresponding to the work table needs to
> be re-created.

Oh, I see.  I keep getting confused about whether RecursiveScan is at the
top or the bottom of the recursion plan tree :-(.  Maybe it would help
to use a different name for it?  RecursionInjector or something like
that?

>> If it is needed then
>> the current implementation is completely broken anyway, since it would only
>> detect a RecursiveScan node that is directly underneath an agg or hash node.

> Yeah, that's right. What I have in my mind is to implement something
> similar to UpdateChangedParamSet family like mechanism which will
> inherit working table change event to child node.

I think it could actually *be* UpdateChangedParamSet, if you just
associate some otherwise-unused Param with each RecursiveScan node,
and have the Recursion node signal a change of that Param when it
revises the work table.

In fact, why not combine that with getting rid of the klugy addition to
ExecutorState?  Make the param be actually useful: it could contain
some internal datastructure that passes the work table down to the
RecursiveScan node.
        regards, tom lane