On Sun, Jul 14, 2013 at 8:27 PM, Robert Haas
<robertmhaas@gmail.com> wrote:
On Wed, Jul 10, 2013 at 9:02 PM, Josh Berkus <
josh@agliodbs.com> wrote:
>> I think it's a waste of code to try to handle bushy trees. A list is
>> not a particularly efficient representation of the pending list; this
>> will probably be slower than recusing in the common case. I'd suggest
>> keeping the logic to handle left-deep trees, which I find rather
>> elegant, but ditching the pending list.
Somehow I find it hard to believe that recursing would be more efficient than processing the items right there. The recursion is not direct either; transformExprRecurse() is going to call this function again, but after a few more switch-case comparisons.
Agreed that there's overhead in allocating list items, but is it more overhead than pushing functions on the call stack? Not sure, so I leave it to others who understand such things better than I do.
If by common-case you mean a list of just one logical AND/OR operator, then I agree that creating and destroying a list may incur overhead that is relatively very expensive. To that end, I have altered the patch, attached, to not build a pending list until we encounter a node with root_expr_kind in a right branch.
We're getting bushy-tree processing with very little extra code, but if you deem it not worthwhile or adding complexity, please feel free to rip it out.
>
> Is there going to be further discussion of this patch, or do I return it?
Considering it's not been updated, nor my comments responded to, in
almost two weeks, I think we return it at this point.
Sorry, I didn't notice that this patch was put back in 'Waiting on Author' state.
Best regards,
--