From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
---|---|
To: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, Stephen Frost <sfrost(at)snowman(dot)net>, Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [HACKERS] pgbench - allow to store select results into variables |
Date: | 2019-01-10 09:12:26 |
Message-ID: | alpine.DEB.2.21.1901100923110.13058@lancre |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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.
Attachment | Content-Type | Size |
---|---|---|
pgbench-into-29.patch | text/x-diff | 34.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | David Rowley | 2019-01-10 09:12:46 | Re: speeding up planning with partitions |
Previous Message | Amit Langote | 2019-01-10 08:58:10 | Re: [Sender Address Forgery]Re: error message when subscription target is a partitioned table |