Douglas Doole <dougdoole@gmail.com> writes:
> Would we be better off moving those cases into the while loop I added to
> avoid the recursion?
No. You can't easily avoid recursion for the merge-append case, since
that has to descend to multiple children. TBH I dislike the fact that
you did the subquery case randomly differently from the existing cases;
it should just have been added as an additional recursive case. Since
this is done only once at query startup, worrying about hypothetical
micro-performance issues seems rather misguided.
Perhaps, since this function is recursive, it ought to have a
check_stack_depth call, just to be safe. I'm dubious though that it could
ever eat more stack than the preceding node-initialization calls, so maybe
we needn't bother.
In the long run I wonder whether we should convert "push down bound" into
a generic node operation like the others managed by execAmi.c. It seems
like you could push a limit through any node type that's guaranteed not to
reduce the number of rows returned, and so we're missing cases. Maybe
they're not ones that are important in practice, but I'm unsure.
> The two casts I added (to SubqueryScanState and Node) should probably be
> changed to castNode() calls.
The SubqueryScanState cast seems fine given that it immediately follows an
IsA() check. You can't use castNode() for a cast to Node, because that's
a generic not a specific node type.
regards, tom lane