Re: review: Non-recursive processing of AND/OR lists

On Sun, Jul 14, 2013 at 8:27 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Wed, Jul 10, 2013 at 9:02 PM, Josh Berkus <josh(at)agliodbs(dot)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,
--
Gurjeet Singh

http://gurjeet.singh.im/

EnterpriseDB Inc.