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

On Thu, Jul 18, 2013 at 10:19 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

>
> > Because simpler code is less likely to have bugs and is easier to
> > maintain.
>
> I agree with that point, but one should also remember Polya's Inventor's
> Paradox: the more general problem may be easier to solve. That is, if
> done right, code that fully flattens an AND tree might actually be
> simpler than code that does just a subset of the transformation. The
> current patch fails to meet this expectation,

The current patch does completely flatten any type of tree (left/right-deep
or bushy) without recursing, and right-deep and bushy tree processing is
what Robert is recommending to defer to recursive processing. Maybe I
haven't considered a case where it doesn't flatten the tree; do you have an
example in mind.

> but maybe you just haven't
> thought about it the right way.
>
> My concerns about this patch have little to do with that, though, and
> much more to do with the likelihood that it breaks some other piece of
> code that is expecting AND/OR to be strictly binary operators, which
> is what they've always been in parsetrees that haven't reached the
> planner. It doesn't appear to me that you've done any research on that
> point whatsoever

No, I haven't, and I might not be able to research it for a few more weeks.

> you have not even updated the comment for BoolExpr
> (in primnodes.h) that this patch falsifies.
>

I will fix that.

Best regards,
--
Gurjeet Singh

http://gurjeet.singh.im/

EnterpriseDB Inc.