On 09/02/2014 11:52 AM, Fabien COELHO wrote:

I've changed the loop slightly.  Do you find this more readable than the way
the loop was previously written?

It is 50% better:-)

It is no big deal, but I still fail to find the remaining continue as
usefull in this case. If you remove the "continue" line and invert the
condition, it works exactly the same, so it is just one useless
instruction within that loop. From a logical point of view the loop is
looking for '%' and then check whether the next char is '%' or not, so the
straightforward code helps my understanding as it does exactly that, and
the continue is just an hindrance to comprehension.

Note that I would buy it if it helped avoid indenting further a
significant portion of complex code, but this is not the case here.

FWIW, I agree.

[doc] I've incorporated these changes into this version of the patch,
with small changes.

Ok.

With elog(ERROR, ..) it's still reported, but the user isn't fooled into
thinking that the error is to be expected, and hopefully we would see a bug
report.  If it's impossible to tell the two errors apart, we might have
subtly broken code carried around for who knows how long.

Ok.

In that case, it would make sense to keep distinct wordings of both
exceptions in the execution code, so that they also can be set apart,
i.e. keep the "too many/few" somewhere in the error?

Well, you can do "set log_error_verbosity='verbose'" if you run into that.

I think this patch has been thoroughly reviewed now. Committed, thanks!

- Heikki



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

Reply via email to