On Mon, Aug 27, 2012 at 7:52 AM, Dimitri Fontaine <dimi...@2ndquadrant.fr> wrote: > I've been reviewing your changes and here's a very small patch with some > details I would have spelled out differently. See what you think, I > mostly needed to edit some code to get back in shape :)
I guess I don't particularly like either of these changes. The first one is mostly harmless, but I don't really see why it's any better, and it does have the downside of traversing the string twice (once for strlen and a second time in str_toupper) instead of just once. It also makes a line wider than 80 columns, which is a bit ugly. In the second hunk, the point is that we never have to do CreateCommandTag() here at all unless either casserts are enabled or EventCacheLookup returns a non-empty list. That means that in a non-assert-enabled build, we get to skip that work altogether in the presumably-common case where there are no relevant event triggers. Your proposed change would avoid doing it twice when asserts are disabled, but the cost would be that we'd have to do it once when asserts were disabled even if no event triggers exist. I don't think that's a good trade-off. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
