Re: pgbench: new feature allowing to launch shell commands

Takahiro Itagaki wrote:
> Michael Paquier <michael(dot)paquier(at)gmail(dot)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
greg(at)2ndQuadrant(dot)com www.2ndQuadrant.com