Thread

1. Verifying variable names in pgbench

   Takahiro Itagaki <itagaki.takahiro@oss.ntt.co.jp> — 2009-12-28T03:38:59Z

   We can define variables with any names in pgbench,
   but only can refer them with names that consist of [A-Za-z0-9_]+ .
   It could cause confusion discussed here:
   http://archives.postgresql.org/message-id/4B272833.8080500@2ndquadrant.com
   
   The attached patch verifies variable names at definition.
       $ pgbench -D var:name=value
       (global): invalid variable name 'var:name'
   
   It would help users to determine why their custom scripts failed
   when they misuse "\setXXX :varname" (the colon should be removed).
   
   Regards,
   ---
   Takahiro Itagaki
   NTT Open Source Software Center
   

2. Re: Verifying variable names in pgbench

   Takahiro Itagaki <itagaki.takahiro@oss.ntt.co.jp> — 2010-01-04T01:34:42Z

   Robert Haas <robertmhaas@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
   
   

3. Re: Verifying variable names in pgbench

   Takahiro Itagaki <itagaki.takahiro@oss.ntt.co.jp> — 2010-01-05T03:00:51Z

   Takahiro Itagaki <itagaki.takahiro@oss.ntt.co.jp> wrote:
   
   > We can remove the complexity if we give up showing the command (arg0)
   > in error messages. Shall we remove it? Simplified patch attached.
   
   Here is the proposal for the arg0 issue.
   I added "context" argument to putVariable(). The context is a command
   name for \setXXX commands, 'option' for -D option, or 'startup' for
   internal assignment in startup.
   
   Regards,
   ---
   Takahiro Itagaki
   NTT Open Source Software Center
   

4. Re: Verifying variable names in pgbench

   Robert Haas <robertmhaas@gmail.com> — 2010-01-05T03:33:47Z

   On Mon, Jan 4, 2010 at 10:00 PM, Takahiro Itagaki
   <itagaki.takahiro@oss.ntt.co.jp> wrote:
   >
   > Takahiro Itagaki <itagaki.takahiro@oss.ntt.co.jp> wrote:
   >
   >> We can remove the complexity if we give up showing the command (arg0)
   >> in error messages. Shall we remove it? Simplified patch attached.
   >
   > Here is the proposal for the arg0 issue.
   > I added "context" argument to putVariable(). The context is a command
   > name for \setXXX commands, 'option' for -D option, or 'startup' for
   > internal assignment in startup.
   
   What is currently done for other, similar error messages?
   
   ...Robert
   
   

5. Re: Verifying variable names in pgbench

   Takahiro Itagaki <itagaki.takahiro@oss.ntt.co.jp> — 2010-01-05T03:44:38Z

   Robert Haas <robertmhaas@gmail.com> wrote:
   
   > What is currently done for other, similar error messages?
   
   Current error messages are:
     for commands: "<context>: out of memory"
     for others  : "Couldn't allocate memory for variable"
   
   The new message is: "<context>: out of memory for variable '<name>'"
   
   Regards,
   ---
   Takahiro Itagaki
   NTT Open Source Software Center
   
   
   
   

6. Re: Verifying variable names in pgbench

   Robert Haas <robertmhaas@gmail.com> — 2010-01-05T03:51:10Z

   On Mon, Jan 4, 2010 at 10:44 PM, Takahiro Itagaki
   <itagaki.takahiro@oss.ntt.co.jp> wrote:
   > Robert Haas <robertmhaas@gmail.com> wrote:
   >
   >> What is currently done for other, similar error messages?
   >
   > Current error messages are:
   >  for commands: "<context>: out of memory"
   >  for others  : "Couldn't allocate memory for variable"
   >
   > The new message is: "<context>: out of memory for variable '<name>'"
   
   OK, I see.  I think this is reasonable, then.
   
   ...Robert