Re: Verifying variable names in pgbench

From: Takahiro Itagaki <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Verifying variable names in pgbench
Date: 2010-01-04 01:34:42
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Robert Haas <robertmhaas(at)gmail(dot)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.


Takahiro Itagaki
NTT Open Source Software Center

Attachment Content-Type Size
pgbench_verify_varname_20100104.patch application/octet-stream 2.7 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Takahiro Itagaki 2010-01-04 02:10:07 Re: pg_read_file() and non-ascii input file
Previous Message Andrew Dunstan 2010-01-04 00:30:05 Re: PATCH: Add hstore_to_json()