On Mon, Aug 27, 2012 at 7:52 AM, Dimitri Fontaine
<dimitri@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