Re: pgbench - refactor init functions with buffers
Fabien COELHO <coelho@cri.ensmp.fr>
From: Fabien COELHO <coelho@cri.ensmp.fr>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: David Steele <david@pgmasters.net>, Andres Freund <andres@anarazel.de>,
Jeevan Ladhe <jeevan.ladhe@enterprisedb.com>,
Dilip Kumar <dilipbalaut@gmail.com>,
PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Date: 2020-03-28T09:46:02Z
Lists: pgsql-hackers
Hello Tom, >> I cannot say that I "want" to fix something which already works the same >> way, because it is against my coding principles. [...] >> I counted nearly 3500 calls under src/bin. > > Yeah, that's the problem. If someone does come forward with a patch to do > that, I think it'd be summarily rejected, at least in high-traffic code > like pg_dump. The pain it'd cause for back-patching would outweigh the > value. What about "typedef StringInfoData PQExpBufferData" and replacing PQExpBuffer by StringInfo internally, just keeping the old interface around because it is there? That would remove a few hundreds clocs. ISTM that with inline and varargs macro the substition can be managed reasonably lightly, depending on what level of compatibility is required for libpq: should it be linkability, or requiring a recompilation is ok? A clear benefit is that there are quite a few utils for PQExpBuffer in "fe_utils/string_utils.c" which would become available for StringInfo, which would help using StringInfo without duplicating them. > That being the case, I'd think a better design principle is "make your > new code look like the code around it", Yep. > which would tend to weigh against introducing StringInfo uses into > pgbench when there's none there now and a bunch of PQExpBuffer instead. > So I can't help thinking the advice you're being given here is suspect. Well, that is what I was saying, but at 2 against 1, I fold. -- Fabien.
Commits
-
pgbench: Use PQExpBuffer to simplify code that constructs SQL.
- 9796f455c38e 14.0 landed
-
Make command order in test more sensible
- ad4b7aeb8443 13.0 cited