Robert Haas <robertmh...@gmail.com> wrote: > > The attached patch verifies variable names at definition. > > $ pgbench -D var:name=value > > (global): invalid variable name 'var:name' > > I have reviewed this patch. I think that the basic idea of rejecting > invalid variable names is probably a good one, but I'm not totally > happy with the implementation. In particular: > > 1. The code that prints the invalid variable name message seems > bizarrely complex and inexplicable relative to the way errors are > handled elsewhere in the code. If we really need to do this, it > should be in its own function, not buried inside putVariable(), but > isn't there some simpler alternative?
We can remove the complexity if we give up showing the command (arg0) in error messages. Shall we remove it? Simplified patch attached. > 2. I think it would be worth abstracting the actual test into a > separate function, like isLegalVariableName(). > 3. In the department of nitpicking, I believe that the for loop test > should be written as something like name[i] != '\0' rather than just > name[i], for clarity. Adjusted. Regards, --- Takahiro Itagaki NTT Open Source Software Center
pgbench_verify_varname_20100104.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
