Hello just one small notices
I dislike a name "root_bool_expr", because, there is not a expression, but expression type. Can you use "root_bool_expr_type" instead? It is little bit longer, but more correct. Same not best name is "root_char", maybe "root_bool_op_name" or root_expr_type and root_op_name ??? Have no other comments Regards Pavel 2013/6/30 Gurjeet Singh <gurj...@singh.im>: > On Tue, Jun 18, 2013 at 3:01 PM, Pavel Stehule <pavel.steh...@gmail.com> > wrote: >> >> >> related to >> >> https://commitfest.postgresql.org/action/patch_view?id=1130 >> >> http://www.postgresql.org/message-id/cabwtf4v9rsjibwe+87pk83mmm7acdrg7sz08rq-4qyme8jv...@mail.gmail.com >> >> >> * motivation: remove recursive procession of AND/OR list (hangs with >> 10062 and more subexpressions) >> >> * patch is short, clean and respect postgresql source code requirements >> * patch was applied cleanly without warnings >> * all regression tests was passed >> * I successfully evaluated expression with 100000 subexpressions >> * there is no significant slowdown >> >> possible improvements >> >> a = (A_Expr*) list_nth(pending, 0); >> >> a = (A_Expr*) linitial(pending); > > > I made that change, hesitantly. The comments above definition of linitial() > macro describe the confusion that API causes. I wanted to avoid that > confusion for new code, so I used the newer API which makes the intention > quite clear. But looking at that code closely, list_nth() causes at least 2 > function calls, and that's pretty heavy compared to the linitiali() macro. > >> >> >> not well comment >> >> should be -- "If the right branch is also an SAME condition, append it to >> the" > > > I moved that comment above the outer bock, so that the intention of the > whole do-while code block is described in one place. > >> I don't see any other issues, so after fixing comments this patch is >> ready for commit > > > Thanks for the review Pavel. > > Attached is the updated patch, v4. It has the above edits, and a few code > improvements, like not repeating the (root_kind == AEPR_AND ? .. : ..) > ternary expression. > > Best regards, > -- > Gurjeet Singh > > http://gurjeet.singh.im/ > > EnterpriseDB Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
