On 10/29/17 08:50, Michael Paquier wrote:
> I had a look at this patch series. Patches 1, 2 (macos headers indeed
> show that NSUNLINKMODULE_OPTION_NONE is set to 0x0), 3 to 7 look fine
> to me.

Committed 4 and 5 together.

> I spotted a couple of other things while looking at your patches and
> the code tree.
> -   return (ginCompareItemPointers(&btree->itemptr, iptr) > 0) ? TRUE : FALSE;
> +   return (ginCompareItemPointers(&btree->itemptr, iptr) > 0) ? true : false;
> You could simplify that at the same time by removing such things. The
> "false : true" pattern is less frequent than the "true : false"
> pattern.

I have found many more instances like that.  It might be worth
simplifying a bit, but that sounds like a separate undertaking.

> Some comments in the code still mention FALSE or TRUE:
> - hashsearch.c uses FALSE in some comments.
> - In execExpr.c, ExecCheck mentions TRUE.

That one is an SQL TRUE, so I left it.

> - AggStateIsShared mentions TRUE and FALSE.
> - In arrayfuncs.c, ReadArrayStr, CopyArrayEls and ReadArrayBinary use TRUE.

Fixed the other ones.

Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to