On Wed, Jul 17, 2013 at 2:03 PM, Gurjeet Singh <gurjeet@singh.im> wrote:
> In v6 of the patch, I have deferred the 'pending' list initialization to
> until we actually hit a candidate right-branch. So in the common case the
> pending list will never be populated, and if we find a bushy or right-deep
> tree (for some reason an ORM/tool may choose to build AND/OR lists that may
> end being right-deep when in Postgres), then the pending list will be used
> to process them iteratively.
>
> Does that alleviate your concern about 'pending' list management causing an
> overhead.
>
> Agreed that bushy/right-deep trees are a remote corner case, but we are
> addressing a remote corner case in the first place (insanely long AND lists)
> and why not handle another remote corner case right now if it doesn't cause
> an overhead for common case.
Because simpler code is less likely to have bugs and is easier to
maintain. It's worth noting that the change you're proposing is in
fact user-visible, as demonstrated by the fact that you had to update
the regression test output:
- | WHERE (((rsl.sl_color =
rsh.slcolor) AND (rsl.sl_len_cm >= rsh.slminlen_cm)) AND
(rsl.sl_len_cm <= rsh.slmaxlen_cm));
+ | WHERE ((rsl.sl_color =
rsh.slcolor) AND (rsl.sl_len_cm >= rsh.slminlen_cm) AND (rsl.sl_len_cm
<= rsh.slmaxlen_cm));
Now, I think that change is actually an improvement, because here's
what that WHERE clause looked like when it was entered:
WHERE rsl.sl_color = rsh.slcolor AND rsl.sl_len_cm >= rsh.slminlen_cm AND rsl.sl_len_cm <=
rsh.slmaxlen_cm;
But flattening a = 1 AND (b = 1 AND c = 1 AND d = 1) AND e = 1 to a
flat list doesn't have any of the same advantages.
At the end of the day, this is a judgement call, and I'm giving you
mine. If somebody else wants to weigh in, that's fine. I can't think
of anything that would actually be outright broken under your proposed
approach, but my personal feeling is that it's better to only add the
amount of code that we know is needed to solve the problem actually
observed in practice, and no more.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company