Takahiro Itagaki wrote:
Michael Paquier <michael.paqu...@gmail.com> wrote:

Yeah the grammar ":variable" is used to set a parameter, the user will feel
disorientated if there is no support.
The attached patch supports this new case, based on Itagaki-san's last
version.
What is the spec of "\setshell :variable" ?
Literally interpreted, it declares a variable with name ":variable".
But pgbench cannot use such variables because only variables named
with alphabets, numbers, and _ can be accepted. Should we forbid to
assign variables that name contain invalid characters instead?
After reviewing this a bit more I've realized my idea wasn't very good here. If this is what we're already accepting:

\set nbranches :scale

It's completely reasonable to say that *only* this works:

\setshell aid expr 1 + :naccounts

While this does not:

\setshell :aid expr 1 + :naccounts

And I was wrong to ask that it should. Sorry to have lead you both around over this, my bad.

Proposed patch attached. It checks for variable names whether they
consist of isalnum or _. The check is only performed when a new variable
is assigned to avoid overhead. In normal workload, variables with the
same names are re-used repeatedly. I don't think the additinal check would
decrease performance of pgbench.
This is interesting, but we're already past where this should have wrapped up and I'm not sure if it's worth fiddling with this code path right now. I think your idea is good, just outside of what we should fool with right now and I'm out of time to work on testing it further too.

How about this: the version you updated at http://archives.postgresql.org/pgsql-hackers/2009-12/msg00847.php , your pgbenchshell_20091210.patch, worked perfectly for me except for one complaint I've since dropped. How about we move toward committing that one, and maybe we look at this whole variable name handling improvement as a separate patch later if you think it's worth pursuing? At this point I'd just like to have a version of the shell/setshell feature go into the pgbench code without dragging things out further into new territory.

--

Greg Smith    2ndQuadrant   Baltimore, MD
PostgreSQL Training, Services and Support
g...@2ndquadrant.com  www.2ndQuadrant.com


--
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