Re: [HACKERS] pgbench - allow to store select results intovariables

Hello Alvaro,

> Here are my proposed final changes.

Thanks again for improving the patch!

> I noticed that you were allocating the prefixes for all cases even when 
> there were no cset/gset in the command; I changed it to delay the 
> allocation until needed.

Ok, why not.

> I also noticed you were skipping the Meta enum dance for no good reason;

Indeed. I think that the initial version of the patch was before the 
"dance" was added, and it did not keep up with it.

> added that makes conditionals simpler.  The read_response routine seemed 
> misplaced and I gave it a name in a style closer to the rest of the 
> pgbench code.

Fine.

> Also, you were missing to free the ->lines pqexpbuffer when the command 
> is discarded.  I grant that the free_command() stuff might be bogus 
> since it's only tested with a command that's barely initialized, but it 
> seems better to make it bogus in this way (fixable if we ever extend its 
> use) than to forever leak memory silently.

Ok.

However, I switched "pg_free" to "termPQExpBuffer", which seems more 
appropriate, even if it just does the same thing. I also ensured that 
prefixes are allocated & freed. I've commented about expr which is not 
freed.

> I didn't test this beyond running "make check".

That's a good start.

I'm not keen on having the command array size checked and updated *after* 
the command is appended, even if the initial allocation ensures that there 
is no overflow, but I let it as is.

Further tests did not yield any new issue.

Attached a v29 with the above minor changes wrt your version.

-- 
Fabien.