some more pg_dump refactoring
Started by Peter Eisentrautover 5 years ago10 messages
Here is a patch to reorganize dumpFunc() and dumpAgg() in pg_dump,
similar to daa9fe8a5264a3f192efa5ddee8fb011ad9da365. Instead of
repeating the almost same large query in each version branch, use one
query and add a few columns to the SELECT list depending on the
version. This saves a lot of duplication.
I have tested this with various old versions of PostgreSQL I had
available, but a bit more random testing with old versions would be welcome.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-pg_dump-Reorganize-dumpFunc-and-dumpAgg.patchtext/plain; charset=UTF-8; name=0001-pg_dump-Reorganize-dumpFunc-and-dumpAgg.patch; x-mac-creator=0; x-mac-type=0Download
From 397a3ea142521e5618b245107d5b401135d76d3c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Tue, 23 Jun 2020 14:32:31 +0200
Subject: [PATCH] pg_dump: Reorganize dumpFunc() and dumpAgg()
Similar to daa9fe8a5264a3f192efa5ddee8fb011ad9da365, instead of
repeating the almost same large query in each version branch, use one
query and add a few columns to the SELECT list depending on the
version. This saves a lot of duplication.
---
src/bin/pg_dump/pg_dump.c | 489 ++++++++++++--------------------------
1 file changed, 154 insertions(+), 335 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index a41a3db876..b947bf86a7 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -11847,171 +11847,88 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
asPart = createPQExpBuffer();
/* Fetch function-specific details */
+ appendPQExpBuffer(query,
+ "SELECT\n"
+ "proretset,\n"
+ "prosrc,\n"
+ "probin,\n"
+ "provolatile,\n"
+ "proisstrict,\n"
+ "prosecdef,\n"
+ "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname,\n");
+
if (fout->remoteVersion >= 120000)
- {
- /*
- * prosupport was added in 12
- */
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "pg_catalog.pg_get_function_arguments(oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(oid) AS funciargs, "
- "pg_catalog.pg_get_function_result(oid) AS funcresult, "
- "array_to_string(protrftypes, ' ') AS protrftypes, "
- "prokind, provolatile, proisstrict, prosecdef, "
- "proleakproof, proconfig, procost, prorows, "
- "prosupport, proparallel, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 110000)
- {
- /*
- * prokind was added in 11
- */
+ "prosupport,\n");
+ else
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "pg_catalog.pg_get_function_arguments(oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(oid) AS funciargs, "
- "pg_catalog.pg_get_function_result(oid) AS funcresult, "
- "array_to_string(protrftypes, ' ') AS protrftypes, "
- "prokind, provolatile, proisstrict, prosecdef, "
- "proleakproof, proconfig, procost, prorows, "
- "'-' AS prosupport, proparallel, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 90600)
- {
- /*
- * proparallel was added in 9.6
- */
+ "'-' AS prosupport,\n");
+
+ if (fout->remoteVersion >= 110000)
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "pg_catalog.pg_get_function_arguments(oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(oid) AS funciargs, "
- "pg_catalog.pg_get_function_result(oid) AS funcresult, "
- "array_to_string(protrftypes, ' ') AS protrftypes, "
- "CASE WHEN proiswindow THEN 'w' ELSE 'f' END AS prokind, "
- "provolatile, proisstrict, prosecdef, "
- "proleakproof, proconfig, procost, prorows, "
- "'-' AS prosupport, proparallel, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 90500)
- {
- /*
- * protrftypes was added in 9.5
- */
+ "prokind,\n");
+ else if (fout->remoteVersion >= 80400)
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "pg_catalog.pg_get_function_arguments(oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(oid) AS funciargs, "
- "pg_catalog.pg_get_function_result(oid) AS funcresult, "
- "array_to_string(protrftypes, ' ') AS protrftypes, "
- "CASE WHEN proiswindow THEN 'w' ELSE 'f' END AS prokind, "
- "provolatile, proisstrict, prosecdef, "
- "proleakproof, proconfig, procost, prorows, "
- "'-' AS prosupport, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 90200)
- {
- /*
- * proleakproof was added in 9.2
- */
+ "CASE WHEN proiswindow THEN 'w' ELSE 'f' END AS prokind,\n");
+ else
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "pg_catalog.pg_get_function_arguments(oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(oid) AS funciargs, "
- "pg_catalog.pg_get_function_result(oid) AS funcresult, "
- "CASE WHEN proiswindow THEN 'w' ELSE 'f' END AS prokind, "
- "provolatile, proisstrict, prosecdef, "
- "proleakproof, proconfig, procost, prorows, "
- "'-' AS prosupport, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 80400)
+ "'f' AS prokind,\n");
+
+ if (fout->remoteVersion >= 90600)
+ appendPQExpBuffer(query,
+ "proparallel,\n");
+ else
+ appendPQExpBuffer(query,
+ "'u' AS proparallel,\n");
+
+ if (fout->remoteVersion >= 90500)
+ appendPQExpBuffer(query,
+ "array_to_string(protrftypes, ' ') AS protrftypes,\n");
+
+ if (fout->remoteVersion >= 90200)
+ appendPQExpBuffer(query,
+ "proleakproof,\n");
+ else
+ appendPQExpBuffer(query,
+ "false AS proleakproof,\n");
+
+ if (fout->remoteVersion >= 80400)
{
/*
* In 8.4 and up we rely on pg_get_function_arguments and
* pg_get_function_result instead of examining proallargtypes etc.
*/
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "pg_catalog.pg_get_function_arguments(oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(oid) AS funciargs, "
- "pg_catalog.pg_get_function_result(oid) AS funcresult, "
- "CASE WHEN proiswindow THEN 'w' ELSE 'f' END AS prokind, "
- "provolatile, proisstrict, prosecdef, "
- "false AS proleakproof, "
- " proconfig, procost, prorows, "
- "'-' AS prosupport, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 80300)
- {
- appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "proallargtypes, proargmodes, proargnames, "
- "'f' AS prokind, "
- "provolatile, proisstrict, prosecdef, "
- "false AS proleakproof, "
- "proconfig, procost, prorows, "
- "'-' AS prosupport, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
+ "pg_catalog.pg_get_function_arguments(oid) AS funcargs,\n"
+ "pg_catalog.pg_get_function_identity_arguments(oid) AS funciargs,\n"
+ "pg_catalog.pg_get_function_result(oid) AS funcresult,\n");
}
else if (fout->remoteVersion >= 80100)
- {
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "proallargtypes, proargmodes, proargnames, "
- "'f' AS prokind, "
- "provolatile, proisstrict, prosecdef, "
- "false AS proleakproof, "
- "null AS proconfig, 0 AS procost, 0 AS prorows, "
- "'-' AS prosupport, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
+ "proallargtypes,\n"
+ "proargmodes,\n"
+ "proargnames,\n");
else
- {
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "null AS proallargtypes, "
- "null AS proargmodes, "
- "proargnames, "
- "'f' AS prokind, "
- "provolatile, proisstrict, prosecdef, "
- "false AS proleakproof, "
- "null AS proconfig, 0 AS procost, 0 AS prorows, "
- "'-' AS prosupport, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
+ "null AS proallargtypes,\n"
+ "null AS proargmodes,\n"
+ "proargnames,\n");
+
+ if (fout->remoteVersion >= 80300)
+ appendPQExpBuffer(query,
+ "proconfig,\n"
+ "procost,\n"
+ "prorows\n");
+ else
+ appendPQExpBuffer(query,
+ "null AS proconfig,\n"
+ "0 AS procost,\n"
+ "0 AS prorows\n");
+
+ appendPQExpBuffer(query,
+ "FROM pg_catalog.pg_proc "
+ "WHERE oid = '%u'::pg_catalog.oid",
+ finfo->dobj.catId.oid);
res = ExecuteSqlQueryForSingleRow(fout, query->data);
@@ -12045,12 +11962,7 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
procost = PQgetvalue(res, 0, PQfnumber(res, "procost"));
prorows = PQgetvalue(res, 0, PQfnumber(res, "prorows"));
prosupport = PQgetvalue(res, 0, PQfnumber(res, "prosupport"));
-
- if (PQfnumber(res, "proparallel") != -1)
- proparallel = PQgetvalue(res, 0, PQfnumber(res, "proparallel"));
- else
- proparallel = NULL;
-
+ proparallel = PQgetvalue(res, 0, PQfnumber(res, "proparallel"));
lanname = PQgetvalue(res, 0, PQfnumber(res, "lanname"));
/*
@@ -12264,7 +12176,7 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
appendPQExpBuffer(q, " SUPPORT %s", prosupport);
}
- if (proparallel != NULL && proparallel[0] != PROPARALLEL_UNSAFE)
+ if (proparallel[0] != PROPARALLEL_UNSAFE)
{
if (proparallel[0] == PROPARALLEL_SAFE)
appendPQExpBufferStr(q, " PARALLEL SAFE");
@@ -13939,27 +13851,8 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
char *aggfullsig = NULL; /* full signature */
char *aggsig_tag;
PGresult *res;
- int i_aggtransfn;
- int i_aggfinalfn;
- int i_aggcombinefn;
- int i_aggserialfn;
- int i_aggdeserialfn;
- int i_aggmtransfn;
- int i_aggminvtransfn;
- int i_aggmfinalfn;
- int i_aggfinalextra;
- int i_aggmfinalextra;
- int i_aggfinalmodify;
- int i_aggmfinalmodify;
- int i_aggsortop;
- int i_aggkind;
- int i_aggtranstype;
- int i_aggtransspace;
- int i_aggmtranstype;
- int i_aggmtransspace;
int i_agginitval;
int i_aggminitval;
- int i_proparallel;
const char *aggtransfn;
const char *aggfinalfn;
const char *aggcombinefn;
@@ -13994,170 +13887,101 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
details = createPQExpBuffer();
/* Get aggregate-specific details */
+ appendPQExpBuffer(query,
+ "SELECT\n"
+ "aggtransfn,\n"
+ "aggfinalfn,\n"
+ "aggtranstype::pg_catalog.regtype,\n"
+ "agginitval,\n");
+
if (fout->remoteVersion >= 110000)
- {
- appendPQExpBuffer(query, "SELECT aggtransfn, "
- "aggfinalfn, aggtranstype::pg_catalog.regtype, "
- "aggcombinefn, aggserialfn, aggdeserialfn, aggmtransfn, "
- "aggminvtransfn, aggmfinalfn, aggmtranstype::pg_catalog.regtype, "
- "aggfinalextra, aggmfinalextra, "
- "aggfinalmodify, aggmfinalmodify, "
- "aggsortop, "
- "aggkind, "
- "aggtransspace, agginitval, "
- "aggmtransspace, aggminitval, "
- "pg_catalog.pg_get_function_arguments(p.oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(p.oid) AS funciargs, "
- "p.proparallel "
- "FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
- "WHERE a.aggfnoid = p.oid "
- "AND p.oid = '%u'::pg_catalog.oid",
- agginfo->aggfn.dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 90600)
- {
- appendPQExpBuffer(query, "SELECT aggtransfn, "
- "aggfinalfn, aggtranstype::pg_catalog.regtype, "
- "aggcombinefn, aggserialfn, aggdeserialfn, aggmtransfn, "
- "aggminvtransfn, aggmfinalfn, aggmtranstype::pg_catalog.regtype, "
- "aggfinalextra, aggmfinalextra, "
- "'0' AS aggfinalmodify, '0' AS aggmfinalmodify, "
- "aggsortop, "
- "aggkind, "
- "aggtransspace, agginitval, "
- "aggmtransspace, aggminitval, "
- "pg_catalog.pg_get_function_arguments(p.oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(p.oid) AS funciargs, "
- "p.proparallel "
- "FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
- "WHERE a.aggfnoid = p.oid "
- "AND p.oid = '%u'::pg_catalog.oid",
- agginfo->aggfn.dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 90400)
- {
- appendPQExpBuffer(query, "SELECT aggtransfn, "
- "aggfinalfn, aggtranstype::pg_catalog.regtype, "
- "'-' AS aggcombinefn, '-' AS aggserialfn, "
- "'-' AS aggdeserialfn, aggmtransfn, aggminvtransfn, "
- "aggmfinalfn, aggmtranstype::pg_catalog.regtype, "
- "aggfinalextra, aggmfinalextra, "
- "'0' AS aggfinalmodify, '0' AS aggmfinalmodify, "
- "aggsortop, "
- "aggkind, "
- "aggtransspace, agginitval, "
- "aggmtransspace, aggminitval, "
- "pg_catalog.pg_get_function_arguments(p.oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(p.oid) AS funciargs "
- "FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
- "WHERE a.aggfnoid = p.oid "
- "AND p.oid = '%u'::pg_catalog.oid",
- agginfo->aggfn.dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 80400)
- {
- appendPQExpBuffer(query, "SELECT aggtransfn, "
- "aggfinalfn, aggtranstype::pg_catalog.regtype, "
- "'-' AS aggcombinefn, '-' AS aggserialfn, "
- "'-' AS aggdeserialfn, '-' AS aggmtransfn, "
- "'-' AS aggminvtransfn, '-' AS aggmfinalfn, "
- "0 AS aggmtranstype, false AS aggfinalextra, "
- "false AS aggmfinalextra, "
- "'0' AS aggfinalmodify, '0' AS aggmfinalmodify, "
- "aggsortop, "
- "'n' AS aggkind, "
- "0 AS aggtransspace, agginitval, "
- "0 AS aggmtransspace, NULL AS aggminitval, "
- "pg_catalog.pg_get_function_arguments(p.oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(p.oid) AS funciargs "
- "FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
- "WHERE a.aggfnoid = p.oid "
- "AND p.oid = '%u'::pg_catalog.oid",
- agginfo->aggfn.dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 80100)
- {
- appendPQExpBuffer(query, "SELECT aggtransfn, "
- "aggfinalfn, aggtranstype::pg_catalog.regtype, "
- "'-' AS aggcombinefn, '-' AS aggserialfn, "
- "'-' AS aggdeserialfn, '-' AS aggmtransfn, "
- "'-' AS aggminvtransfn, '-' AS aggmfinalfn, "
- "0 AS aggmtranstype, false AS aggfinalextra, "
- "false AS aggmfinalextra, "
- "'0' AS aggfinalmodify, '0' AS aggmfinalmodify, "
- "aggsortop, "
- "'n' AS aggkind, "
- "0 AS aggtransspace, agginitval, "
- "0 AS aggmtransspace, NULL AS aggminitval "
- "FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
- "WHERE a.aggfnoid = p.oid "
- "AND p.oid = '%u'::pg_catalog.oid",
- agginfo->aggfn.dobj.catId.oid);
- }
+ appendPQExpBuffer(query,
+ "aggfinalmodify,\n"
+ "aggmfinalmodify,\n");
else
- {
- appendPQExpBuffer(query, "SELECT aggtransfn, "
- "aggfinalfn, aggtranstype::pg_catalog.regtype, "
- "'-' AS aggcombinefn, '-' AS aggserialfn, "
- "'-' AS aggdeserialfn, '-' AS aggmtransfn, "
- "'-' AS aggminvtransfn, '-' AS aggmfinalfn, "
- "0 AS aggmtranstype, false AS aggfinalextra, "
- "false AS aggmfinalextra, "
- "'0' AS aggfinalmodify, '0' AS aggmfinalmodify, "
- "0 AS aggsortop, "
- "'n' AS aggkind, "
- "0 AS aggtransspace, agginitval, "
- "0 AS aggmtransspace, NULL AS aggminitval "
- "FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
- "WHERE a.aggfnoid = p.oid "
- "AND p.oid = '%u'::pg_catalog.oid",
- agginfo->aggfn.dobj.catId.oid);
- }
+ appendPQExpBuffer(query,
+ "'0' AS aggfinalmodify,\n"
+ "'0' AS aggmfinalmodify,\n");
+
+ if (fout->remoteVersion >= 90600)
+ appendPQExpBuffer(query,
+ "aggcombinefn,\n"
+ "aggserialfn,\n"
+ "aggdeserialfn,\n"
+ "proparallel,\n");
+ else
+ appendPQExpBuffer(query,
+ "'-' AS aggcombinefn,\n"
+ "'-' AS aggserialfn,\n"
+ "'-' AS aggdeserialfn,\n"
+ "'u' AS proparallel,\n");
+
+ if (fout->remoteVersion >= 90400)
+ appendPQExpBuffer(query,
+ "aggkind,\n"
+ "aggmtransfn,\n"
+ "aggminvtransfn,\n"
+ "aggmfinalfn,\n"
+ "aggmtranstype::pg_catalog.regtype,\n"
+ "aggfinalextra,\n"
+ "aggmfinalextra,\n"
+ "aggtransspace,\n"
+ "aggmtransspace,\n"
+ "aggminitval,\n");
+ else
+ appendPQExpBuffer(query,
+ "'n' AS aggkind,\n"
+ "'-' AS aggmtransfn,\n"
+ "'-' AS aggminvtransfn,\n"
+ "'-' AS aggmfinalfn,\n"
+ "0 AS aggmtranstype,\n"
+ "false AS aggfinalextra,\n"
+ "false AS aggmfinalextra,\n"
+ "0 AS aggtransspace,\n"
+ "0 AS aggmtransspace,\n"
+ "NULL AS aggminitval,\n");
+
+ if (fout->remoteVersion >= 80100)
+ appendPQExpBuffer(query,
+ "aggsortop,\n");
+ else
+ appendPQExpBuffer(query,
+ "0 AS aggsortop,\n");
+
+ appendPQExpBuffer(query,
+ "pg_catalog.pg_get_function_arguments(p.oid) AS funcargs,\n"
+ "pg_catalog.pg_get_function_identity_arguments(p.oid) AS funciargs\n"
+ "FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
+ "WHERE a.aggfnoid = p.oid "
+ "AND p.oid = '%u'::pg_catalog.oid",
+ agginfo->aggfn.dobj.catId.oid);
res = ExecuteSqlQueryForSingleRow(fout, query->data);
- i_aggtransfn = PQfnumber(res, "aggtransfn");
- i_aggfinalfn = PQfnumber(res, "aggfinalfn");
- i_aggcombinefn = PQfnumber(res, "aggcombinefn");
- i_aggserialfn = PQfnumber(res, "aggserialfn");
- i_aggdeserialfn = PQfnumber(res, "aggdeserialfn");
- i_aggmtransfn = PQfnumber(res, "aggmtransfn");
- i_aggminvtransfn = PQfnumber(res, "aggminvtransfn");
- i_aggmfinalfn = PQfnumber(res, "aggmfinalfn");
- i_aggfinalextra = PQfnumber(res, "aggfinalextra");
- i_aggmfinalextra = PQfnumber(res, "aggmfinalextra");
- i_aggfinalmodify = PQfnumber(res, "aggfinalmodify");
- i_aggmfinalmodify = PQfnumber(res, "aggmfinalmodify");
- i_aggsortop = PQfnumber(res, "aggsortop");
- i_aggkind = PQfnumber(res, "aggkind");
- i_aggtranstype = PQfnumber(res, "aggtranstype");
- i_aggtransspace = PQfnumber(res, "aggtransspace");
- i_aggmtranstype = PQfnumber(res, "aggmtranstype");
- i_aggmtransspace = PQfnumber(res, "aggmtransspace");
i_agginitval = PQfnumber(res, "agginitval");
i_aggminitval = PQfnumber(res, "aggminitval");
- i_proparallel = PQfnumber(res, "proparallel");
-
- aggtransfn = PQgetvalue(res, 0, i_aggtransfn);
- aggfinalfn = PQgetvalue(res, 0, i_aggfinalfn);
- aggcombinefn = PQgetvalue(res, 0, i_aggcombinefn);
- aggserialfn = PQgetvalue(res, 0, i_aggserialfn);
- aggdeserialfn = PQgetvalue(res, 0, i_aggdeserialfn);
- aggmtransfn = PQgetvalue(res, 0, i_aggmtransfn);
- aggminvtransfn = PQgetvalue(res, 0, i_aggminvtransfn);
- aggmfinalfn = PQgetvalue(res, 0, i_aggmfinalfn);
- aggfinalextra = (PQgetvalue(res, 0, i_aggfinalextra)[0] == 't');
- aggmfinalextra = (PQgetvalue(res, 0, i_aggmfinalextra)[0] == 't');
- aggfinalmodify = PQgetvalue(res, 0, i_aggfinalmodify)[0];
- aggmfinalmodify = PQgetvalue(res, 0, i_aggmfinalmodify)[0];
- aggsortop = PQgetvalue(res, 0, i_aggsortop);
- aggkind = PQgetvalue(res, 0, i_aggkind)[0];
- aggtranstype = PQgetvalue(res, 0, i_aggtranstype);
- aggtransspace = PQgetvalue(res, 0, i_aggtransspace);
- aggmtranstype = PQgetvalue(res, 0, i_aggmtranstype);
- aggmtransspace = PQgetvalue(res, 0, i_aggmtransspace);
+
+ aggtransfn = PQgetvalue(res, 0, PQfnumber(res, "aggtransfn"));
+ aggfinalfn = PQgetvalue(res, 0, PQfnumber(res, "aggfinalfn"));
+ aggcombinefn = PQgetvalue(res, 0, PQfnumber(res, "aggcombinefn"));
+ aggserialfn = PQgetvalue(res, 0, PQfnumber(res, "aggserialfn"));
+ aggdeserialfn = PQgetvalue(res, 0, PQfnumber(res, "aggdeserialfn"));
+ aggmtransfn = PQgetvalue(res, 0, PQfnumber(res, "aggmtransfn"));
+ aggminvtransfn = PQgetvalue(res, 0, PQfnumber(res, "aggminvtransfn"));
+ aggmfinalfn = PQgetvalue(res, 0, PQfnumber(res, "aggmfinalfn"));
+ aggfinalextra = (PQgetvalue(res, 0, PQfnumber(res, "aggfinalextra"))[0] == 't');
+ aggmfinalextra = (PQgetvalue(res, 0, PQfnumber(res, "aggmfinalextra"))[0] == 't');
+ aggfinalmodify = PQgetvalue(res, 0, PQfnumber(res, "aggfinalmodify"))[0];
+ aggmfinalmodify = PQgetvalue(res, 0, PQfnumber(res, "aggmfinalmodify"))[0];
+ aggsortop = PQgetvalue(res, 0, PQfnumber(res, "aggsortop"));
+ aggkind = PQgetvalue(res, 0, PQfnumber(res, "aggkind"))[0];
+ aggtranstype = PQgetvalue(res, 0, PQfnumber(res, "aggtranstype"));
+ aggtransspace = PQgetvalue(res, 0, PQfnumber(res, "aggtransspace"));
+ aggmtranstype = PQgetvalue(res, 0, PQfnumber(res, "aggmtranstype"));
+ aggmtransspace = PQgetvalue(res, 0, PQfnumber(res, "aggmtransspace"));
agginitval = PQgetvalue(res, 0, i_agginitval);
aggminitval = PQgetvalue(res, 0, i_aggminitval);
+ proparallel = PQgetvalue(res, 0, PQfnumber(res, "proparallel"));
if (fout->remoteVersion >= 80400)
{
@@ -14176,11 +14000,6 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
aggsig_tag = format_aggregate_signature(agginfo, fout, false);
- if (i_proparallel != -1)
- proparallel = PQgetvalue(res, 0, PQfnumber(res, "proparallel"));
- else
- proparallel = NULL;
-
/* identify default modify flag for aggkind (must match DefineAggregate) */
defaultfinalmodify = (aggkind == AGGKIND_NORMAL) ? AGGMODIFY_READ_ONLY : AGGMODIFY_READ_WRITE;
/* replace omitted flags for old versions */
@@ -14299,7 +14118,7 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
if (aggkind == AGGKIND_HYPOTHETICAL)
appendPQExpBufferStr(details, ",\n HYPOTHETICAL");
- if (proparallel != NULL && proparallel[0] != PROPARALLEL_UNSAFE)
+ if (proparallel[0] != PROPARALLEL_UNSAFE)
{
if (proparallel[0] == PROPARALLEL_SAFE)
appendPQExpBufferStr(details, ",\n PARALLEL = safe");
--
2.27.0
Re: some more pg_dump refactoring
On 23 Jun 2020, at 14:57, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:
Here is a patch to reorganize dumpFunc() and dumpAgg() in pg_dump, similar to daa9fe8a5264a3f192efa5ddee8fb011ad9da365. Instead of repeating the almost same large query in each version branch, use one
query and add a few columns to the SELECT list depending on the
version. This saves a lot of duplication.I have tested this with various old versions of PostgreSQL I had available, but a bit more random testing with old versions would be welcome.
+1 from reading the patch and lightly testing it with the older versions I had
handy, and another big +1 on the refactoring to remove the duplication.
cheers ./daniel
Re: some more pg_dump refactoring
Hallo Peter,
My 0.02 €:
Patch applies cleanly, compiles, make check and pg_dump tap tests ok. The
refactoring is a definite improvements.
You changed the query strings to use "\n" instead of " ". I would not have
changed that, because it departs from the style around, and I do not think
it improves readability at the C code level.
I tried to check manually and randomly that the same query is built for
the same version, although my check may have been partial, especially on
the aggregate query which does not comment about what is changed between
versions, and my eyes are not very good at diffing.
I've notice that some attributes are given systematic replacements (eg
proparallel), removing the need to test for presence afterwards. This
looks fine to me.
However, on version < 8.4, ISTM that funcargs and funciargs are always
added: is this voluntary?.
Would it make sense to accumulate in the other direction, older to newer,
so that new attributes are added at the end of the select?
Should array_to_string be pg_catalog.array_to_string? All other calls seem
to have an explicit schema.
I'm fine with inlining most PQfnumber calls.
I do not have old versions at hand for testing.
Here is a patch to reorganize dumpFunc() and dumpAgg() in pg_dump, similar to
daa9fe8a5264a3f192efa5ddee8fb011ad9da365. Instead of repeating the almost
same large query in each version branch, use one
query and add a few columns to the SELECT list depending on the
version. This saves a lot of duplication.I have tested this with various old versions of PostgreSQL I had available,
but a bit more random testing with old versions would be welcome.
--
Fabien.
Re: some more pg_dump refactoring
On 2020-06-25 08:58, Fabien COELHO wrote:
You changed the query strings to use "\n" instead of " ". I would not have
changed that, because it departs from the style around, and I do not think
it improves readability at the C code level.
This was the style that was introduced by
daa9fe8a5264a3f192efa5ddee8fb011ad9da365.
However, on version < 8.4, ISTM that funcargs and funciargs are always
added: is this voluntary?.
That was a mistake.
Would it make sense to accumulate in the other direction, older to newer,
so that new attributes are added at the end of the select?
I think that could make sense, but the current style was introduced by
daa9fe8a5264a3f192efa5ddee8fb011ad9da365. Should we revise that?
Should array_to_string be pg_catalog.array_to_string? All other calls seem
to have an explicit schema.
It's not handled fully consistently in pg_dump. But my understanding is
that this is no longer necessary because pg_dump explicitly sets the
search path before running.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
v2-0001-pg_dump-Reorganize-dumpFunc-and-dumpAgg.patchtext/plain; charset=UTF-8; name=v2-0001-pg_dump-Reorganize-dumpFunc-and-dumpAgg.patch; x-mac-creator=0; x-mac-type=0Download
From 617adf2512484610e40f2178a46d8c0af9b746b4 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 29 Jun 2020 15:03:07 +0200
Subject: [PATCH v2] pg_dump: Reorganize dumpFunc() and dumpAgg()
Similar to daa9fe8a5264a3f192efa5ddee8fb011ad9da365, instead of
repeating the almost same large query in each version branch, use one
query and add a few columns to the SELECT list depending on the
version. This saves a lot of duplication.
---
src/bin/pg_dump/pg_dump.c | 492 ++++++++++++--------------------------
1 file changed, 157 insertions(+), 335 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index a41a3db876..f3ad1a5d9e 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -11847,171 +11847,88 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
asPart = createPQExpBuffer();
/* Fetch function-specific details */
+ appendPQExpBuffer(query,
+ "SELECT\n"
+ "proretset,\n"
+ "prosrc,\n"
+ "probin,\n"
+ "provolatile,\n"
+ "proisstrict,\n"
+ "prosecdef,\n"
+ "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname,\n");
+
if (fout->remoteVersion >= 120000)
- {
- /*
- * prosupport was added in 12
- */
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "pg_catalog.pg_get_function_arguments(oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(oid) AS funciargs, "
- "pg_catalog.pg_get_function_result(oid) AS funcresult, "
- "array_to_string(protrftypes, ' ') AS protrftypes, "
- "prokind, provolatile, proisstrict, prosecdef, "
- "proleakproof, proconfig, procost, prorows, "
- "prosupport, proparallel, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 110000)
- {
- /*
- * prokind was added in 11
- */
+ "prosupport,\n");
+ else
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "pg_catalog.pg_get_function_arguments(oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(oid) AS funciargs, "
- "pg_catalog.pg_get_function_result(oid) AS funcresult, "
- "array_to_string(protrftypes, ' ') AS protrftypes, "
- "prokind, provolatile, proisstrict, prosecdef, "
- "proleakproof, proconfig, procost, prorows, "
- "'-' AS prosupport, proparallel, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 90600)
- {
- /*
- * proparallel was added in 9.6
- */
+ "'-' AS prosupport,\n");
+
+ if (fout->remoteVersion >= 110000)
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "pg_catalog.pg_get_function_arguments(oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(oid) AS funciargs, "
- "pg_catalog.pg_get_function_result(oid) AS funcresult, "
- "array_to_string(protrftypes, ' ') AS protrftypes, "
- "CASE WHEN proiswindow THEN 'w' ELSE 'f' END AS prokind, "
- "provolatile, proisstrict, prosecdef, "
- "proleakproof, proconfig, procost, prorows, "
- "'-' AS prosupport, proparallel, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 90500)
- {
- /*
- * protrftypes was added in 9.5
- */
+ "prokind,\n");
+ else if (fout->remoteVersion >= 80400)
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "pg_catalog.pg_get_function_arguments(oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(oid) AS funciargs, "
- "pg_catalog.pg_get_function_result(oid) AS funcresult, "
- "array_to_string(protrftypes, ' ') AS protrftypes, "
- "CASE WHEN proiswindow THEN 'w' ELSE 'f' END AS prokind, "
- "provolatile, proisstrict, prosecdef, "
- "proleakproof, proconfig, procost, prorows, "
- "'-' AS prosupport, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 90200)
- {
- /*
- * proleakproof was added in 9.2
- */
+ "CASE WHEN proiswindow THEN 'w' ELSE 'f' END AS prokind,\n");
+ else
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "pg_catalog.pg_get_function_arguments(oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(oid) AS funciargs, "
- "pg_catalog.pg_get_function_result(oid) AS funcresult, "
- "CASE WHEN proiswindow THEN 'w' ELSE 'f' END AS prokind, "
- "provolatile, proisstrict, prosecdef, "
- "proleakproof, proconfig, procost, prorows, "
- "'-' AS prosupport, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 80400)
+ "'f' AS prokind,\n");
+
+ if (fout->remoteVersion >= 90600)
+ appendPQExpBuffer(query,
+ "proparallel,\n");
+ else
+ appendPQExpBuffer(query,
+ "'u' AS proparallel,\n");
+
+ if (fout->remoteVersion >= 90500)
+ appendPQExpBuffer(query,
+ "array_to_string(protrftypes, ' ') AS protrftypes,\n");
+
+ if (fout->remoteVersion >= 90200)
+ appendPQExpBuffer(query,
+ "proleakproof,\n");
+ else
+ appendPQExpBuffer(query,
+ "false AS proleakproof,\n");
+
+ if (fout->remoteVersion >= 80400)
{
/*
* In 8.4 and up we rely on pg_get_function_arguments and
* pg_get_function_result instead of examining proallargtypes etc.
*/
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "pg_catalog.pg_get_function_arguments(oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(oid) AS funciargs, "
- "pg_catalog.pg_get_function_result(oid) AS funcresult, "
- "CASE WHEN proiswindow THEN 'w' ELSE 'f' END AS prokind, "
- "provolatile, proisstrict, prosecdef, "
- "false AS proleakproof, "
- " proconfig, procost, prorows, "
- "'-' AS prosupport, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 80300)
- {
- appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "proallargtypes, proargmodes, proargnames, "
- "'f' AS prokind, "
- "provolatile, proisstrict, prosecdef, "
- "false AS proleakproof, "
- "proconfig, procost, prorows, "
- "'-' AS prosupport, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
+ "pg_catalog.pg_get_function_arguments(oid) AS funcargs,\n"
+ "pg_catalog.pg_get_function_identity_arguments(oid) AS funciargs,\n"
+ "pg_catalog.pg_get_function_result(oid) AS funcresult,\n");
}
else if (fout->remoteVersion >= 80100)
- {
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "proallargtypes, proargmodes, proargnames, "
- "'f' AS prokind, "
- "provolatile, proisstrict, prosecdef, "
- "false AS proleakproof, "
- "null AS proconfig, 0 AS procost, 0 AS prorows, "
- "'-' AS prosupport, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
+ "proallargtypes,\n"
+ "proargmodes,\n"
+ "proargnames,\n");
else
- {
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "null AS proallargtypes, "
- "null AS proargmodes, "
- "proargnames, "
- "'f' AS prokind, "
- "provolatile, proisstrict, prosecdef, "
- "false AS proleakproof, "
- "null AS proconfig, 0 AS procost, 0 AS prorows, "
- "'-' AS prosupport, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
+ "null AS proallargtypes,\n"
+ "null AS proargmodes,\n"
+ "proargnames,\n");
+
+ if (fout->remoteVersion >= 80300)
+ appendPQExpBuffer(query,
+ "proconfig,\n"
+ "procost,\n"
+ "prorows\n");
+ else
+ appendPQExpBuffer(query,
+ "null AS proconfig,\n"
+ "0 AS procost,\n"
+ "0 AS prorows\n");
+
+ appendPQExpBuffer(query,
+ "FROM pg_catalog.pg_proc "
+ "WHERE oid = '%u'::pg_catalog.oid",
+ finfo->dobj.catId.oid);
res = ExecuteSqlQueryForSingleRow(fout, query->data);
@@ -12045,12 +11962,7 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
procost = PQgetvalue(res, 0, PQfnumber(res, "procost"));
prorows = PQgetvalue(res, 0, PQfnumber(res, "prorows"));
prosupport = PQgetvalue(res, 0, PQfnumber(res, "prosupport"));
-
- if (PQfnumber(res, "proparallel") != -1)
- proparallel = PQgetvalue(res, 0, PQfnumber(res, "proparallel"));
- else
- proparallel = NULL;
-
+ proparallel = PQgetvalue(res, 0, PQfnumber(res, "proparallel"));
lanname = PQgetvalue(res, 0, PQfnumber(res, "lanname"));
/*
@@ -12264,7 +12176,7 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
appendPQExpBuffer(q, " SUPPORT %s", prosupport);
}
- if (proparallel != NULL && proparallel[0] != PROPARALLEL_UNSAFE)
+ if (proparallel[0] != PROPARALLEL_UNSAFE)
{
if (proparallel[0] == PROPARALLEL_SAFE)
appendPQExpBufferStr(q, " PARALLEL SAFE");
@@ -13939,27 +13851,8 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
char *aggfullsig = NULL; /* full signature */
char *aggsig_tag;
PGresult *res;
- int i_aggtransfn;
- int i_aggfinalfn;
- int i_aggcombinefn;
- int i_aggserialfn;
- int i_aggdeserialfn;
- int i_aggmtransfn;
- int i_aggminvtransfn;
- int i_aggmfinalfn;
- int i_aggfinalextra;
- int i_aggmfinalextra;
- int i_aggfinalmodify;
- int i_aggmfinalmodify;
- int i_aggsortop;
- int i_aggkind;
- int i_aggtranstype;
- int i_aggtransspace;
- int i_aggmtranstype;
- int i_aggmtransspace;
int i_agginitval;
int i_aggminitval;
- int i_proparallel;
const char *aggtransfn;
const char *aggfinalfn;
const char *aggcombinefn;
@@ -13994,170 +13887,104 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
details = createPQExpBuffer();
/* Get aggregate-specific details */
+ appendPQExpBuffer(query,
+ "SELECT\n"
+ "aggtransfn,\n"
+ "aggfinalfn,\n"
+ "aggtranstype::pg_catalog.regtype,\n"
+ "agginitval,\n");
+
if (fout->remoteVersion >= 110000)
- {
- appendPQExpBuffer(query, "SELECT aggtransfn, "
- "aggfinalfn, aggtranstype::pg_catalog.regtype, "
- "aggcombinefn, aggserialfn, aggdeserialfn, aggmtransfn, "
- "aggminvtransfn, aggmfinalfn, aggmtranstype::pg_catalog.regtype, "
- "aggfinalextra, aggmfinalextra, "
- "aggfinalmodify, aggmfinalmodify, "
- "aggsortop, "
- "aggkind, "
- "aggtransspace, agginitval, "
- "aggmtransspace, aggminitval, "
- "pg_catalog.pg_get_function_arguments(p.oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(p.oid) AS funciargs, "
- "p.proparallel "
- "FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
- "WHERE a.aggfnoid = p.oid "
- "AND p.oid = '%u'::pg_catalog.oid",
- agginfo->aggfn.dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 90600)
- {
- appendPQExpBuffer(query, "SELECT aggtransfn, "
- "aggfinalfn, aggtranstype::pg_catalog.regtype, "
- "aggcombinefn, aggserialfn, aggdeserialfn, aggmtransfn, "
- "aggminvtransfn, aggmfinalfn, aggmtranstype::pg_catalog.regtype, "
- "aggfinalextra, aggmfinalextra, "
- "'0' AS aggfinalmodify, '0' AS aggmfinalmodify, "
- "aggsortop, "
- "aggkind, "
- "aggtransspace, agginitval, "
- "aggmtransspace, aggminitval, "
- "pg_catalog.pg_get_function_arguments(p.oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(p.oid) AS funciargs, "
- "p.proparallel "
- "FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
- "WHERE a.aggfnoid = p.oid "
- "AND p.oid = '%u'::pg_catalog.oid",
- agginfo->aggfn.dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 90400)
- {
- appendPQExpBuffer(query, "SELECT aggtransfn, "
- "aggfinalfn, aggtranstype::pg_catalog.regtype, "
- "'-' AS aggcombinefn, '-' AS aggserialfn, "
- "'-' AS aggdeserialfn, aggmtransfn, aggminvtransfn, "
- "aggmfinalfn, aggmtranstype::pg_catalog.regtype, "
- "aggfinalextra, aggmfinalextra, "
- "'0' AS aggfinalmodify, '0' AS aggmfinalmodify, "
- "aggsortop, "
- "aggkind, "
- "aggtransspace, agginitval, "
- "aggmtransspace, aggminitval, "
- "pg_catalog.pg_get_function_arguments(p.oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(p.oid) AS funciargs "
- "FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
- "WHERE a.aggfnoid = p.oid "
- "AND p.oid = '%u'::pg_catalog.oid",
- agginfo->aggfn.dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 80400)
- {
- appendPQExpBuffer(query, "SELECT aggtransfn, "
- "aggfinalfn, aggtranstype::pg_catalog.regtype, "
- "'-' AS aggcombinefn, '-' AS aggserialfn, "
- "'-' AS aggdeserialfn, '-' AS aggmtransfn, "
- "'-' AS aggminvtransfn, '-' AS aggmfinalfn, "
- "0 AS aggmtranstype, false AS aggfinalextra, "
- "false AS aggmfinalextra, "
- "'0' AS aggfinalmodify, '0' AS aggmfinalmodify, "
- "aggsortop, "
- "'n' AS aggkind, "
- "0 AS aggtransspace, agginitval, "
- "0 AS aggmtransspace, NULL AS aggminitval, "
- "pg_catalog.pg_get_function_arguments(p.oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(p.oid) AS funciargs "
- "FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
- "WHERE a.aggfnoid = p.oid "
- "AND p.oid = '%u'::pg_catalog.oid",
- agginfo->aggfn.dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 80100)
- {
- appendPQExpBuffer(query, "SELECT aggtransfn, "
- "aggfinalfn, aggtranstype::pg_catalog.regtype, "
- "'-' AS aggcombinefn, '-' AS aggserialfn, "
- "'-' AS aggdeserialfn, '-' AS aggmtransfn, "
- "'-' AS aggminvtransfn, '-' AS aggmfinalfn, "
- "0 AS aggmtranstype, false AS aggfinalextra, "
- "false AS aggmfinalextra, "
- "'0' AS aggfinalmodify, '0' AS aggmfinalmodify, "
- "aggsortop, "
- "'n' AS aggkind, "
- "0 AS aggtransspace, agginitval, "
- "0 AS aggmtransspace, NULL AS aggminitval "
- "FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
- "WHERE a.aggfnoid = p.oid "
- "AND p.oid = '%u'::pg_catalog.oid",
- agginfo->aggfn.dobj.catId.oid);
- }
+ appendPQExpBuffer(query,
+ "aggfinalmodify,\n"
+ "aggmfinalmodify,\n");
else
- {
- appendPQExpBuffer(query, "SELECT aggtransfn, "
- "aggfinalfn, aggtranstype::pg_catalog.regtype, "
- "'-' AS aggcombinefn, '-' AS aggserialfn, "
- "'-' AS aggdeserialfn, '-' AS aggmtransfn, "
- "'-' AS aggminvtransfn, '-' AS aggmfinalfn, "
- "0 AS aggmtranstype, false AS aggfinalextra, "
- "false AS aggmfinalextra, "
- "'0' AS aggfinalmodify, '0' AS aggmfinalmodify, "
- "0 AS aggsortop, "
- "'n' AS aggkind, "
- "0 AS aggtransspace, agginitval, "
- "0 AS aggmtransspace, NULL AS aggminitval "
- "FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
- "WHERE a.aggfnoid = p.oid "
- "AND p.oid = '%u'::pg_catalog.oid",
- agginfo->aggfn.dobj.catId.oid);
- }
+ appendPQExpBuffer(query,
+ "'0' AS aggfinalmodify,\n"
+ "'0' AS aggmfinalmodify,\n");
+
+ if (fout->remoteVersion >= 90600)
+ appendPQExpBuffer(query,
+ "aggcombinefn,\n"
+ "aggserialfn,\n"
+ "aggdeserialfn,\n"
+ "proparallel,\n");
+ else
+ appendPQExpBuffer(query,
+ "'-' AS aggcombinefn,\n"
+ "'-' AS aggserialfn,\n"
+ "'-' AS aggdeserialfn,\n"
+ "'u' AS proparallel,\n");
+
+ if (fout->remoteVersion >= 90400)
+ appendPQExpBuffer(query,
+ "aggkind,\n"
+ "aggmtransfn,\n"
+ "aggminvtransfn,\n"
+ "aggmfinalfn,\n"
+ "aggmtranstype::pg_catalog.regtype,\n"
+ "aggfinalextra,\n"
+ "aggmfinalextra,\n"
+ "aggtransspace,\n"
+ "aggmtransspace,\n"
+ "aggminitval,\n");
+ else
+ appendPQExpBuffer(query,
+ "'n' AS aggkind,\n"
+ "'-' AS aggmtransfn,\n"
+ "'-' AS aggminvtransfn,\n"
+ "'-' AS aggmfinalfn,\n"
+ "0 AS aggmtranstype,\n"
+ "false AS aggfinalextra,\n"
+ "false AS aggmfinalextra,\n"
+ "0 AS aggtransspace,\n"
+ "0 AS aggmtransspace,\n"
+ "NULL AS aggminitval,\n");
+
+ if (fout->remoteVersion >= 80400)
+ appendPQExpBuffer(query,
+ "pg_catalog.pg_get_function_arguments(p.oid) AS funcargs,\n"
+ "pg_catalog.pg_get_function_identity_arguments(p.oid) AS funciargs,\n");
+
+ if (fout->remoteVersion >= 80100)
+ appendPQExpBuffer(query,
+ "aggsortop\n");
+ else
+ appendPQExpBuffer(query,
+ "0 AS aggsortop\n");
+
+ appendPQExpBuffer(query,
+ "FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
+ "WHERE a.aggfnoid = p.oid "
+ "AND p.oid = '%u'::pg_catalog.oid",
+ agginfo->aggfn.dobj.catId.oid);
res = ExecuteSqlQueryForSingleRow(fout, query->data);
- i_aggtransfn = PQfnumber(res, "aggtransfn");
- i_aggfinalfn = PQfnumber(res, "aggfinalfn");
- i_aggcombinefn = PQfnumber(res, "aggcombinefn");
- i_aggserialfn = PQfnumber(res, "aggserialfn");
- i_aggdeserialfn = PQfnumber(res, "aggdeserialfn");
- i_aggmtransfn = PQfnumber(res, "aggmtransfn");
- i_aggminvtransfn = PQfnumber(res, "aggminvtransfn");
- i_aggmfinalfn = PQfnumber(res, "aggmfinalfn");
- i_aggfinalextra = PQfnumber(res, "aggfinalextra");
- i_aggmfinalextra = PQfnumber(res, "aggmfinalextra");
- i_aggfinalmodify = PQfnumber(res, "aggfinalmodify");
- i_aggmfinalmodify = PQfnumber(res, "aggmfinalmodify");
- i_aggsortop = PQfnumber(res, "aggsortop");
- i_aggkind = PQfnumber(res, "aggkind");
- i_aggtranstype = PQfnumber(res, "aggtranstype");
- i_aggtransspace = PQfnumber(res, "aggtransspace");
- i_aggmtranstype = PQfnumber(res, "aggmtranstype");
- i_aggmtransspace = PQfnumber(res, "aggmtransspace");
i_agginitval = PQfnumber(res, "agginitval");
i_aggminitval = PQfnumber(res, "aggminitval");
- i_proparallel = PQfnumber(res, "proparallel");
-
- aggtransfn = PQgetvalue(res, 0, i_aggtransfn);
- aggfinalfn = PQgetvalue(res, 0, i_aggfinalfn);
- aggcombinefn = PQgetvalue(res, 0, i_aggcombinefn);
- aggserialfn = PQgetvalue(res, 0, i_aggserialfn);
- aggdeserialfn = PQgetvalue(res, 0, i_aggdeserialfn);
- aggmtransfn = PQgetvalue(res, 0, i_aggmtransfn);
- aggminvtransfn = PQgetvalue(res, 0, i_aggminvtransfn);
- aggmfinalfn = PQgetvalue(res, 0, i_aggmfinalfn);
- aggfinalextra = (PQgetvalue(res, 0, i_aggfinalextra)[0] == 't');
- aggmfinalextra = (PQgetvalue(res, 0, i_aggmfinalextra)[0] == 't');
- aggfinalmodify = PQgetvalue(res, 0, i_aggfinalmodify)[0];
- aggmfinalmodify = PQgetvalue(res, 0, i_aggmfinalmodify)[0];
- aggsortop = PQgetvalue(res, 0, i_aggsortop);
- aggkind = PQgetvalue(res, 0, i_aggkind)[0];
- aggtranstype = PQgetvalue(res, 0, i_aggtranstype);
- aggtransspace = PQgetvalue(res, 0, i_aggtransspace);
- aggmtranstype = PQgetvalue(res, 0, i_aggmtranstype);
- aggmtransspace = PQgetvalue(res, 0, i_aggmtransspace);
+
+ aggtransfn = PQgetvalue(res, 0, PQfnumber(res, "aggtransfn"));
+ aggfinalfn = PQgetvalue(res, 0, PQfnumber(res, "aggfinalfn"));
+ aggcombinefn = PQgetvalue(res, 0, PQfnumber(res, "aggcombinefn"));
+ aggserialfn = PQgetvalue(res, 0, PQfnumber(res, "aggserialfn"));
+ aggdeserialfn = PQgetvalue(res, 0, PQfnumber(res, "aggdeserialfn"));
+ aggmtransfn = PQgetvalue(res, 0, PQfnumber(res, "aggmtransfn"));
+ aggminvtransfn = PQgetvalue(res, 0, PQfnumber(res, "aggminvtransfn"));
+ aggmfinalfn = PQgetvalue(res, 0, PQfnumber(res, "aggmfinalfn"));
+ aggfinalextra = (PQgetvalue(res, 0, PQfnumber(res, "aggfinalextra"))[0] == 't');
+ aggmfinalextra = (PQgetvalue(res, 0, PQfnumber(res, "aggmfinalextra"))[0] == 't');
+ aggfinalmodify = PQgetvalue(res, 0, PQfnumber(res, "aggfinalmodify"))[0];
+ aggmfinalmodify = PQgetvalue(res, 0, PQfnumber(res, "aggmfinalmodify"))[0];
+ aggsortop = PQgetvalue(res, 0, PQfnumber(res, "aggsortop"));
+ aggkind = PQgetvalue(res, 0, PQfnumber(res, "aggkind"))[0];
+ aggtranstype = PQgetvalue(res, 0, PQfnumber(res, "aggtranstype"));
+ aggtransspace = PQgetvalue(res, 0, PQfnumber(res, "aggtransspace"));
+ aggmtranstype = PQgetvalue(res, 0, PQfnumber(res, "aggmtranstype"));
+ aggmtransspace = PQgetvalue(res, 0, PQfnumber(res, "aggmtransspace"));
agginitval = PQgetvalue(res, 0, i_agginitval);
aggminitval = PQgetvalue(res, 0, i_aggminitval);
+ proparallel = PQgetvalue(res, 0, PQfnumber(res, "proparallel"));
if (fout->remoteVersion >= 80400)
{
@@ -14176,11 +14003,6 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
aggsig_tag = format_aggregate_signature(agginfo, fout, false);
- if (i_proparallel != -1)
- proparallel = PQgetvalue(res, 0, PQfnumber(res, "proparallel"));
- else
- proparallel = NULL;
-
/* identify default modify flag for aggkind (must match DefineAggregate) */
defaultfinalmodify = (aggkind == AGGKIND_NORMAL) ? AGGMODIFY_READ_ONLY : AGGMODIFY_READ_WRITE;
/* replace omitted flags for old versions */
@@ -14299,7 +14121,7 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
if (aggkind == AGGKIND_HYPOTHETICAL)
appendPQExpBufferStr(details, ",\n HYPOTHETICAL");
- if (proparallel != NULL && proparallel[0] != PROPARALLEL_UNSAFE)
+ if (proparallel[0] != PROPARALLEL_UNSAFE)
{
if (proparallel[0] == PROPARALLEL_SAFE)
appendPQExpBufferStr(details, ",\n PARALLEL = safe");
--
2.27.0
Re: some more pg_dump refactoring
Hello,
You changed the query strings to use "\n" instead of " ". I would not have
changed that, because it departs from the style around, and I do not think
it improves readability at the C code level.This was the style that was introduced by
daa9fe8a5264a3f192efa5ddee8fb011ad9da365.
Yep. This does not imply that it is better, or worst. Currently it is not
consistent within the file, and there are only few instances, so maybe it
could be discussed anyway.
After giving it some thought, I'd say that at least I'd like the query to
be easy to read when dumped. This is not incompatible with some embedded
eol, on the contrary, but ISTM that it could keep some indentation as
well, which would be kind-of a middle ground. For readability, I'd also
consider turning keywords to upper case. Maybe it could look like:
"SELECT\n"
" somefield,\n"
" someotherfiled,\n"
...
"FROM some_table\n"
"JOIN ... ON ...\n" ...
All this is highly debatable, so ignore if you feel like it.
Would it make sense to accumulate in the other direction, older to newer,
so that new attributes are added at the end of the select?I think that could make sense, but the current style was introduced by
daa9fe8a5264a3f192efa5ddee8fb011ad9da365. Should we revise that?
It seems to me more logical to do it while you're at it, but you are the
one writing the patches:-)
Should array_to_string be pg_catalog.array_to_string? All other calls seem
to have an explicit schema.It's not handled fully consistently in pg_dump. But my understanding is that
this is no longer necessary because pg_dump explicitly sets the search path
before running.
Dunno. It does not look consistent with a mix because the wary reviewer
think that there may be a potential bug:-) ISTM that explicit is better
than implicit in this context: not relying on search path would allow to
test the query independently, and anyway what you see is what you get.
Otherwise: v2 patch applies cleanly, compiles, global make check ok,
pg_dump tap ok.
"progargnames" is added in both branches of an if, which looks awkward.
I'd suggest maybe to add it once unconditionnaly.
I cannot test easily on older versions.
--
Fabien.
Re: some more pg_dump refactoring
On 2020-06-30 16:56, Fabien COELHO wrote:
Would it make sense to accumulate in the other direction, older to newer,
so that new attributes are added at the end of the select?I think that could make sense, but the current style was introduced by
daa9fe8a5264a3f192efa5ddee8fb011ad9da365. Should we revise that?It seems to me more logical to do it while you're at it, but you are the
one writing the patches:-)
What do you think about this patch to reorganize the existing code from
that old commit?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-pg_dump-Further-reorganize-getTableAttrs.patchtext/plain; charset=UTF-8; name=0001-pg_dump-Further-reorganize-getTableAttrs.patch; x-mac-creator=0; x-mac-type=0Download
From eca6fd80b6ca850a0f5e40c40d7cb1be48ab2e2a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Tue, 7 Jul 2020 08:58:16 +0200
Subject: [PATCH] pg_dump: Further reorganize getTableAttrs()
After further discussion after
daa9fe8a5264a3f192efa5ddee8fb011ad9da365, reorder the version-specific
sections from oldest to newest. Also, remove the variable assignments
from PQfnumber() to reduce vertical space.
---
src/bin/pg_dump/pg_dump.c | 149 ++++++++++++++------------------------
1 file changed, 54 insertions(+), 95 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index a41a3db876..aa0725627a 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -8431,35 +8431,14 @@ void
getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
{
DumpOptions *dopt = fout->dopt;
- int i,
- j;
PQExpBuffer q = createPQExpBuffer();
- int i_attnum;
- int i_attname;
- int i_atttypname;
- int i_atttypmod;
- int i_attstattarget;
- int i_attstorage;
- int i_typstorage;
- int i_attnotnull;
- int i_atthasdef;
- int i_attidentity;
- int i_attgenerated;
- int i_attisdropped;
- int i_attlen;
- int i_attalign;
- int i_attislocal;
- int i_attoptions;
- int i_attcollation;
- int i_attfdwoptions;
- int i_attmissingval;
- PGresult *res;
- int ntups;
- bool hasdefaults;
- for (i = 0; i < numTables; i++)
+ for (int i = 0; i < numTables; i++)
{
TableInfo *tbinfo = &tblinfo[i];
+ PGresult *res;
+ int ntups;
+ bool hasdefaults;
/* Don't bother to collect info for sequences */
if (tbinfo->relkind == RELKIND_SEQUENCE)
@@ -8497,27 +8476,27 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
"a.attislocal,\n"
"pg_catalog.format_type(t.oid, a.atttypmod) AS atttypname,\n");
- if (fout->remoteVersion >= 120000)
- appendPQExpBufferStr(q,
- "a.attgenerated,\n");
- else
- appendPQExpBufferStr(q,
- "'' AS attgenerated,\n");
-
- if (fout->remoteVersion >= 110000)
+ if (fout->remoteVersion >= 90000)
appendPQExpBufferStr(q,
- "CASE WHEN a.atthasmissing AND NOT a.attisdropped "
- "THEN a.attmissingval ELSE null END AS attmissingval,\n");
+ "array_to_string(a.attoptions, ', ') AS attoptions,\n");
else
appendPQExpBufferStr(q,
- "NULL AS attmissingval,\n");
+ "'' AS attoptions,\n");
- if (fout->remoteVersion >= 100000)
+ if (fout->remoteVersion >= 90100)
+ {
+ /*
+ * Since we only want to dump COLLATE clauses for attributes whose
+ * collation is different from their type's default, we use a CASE
+ * here to suppress uninteresting attcollations cheaply.
+ */
appendPQExpBufferStr(q,
- "a.attidentity,\n");
+ "CASE WHEN a.attcollation <> t.typcollation "
+ "THEN a.attcollation ELSE 0 END AS attcollation,\n");
+ }
else
appendPQExpBufferStr(q,
- "'' AS attidentity,\n");
+ "0 AS attcollation,\n");
if (fout->remoteVersion >= 90200)
appendPQExpBufferStr(q,
@@ -8531,27 +8510,27 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
appendPQExpBufferStr(q,
"'' AS attfdwoptions,\n");
- if (fout->remoteVersion >= 90100)
- {
- /*
- * Since we only want to dump COLLATE clauses for attributes whose
- * collation is different from their type's default, we use a CASE
- * here to suppress uninteresting attcollations cheaply.
- */
+ if (fout->remoteVersion >= 100000)
appendPQExpBufferStr(q,
- "CASE WHEN a.attcollation <> t.typcollation "
- "THEN a.attcollation ELSE 0 END AS attcollation,\n");
- }
+ "a.attidentity,\n");
else
appendPQExpBufferStr(q,
- "0 AS attcollation,\n");
+ "'' AS attidentity,\n");
- if (fout->remoteVersion >= 90000)
+ if (fout->remoteVersion >= 110000)
appendPQExpBufferStr(q,
- "array_to_string(a.attoptions, ', ') AS attoptions\n");
+ "CASE WHEN a.atthasmissing AND NOT a.attisdropped "
+ "THEN a.attmissingval ELSE null END AS attmissingval,\n");
else
appendPQExpBufferStr(q,
- "'' AS attoptions\n");
+ "NULL AS attmissingval,\n");
+
+ if (fout->remoteVersion >= 120000)
+ appendPQExpBufferStr(q,
+ "a.attgenerated\n");
+ else
+ appendPQExpBufferStr(q,
+ "'' AS attgenerated\n");
/* need left join here to not fail on dropped columns ... */
appendPQExpBuffer(q,
@@ -8566,26 +8545,6 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
ntups = PQntuples(res);
- i_attnum = PQfnumber(res, "attnum");
- i_attname = PQfnumber(res, "attname");
- i_atttypname = PQfnumber(res, "atttypname");
- i_atttypmod = PQfnumber(res, "atttypmod");
- i_attstattarget = PQfnumber(res, "attstattarget");
- i_attstorage = PQfnumber(res, "attstorage");
- i_typstorage = PQfnumber(res, "typstorage");
- i_attnotnull = PQfnumber(res, "attnotnull");
- i_atthasdef = PQfnumber(res, "atthasdef");
- i_attidentity = PQfnumber(res, "attidentity");
- i_attgenerated = PQfnumber(res, "attgenerated");
- i_attisdropped = PQfnumber(res, "attisdropped");
- i_attlen = PQfnumber(res, "attlen");
- i_attalign = PQfnumber(res, "attalign");
- i_attislocal = PQfnumber(res, "attislocal");
- i_attoptions = PQfnumber(res, "attoptions");
- i_attcollation = PQfnumber(res, "attcollation");
- i_attfdwoptions = PQfnumber(res, "attfdwoptions");
- i_attmissingval = PQfnumber(res, "attmissingval");
-
tbinfo->numatts = ntups;
tbinfo->attnames = (char **) pg_malloc(ntups * sizeof(char *));
tbinfo->atttypnames = (char **) pg_malloc(ntups * sizeof(char *));
@@ -8608,31 +8567,31 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
tbinfo->attrdefs = (AttrDefInfo **) pg_malloc(ntups * sizeof(AttrDefInfo *));
hasdefaults = false;
- for (j = 0; j < ntups; j++)
+ for (int j = 0; j < ntups; j++)
{
- if (j + 1 != atoi(PQgetvalue(res, j, i_attnum)))
+ if (j + 1 != atoi(PQgetvalue(res, j, PQfnumber(res, "attnum"))))
fatal("invalid column numbering in table \"%s\"",
tbinfo->dobj.name);
- tbinfo->attnames[j] = pg_strdup(PQgetvalue(res, j, i_attname));
- tbinfo->atttypnames[j] = pg_strdup(PQgetvalue(res, j, i_atttypname));
- tbinfo->atttypmod[j] = atoi(PQgetvalue(res, j, i_atttypmod));
- tbinfo->attstattarget[j] = atoi(PQgetvalue(res, j, i_attstattarget));
- tbinfo->attstorage[j] = *(PQgetvalue(res, j, i_attstorage));
- tbinfo->typstorage[j] = *(PQgetvalue(res, j, i_typstorage));
- tbinfo->attidentity[j] = *(PQgetvalue(res, j, i_attidentity));
- tbinfo->attgenerated[j] = *(PQgetvalue(res, j, i_attgenerated));
+ tbinfo->attnames[j] = pg_strdup(PQgetvalue(res, j, PQfnumber(res, "attname")));
+ tbinfo->atttypnames[j] = pg_strdup(PQgetvalue(res, j, PQfnumber(res, "atttypname")));
+ tbinfo->atttypmod[j] = atoi(PQgetvalue(res, j, PQfnumber(res, "atttypmod")));
+ tbinfo->attstattarget[j] = atoi(PQgetvalue(res, j, PQfnumber(res, "attstattarget")));
+ tbinfo->attstorage[j] = *(PQgetvalue(res, j, PQfnumber(res, "attstorage")));
+ tbinfo->typstorage[j] = *(PQgetvalue(res, j, PQfnumber(res, "typstorage")));
+ tbinfo->attidentity[j] = *(PQgetvalue(res, j, PQfnumber(res, "attidentity")));
+ tbinfo->attgenerated[j] = *(PQgetvalue(res, j, PQfnumber(res, "attgenerated")));
tbinfo->needs_override = tbinfo->needs_override || (tbinfo->attidentity[j] == ATTRIBUTE_IDENTITY_ALWAYS);
- tbinfo->attisdropped[j] = (PQgetvalue(res, j, i_attisdropped)[0] == 't');
- tbinfo->attlen[j] = atoi(PQgetvalue(res, j, i_attlen));
- tbinfo->attalign[j] = *(PQgetvalue(res, j, i_attalign));
- tbinfo->attislocal[j] = (PQgetvalue(res, j, i_attislocal)[0] == 't');
- tbinfo->notnull[j] = (PQgetvalue(res, j, i_attnotnull)[0] == 't');
- tbinfo->attoptions[j] = pg_strdup(PQgetvalue(res, j, i_attoptions));
- tbinfo->attcollation[j] = atooid(PQgetvalue(res, j, i_attcollation));
- tbinfo->attfdwoptions[j] = pg_strdup(PQgetvalue(res, j, i_attfdwoptions));
- tbinfo->attmissingval[j] = pg_strdup(PQgetvalue(res, j, i_attmissingval));
+ tbinfo->attisdropped[j] = (PQgetvalue(res, j, PQfnumber(res, "attisdropped"))[0] == 't');
+ tbinfo->attlen[j] = atoi(PQgetvalue(res, j, PQfnumber(res, "attlen")));
+ tbinfo->attalign[j] = *(PQgetvalue(res, j, PQfnumber(res, "attalign")));
+ tbinfo->attislocal[j] = (PQgetvalue(res, j, PQfnumber(res, "attislocal"))[0] == 't');
+ tbinfo->notnull[j] = (PQgetvalue(res, j, PQfnumber(res, "attnotnull"))[0] == 't');
+ tbinfo->attoptions[j] = pg_strdup(PQgetvalue(res, j, PQfnumber(res, "attoptions")));
+ tbinfo->attcollation[j] = atooid(PQgetvalue(res, j, PQfnumber(res, "attcollation")));
+ tbinfo->attfdwoptions[j] = pg_strdup(PQgetvalue(res, j, PQfnumber(res, "attfdwoptions")));
+ tbinfo->attmissingval[j] = pg_strdup(PQgetvalue(res, j, PQfnumber(res, "attmissingval")));
tbinfo->attrdefs[j] = NULL; /* fix below */
- if (PQgetvalue(res, j, i_atthasdef)[0] == 't')
+ if (PQgetvalue(res, j, PQfnumber(res, "atthasdef"))[0] == 't')
hasdefaults = true;
/* these flags will be set in flagInhAttrs() */
tbinfo->inhNotNull[j] = false;
@@ -8663,7 +8622,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
numDefaults = PQntuples(res);
attrdefs = (AttrDefInfo *) pg_malloc(numDefaults * sizeof(AttrDefInfo));
- for (j = 0; j < numDefaults; j++)
+ for (int j = 0; j < numDefaults; j++)
{
int adnum;
@@ -8795,7 +8754,7 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
constrs = (ConstraintInfo *) pg_malloc(numConstrs * sizeof(ConstraintInfo));
tbinfo->checkexprs = constrs;
- for (j = 0; j < numConstrs; j++)
+ for (int j = 0; j < numConstrs; j++)
{
bool validated = PQgetvalue(res, j, 5)[0] == 't';
--
2.27.0
Re: some more pg_dump refactoring
Hallo Peter,
Would it make sense to accumulate in the other direction, older to newer,
so that new attributes are added at the end of the select?I think that could make sense, but the current style was introduced by
daa9fe8a5264a3f192efa5ddee8fb011ad9da365. Should we revise that?It seems to me more logical to do it while you're at it, but you are the
one writing the patches:-)What do you think about this patch to reorganize the existing code from that
old commit?
I think it is a definite further improvement.
Patch applies cleanly, compiles, global & pg_dump tap test ok, looks ok to
me.
--
Fabien.
Re: some more pg_dump refactoring
On 2020-07-08 06:42, Fabien COELHO wrote:
What do you think about this patch to reorganize the existing code from that
old commit?I think it is a definite further improvement.
Patch applies cleanly, compiles, global & pg_dump tap test ok, looks ok to
me.
Thanks. I have committed that, and attached is my original patch
adjusted to this newer style.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
v3-0001-pg_dump-Reorganize-dumpFunc-and-dumpAgg.patchtext/plain; charset=UTF-8; name=v3-0001-pg_dump-Reorganize-dumpFunc-and-dumpAgg.patch; x-mac-creator=0; x-mac-type=0Download
From cce4e241c12e3719f1f98703079eaf119452f20f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 9 Jul 2020 16:03:30 +0200
Subject: [PATCH v3] pg_dump: Reorganize dumpFunc() and dumpAgg()
Similar to daa9fe8a5264a3f192efa5ddee8fb011ad9da365, instead of
repeating the almost same large query in each version branch, use one
query and add a few columns to the SELECT list depending on the
version. This saves a lot of duplication.
---
src/bin/pg_dump/pg_dump.c | 496 ++++++++++++--------------------------
1 file changed, 159 insertions(+), 337 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 8bca147a33..370a9a77a9 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -11794,171 +11794,88 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
asPart = createPQExpBuffer();
/* Fetch function-specific details */
- if (fout->remoteVersion >= 120000)
- {
- /*
- * prosupport was added in 12
- */
+ appendPQExpBuffer(query,
+ "SELECT\n"
+ "proretset,\n"
+ "prosrc,\n"
+ "probin,\n"
+ "provolatile,\n"
+ "proisstrict,\n"
+ "prosecdef,\n"
+ "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname,\n");
+
+ if (fout->remoteVersion >= 80300)
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "pg_catalog.pg_get_function_arguments(oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(oid) AS funciargs, "
- "pg_catalog.pg_get_function_result(oid) AS funcresult, "
- "array_to_string(protrftypes, ' ') AS protrftypes, "
- "prokind, provolatile, proisstrict, prosecdef, "
- "proleakproof, proconfig, procost, prorows, "
- "prosupport, proparallel, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 110000)
- {
- /*
- * prokind was added in 11
- */
+ "proconfig,\n"
+ "procost,\n"
+ "prorows,\n");
+ else
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "pg_catalog.pg_get_function_arguments(oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(oid) AS funciargs, "
- "pg_catalog.pg_get_function_result(oid) AS funcresult, "
- "array_to_string(protrftypes, ' ') AS protrftypes, "
- "prokind, provolatile, proisstrict, prosecdef, "
- "proleakproof, proconfig, procost, prorows, "
- "'-' AS prosupport, proparallel, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 90600)
+ "null AS proconfig,\n"
+ "0 AS procost,\n"
+ "0 AS prorows,\n");
+
+ if (fout->remoteVersion >= 80400)
{
/*
- * proparallel was added in 9.6
+ * In 8.4 and up we rely on pg_get_function_arguments and
+ * pg_get_function_result instead of examining proallargtypes etc.
*/
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "pg_catalog.pg_get_function_arguments(oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(oid) AS funciargs, "
- "pg_catalog.pg_get_function_result(oid) AS funcresult, "
- "array_to_string(protrftypes, ' ') AS protrftypes, "
- "CASE WHEN proiswindow THEN 'w' ELSE 'f' END AS prokind, "
- "provolatile, proisstrict, prosecdef, "
- "proleakproof, proconfig, procost, prorows, "
- "'-' AS prosupport, proparallel, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
+ "pg_catalog.pg_get_function_arguments(oid) AS funcargs,\n"
+ "pg_catalog.pg_get_function_identity_arguments(oid) AS funciargs,\n"
+ "pg_catalog.pg_get_function_result(oid) AS funcresult,\n");
}
- else if (fout->remoteVersion >= 90500)
- {
- /*
- * protrftypes was added in 9.5
- */
+ else if (fout->remoteVersion >= 80100)
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "pg_catalog.pg_get_function_arguments(oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(oid) AS funciargs, "
- "pg_catalog.pg_get_function_result(oid) AS funcresult, "
- "array_to_string(protrftypes, ' ') AS protrftypes, "
- "CASE WHEN proiswindow THEN 'w' ELSE 'f' END AS prokind, "
- "provolatile, proisstrict, prosecdef, "
- "proleakproof, proconfig, procost, prorows, "
- "'-' AS prosupport, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 90200)
- {
- /*
- * proleakproof was added in 9.2
- */
+ "proallargtypes,\n"
+ "proargmodes,\n"
+ "proargnames,\n");
+ else
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "pg_catalog.pg_get_function_arguments(oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(oid) AS funciargs, "
- "pg_catalog.pg_get_function_result(oid) AS funcresult, "
- "CASE WHEN proiswindow THEN 'w' ELSE 'f' END AS prokind, "
- "provolatile, proisstrict, prosecdef, "
- "proleakproof, proconfig, procost, prorows, "
- "'-' AS prosupport, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
+ "null AS proallargtypes,\n"
+ "null AS proargmodes,\n"
+ "proargnames,\n");
+
+ if (fout->remoteVersion >= 90200)
+ appendPQExpBuffer(query,
+ "proleakproof,\n");
+ else
+ appendPQExpBuffer(query,
+ "false AS proleakproof,\n");
+
+ if (fout->remoteVersion >= 90500)
+ appendPQExpBuffer(query,
+ "array_to_string(protrftypes, ' ') AS protrftypes,\n");
+
+ if (fout->remoteVersion >= 90600)
+ appendPQExpBuffer(query,
+ "proparallel,\n");
+ else
+ appendPQExpBuffer(query,
+ "'u' AS proparallel,\n");
+
+ if (fout->remoteVersion >= 110000)
+ appendPQExpBuffer(query,
+ "prokind,\n");
else if (fout->remoteVersion >= 80400)
- {
- /*
- * In 8.4 and up we rely on pg_get_function_arguments and
- * pg_get_function_result instead of examining proallargtypes etc.
- */
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "pg_catalog.pg_get_function_arguments(oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(oid) AS funciargs, "
- "pg_catalog.pg_get_function_result(oid) AS funcresult, "
- "CASE WHEN proiswindow THEN 'w' ELSE 'f' END AS prokind, "
- "provolatile, proisstrict, prosecdef, "
- "false AS proleakproof, "
- " proconfig, procost, prorows, "
- "'-' AS prosupport, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 80300)
- {
+ "CASE WHEN proiswindow THEN 'w' ELSE 'f' END AS prokind,\n");
+ else
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "proallargtypes, proargmodes, proargnames, "
- "'f' AS prokind, "
- "provolatile, proisstrict, prosecdef, "
- "false AS proleakproof, "
- "proconfig, procost, prorows, "
- "'-' AS prosupport, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 80100)
- {
+ "'f' AS prokind,\n");
+
+ if (fout->remoteVersion >= 120000)
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "proallargtypes, proargmodes, proargnames, "
- "'f' AS prokind, "
- "provolatile, proisstrict, prosecdef, "
- "false AS proleakproof, "
- "null AS proconfig, 0 AS procost, 0 AS prorows, "
- "'-' AS prosupport, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
+ "prosupport\n");
else
- {
appendPQExpBuffer(query,
- "SELECT proretset, prosrc, probin, "
- "null AS proallargtypes, "
- "null AS proargmodes, "
- "proargnames, "
- "'f' AS prokind, "
- "provolatile, proisstrict, prosecdef, "
- "false AS proleakproof, "
- "null AS proconfig, 0 AS procost, 0 AS prorows, "
- "'-' AS prosupport, "
- "(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
- "FROM pg_catalog.pg_proc "
- "WHERE oid = '%u'::pg_catalog.oid",
- finfo->dobj.catId.oid);
- }
+ "'-' AS prosupport\n");
+
+ appendPQExpBuffer(query,
+ "FROM pg_catalog.pg_proc "
+ "WHERE oid = '%u'::pg_catalog.oid",
+ finfo->dobj.catId.oid);
res = ExecuteSqlQueryForSingleRow(fout, query->data);
@@ -11992,12 +11909,7 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
procost = PQgetvalue(res, 0, PQfnumber(res, "procost"));
prorows = PQgetvalue(res, 0, PQfnumber(res, "prorows"));
prosupport = PQgetvalue(res, 0, PQfnumber(res, "prosupport"));
-
- if (PQfnumber(res, "proparallel") != -1)
- proparallel = PQgetvalue(res, 0, PQfnumber(res, "proparallel"));
- else
- proparallel = NULL;
-
+ proparallel = PQgetvalue(res, 0, PQfnumber(res, "proparallel"));
lanname = PQgetvalue(res, 0, PQfnumber(res, "lanname"));
/*
@@ -12211,7 +12123,7 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
appendPQExpBuffer(q, " SUPPORT %s", prosupport);
}
- if (proparallel != NULL && proparallel[0] != PROPARALLEL_UNSAFE)
+ if (proparallel[0] != PROPARALLEL_UNSAFE)
{
if (proparallel[0] == PROPARALLEL_SAFE)
appendPQExpBufferStr(q, " PARALLEL SAFE");
@@ -13886,27 +13798,8 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
char *aggfullsig = NULL; /* full signature */
char *aggsig_tag;
PGresult *res;
- int i_aggtransfn;
- int i_aggfinalfn;
- int i_aggcombinefn;
- int i_aggserialfn;
- int i_aggdeserialfn;
- int i_aggmtransfn;
- int i_aggminvtransfn;
- int i_aggmfinalfn;
- int i_aggfinalextra;
- int i_aggmfinalextra;
- int i_aggfinalmodify;
- int i_aggmfinalmodify;
- int i_aggsortop;
- int i_aggkind;
- int i_aggtranstype;
- int i_aggtransspace;
- int i_aggmtranstype;
- int i_aggmtransspace;
int i_agginitval;
int i_aggminitval;
- int i_proparallel;
const char *aggtransfn;
const char *aggfinalfn;
const char *aggcombinefn;
@@ -13941,170 +13834,104 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
details = createPQExpBuffer();
/* Get aggregate-specific details */
+ appendPQExpBuffer(query,
+ "SELECT\n"
+ "aggtransfn,\n"
+ "aggfinalfn,\n"
+ "aggtranstype::pg_catalog.regtype,\n"
+ "agginitval,\n");
+
+ if (fout->remoteVersion >= 80100)
+ appendPQExpBuffer(query,
+ "aggsortop,\n");
+ else
+ appendPQExpBuffer(query,
+ "0 AS aggsortop,\n");
+
+ if (fout->remoteVersion >= 80400)
+ appendPQExpBuffer(query,
+ "pg_catalog.pg_get_function_arguments(p.oid) AS funcargs,\n"
+ "pg_catalog.pg_get_function_identity_arguments(p.oid) AS funciargs,\n");
+
+ if (fout->remoteVersion >= 90400)
+ appendPQExpBuffer(query,
+ "aggkind,\n"
+ "aggmtransfn,\n"
+ "aggminvtransfn,\n"
+ "aggmfinalfn,\n"
+ "aggmtranstype::pg_catalog.regtype,\n"
+ "aggfinalextra,\n"
+ "aggmfinalextra,\n"
+ "aggtransspace,\n"
+ "aggmtransspace,\n"
+ "aggminitval,\n");
+ else
+ appendPQExpBuffer(query,
+ "'n' AS aggkind,\n"
+ "'-' AS aggmtransfn,\n"
+ "'-' AS aggminvtransfn,\n"
+ "'-' AS aggmfinalfn,\n"
+ "0 AS aggmtranstype,\n"
+ "false AS aggfinalextra,\n"
+ "false AS aggmfinalextra,\n"
+ "0 AS aggtransspace,\n"
+ "0 AS aggmtransspace,\n"
+ "NULL AS aggminitval,\n");
+
+ if (fout->remoteVersion >= 90600)
+ appendPQExpBuffer(query,
+ "aggcombinefn,\n"
+ "aggserialfn,\n"
+ "aggdeserialfn,\n"
+ "proparallel,\n");
+ else
+ appendPQExpBuffer(query,
+ "'-' AS aggcombinefn,\n"
+ "'-' AS aggserialfn,\n"
+ "'-' AS aggdeserialfn,\n"
+ "'u' AS proparallel,\n");
+
if (fout->remoteVersion >= 110000)
- {
- appendPQExpBuffer(query, "SELECT aggtransfn, "
- "aggfinalfn, aggtranstype::pg_catalog.regtype, "
- "aggcombinefn, aggserialfn, aggdeserialfn, aggmtransfn, "
- "aggminvtransfn, aggmfinalfn, aggmtranstype::pg_catalog.regtype, "
- "aggfinalextra, aggmfinalextra, "
- "aggfinalmodify, aggmfinalmodify, "
- "aggsortop, "
- "aggkind, "
- "aggtransspace, agginitval, "
- "aggmtransspace, aggminitval, "
- "pg_catalog.pg_get_function_arguments(p.oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(p.oid) AS funciargs, "
- "p.proparallel "
- "FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
- "WHERE a.aggfnoid = p.oid "
- "AND p.oid = '%u'::pg_catalog.oid",
- agginfo->aggfn.dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 90600)
- {
- appendPQExpBuffer(query, "SELECT aggtransfn, "
- "aggfinalfn, aggtranstype::pg_catalog.regtype, "
- "aggcombinefn, aggserialfn, aggdeserialfn, aggmtransfn, "
- "aggminvtransfn, aggmfinalfn, aggmtranstype::pg_catalog.regtype, "
- "aggfinalextra, aggmfinalextra, "
- "'0' AS aggfinalmodify, '0' AS aggmfinalmodify, "
- "aggsortop, "
- "aggkind, "
- "aggtransspace, agginitval, "
- "aggmtransspace, aggminitval, "
- "pg_catalog.pg_get_function_arguments(p.oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(p.oid) AS funciargs, "
- "p.proparallel "
- "FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
- "WHERE a.aggfnoid = p.oid "
- "AND p.oid = '%u'::pg_catalog.oid",
- agginfo->aggfn.dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 90400)
- {
- appendPQExpBuffer(query, "SELECT aggtransfn, "
- "aggfinalfn, aggtranstype::pg_catalog.regtype, "
- "'-' AS aggcombinefn, '-' AS aggserialfn, "
- "'-' AS aggdeserialfn, aggmtransfn, aggminvtransfn, "
- "aggmfinalfn, aggmtranstype::pg_catalog.regtype, "
- "aggfinalextra, aggmfinalextra, "
- "'0' AS aggfinalmodify, '0' AS aggmfinalmodify, "
- "aggsortop, "
- "aggkind, "
- "aggtransspace, agginitval, "
- "aggmtransspace, aggminitval, "
- "pg_catalog.pg_get_function_arguments(p.oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(p.oid) AS funciargs "
- "FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
- "WHERE a.aggfnoid = p.oid "
- "AND p.oid = '%u'::pg_catalog.oid",
- agginfo->aggfn.dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 80400)
- {
- appendPQExpBuffer(query, "SELECT aggtransfn, "
- "aggfinalfn, aggtranstype::pg_catalog.regtype, "
- "'-' AS aggcombinefn, '-' AS aggserialfn, "
- "'-' AS aggdeserialfn, '-' AS aggmtransfn, "
- "'-' AS aggminvtransfn, '-' AS aggmfinalfn, "
- "0 AS aggmtranstype, false AS aggfinalextra, "
- "false AS aggmfinalextra, "
- "'0' AS aggfinalmodify, '0' AS aggmfinalmodify, "
- "aggsortop, "
- "'n' AS aggkind, "
- "0 AS aggtransspace, agginitval, "
- "0 AS aggmtransspace, NULL AS aggminitval, "
- "pg_catalog.pg_get_function_arguments(p.oid) AS funcargs, "
- "pg_catalog.pg_get_function_identity_arguments(p.oid) AS funciargs "
- "FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
- "WHERE a.aggfnoid = p.oid "
- "AND p.oid = '%u'::pg_catalog.oid",
- agginfo->aggfn.dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 80100)
- {
- appendPQExpBuffer(query, "SELECT aggtransfn, "
- "aggfinalfn, aggtranstype::pg_catalog.regtype, "
- "'-' AS aggcombinefn, '-' AS aggserialfn, "
- "'-' AS aggdeserialfn, '-' AS aggmtransfn, "
- "'-' AS aggminvtransfn, '-' AS aggmfinalfn, "
- "0 AS aggmtranstype, false AS aggfinalextra, "
- "false AS aggmfinalextra, "
- "'0' AS aggfinalmodify, '0' AS aggmfinalmodify, "
- "aggsortop, "
- "'n' AS aggkind, "
- "0 AS aggtransspace, agginitval, "
- "0 AS aggmtransspace, NULL AS aggminitval "
- "FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
- "WHERE a.aggfnoid = p.oid "
- "AND p.oid = '%u'::pg_catalog.oid",
- agginfo->aggfn.dobj.catId.oid);
- }
+ appendPQExpBuffer(query,
+ "aggfinalmodify,\n"
+ "aggmfinalmodify\n");
else
- {
- appendPQExpBuffer(query, "SELECT aggtransfn, "
- "aggfinalfn, aggtranstype::pg_catalog.regtype, "
- "'-' AS aggcombinefn, '-' AS aggserialfn, "
- "'-' AS aggdeserialfn, '-' AS aggmtransfn, "
- "'-' AS aggminvtransfn, '-' AS aggmfinalfn, "
- "0 AS aggmtranstype, false AS aggfinalextra, "
- "false AS aggmfinalextra, "
- "'0' AS aggfinalmodify, '0' AS aggmfinalmodify, "
- "0 AS aggsortop, "
- "'n' AS aggkind, "
- "0 AS aggtransspace, agginitval, "
- "0 AS aggmtransspace, NULL AS aggminitval "
- "FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
- "WHERE a.aggfnoid = p.oid "
- "AND p.oid = '%u'::pg_catalog.oid",
- agginfo->aggfn.dobj.catId.oid);
- }
+ appendPQExpBuffer(query,
+ "'0' AS aggfinalmodify,\n"
+ "'0' AS aggmfinalmodify\n");
+
+ appendPQExpBuffer(query,
+ "FROM pg_catalog.pg_aggregate a, pg_catalog.pg_proc p "
+ "WHERE a.aggfnoid = p.oid "
+ "AND p.oid = '%u'::pg_catalog.oid",
+ agginfo->aggfn.dobj.catId.oid);
res = ExecuteSqlQueryForSingleRow(fout, query->data);
- i_aggtransfn = PQfnumber(res, "aggtransfn");
- i_aggfinalfn = PQfnumber(res, "aggfinalfn");
- i_aggcombinefn = PQfnumber(res, "aggcombinefn");
- i_aggserialfn = PQfnumber(res, "aggserialfn");
- i_aggdeserialfn = PQfnumber(res, "aggdeserialfn");
- i_aggmtransfn = PQfnumber(res, "aggmtransfn");
- i_aggminvtransfn = PQfnumber(res, "aggminvtransfn");
- i_aggmfinalfn = PQfnumber(res, "aggmfinalfn");
- i_aggfinalextra = PQfnumber(res, "aggfinalextra");
- i_aggmfinalextra = PQfnumber(res, "aggmfinalextra");
- i_aggfinalmodify = PQfnumber(res, "aggfinalmodify");
- i_aggmfinalmodify = PQfnumber(res, "aggmfinalmodify");
- i_aggsortop = PQfnumber(res, "aggsortop");
- i_aggkind = PQfnumber(res, "aggkind");
- i_aggtranstype = PQfnumber(res, "aggtranstype");
- i_aggtransspace = PQfnumber(res, "aggtransspace");
- i_aggmtranstype = PQfnumber(res, "aggmtranstype");
- i_aggmtransspace = PQfnumber(res, "aggmtransspace");
i_agginitval = PQfnumber(res, "agginitval");
i_aggminitval = PQfnumber(res, "aggminitval");
- i_proparallel = PQfnumber(res, "proparallel");
-
- aggtransfn = PQgetvalue(res, 0, i_aggtransfn);
- aggfinalfn = PQgetvalue(res, 0, i_aggfinalfn);
- aggcombinefn = PQgetvalue(res, 0, i_aggcombinefn);
- aggserialfn = PQgetvalue(res, 0, i_aggserialfn);
- aggdeserialfn = PQgetvalue(res, 0, i_aggdeserialfn);
- aggmtransfn = PQgetvalue(res, 0, i_aggmtransfn);
- aggminvtransfn = PQgetvalue(res, 0, i_aggminvtransfn);
- aggmfinalfn = PQgetvalue(res, 0, i_aggmfinalfn);
- aggfinalextra = (PQgetvalue(res, 0, i_aggfinalextra)[0] == 't');
- aggmfinalextra = (PQgetvalue(res, 0, i_aggmfinalextra)[0] == 't');
- aggfinalmodify = PQgetvalue(res, 0, i_aggfinalmodify)[0];
- aggmfinalmodify = PQgetvalue(res, 0, i_aggmfinalmodify)[0];
- aggsortop = PQgetvalue(res, 0, i_aggsortop);
- aggkind = PQgetvalue(res, 0, i_aggkind)[0];
- aggtranstype = PQgetvalue(res, 0, i_aggtranstype);
- aggtransspace = PQgetvalue(res, 0, i_aggtransspace);
- aggmtranstype = PQgetvalue(res, 0, i_aggmtranstype);
- aggmtransspace = PQgetvalue(res, 0, i_aggmtransspace);
+
+ aggtransfn = PQgetvalue(res, 0, PQfnumber(res, "aggtransfn"));
+ aggfinalfn = PQgetvalue(res, 0, PQfnumber(res, "aggfinalfn"));
+ aggcombinefn = PQgetvalue(res, 0, PQfnumber(res, "aggcombinefn"));
+ aggserialfn = PQgetvalue(res, 0, PQfnumber(res, "aggserialfn"));
+ aggdeserialfn = PQgetvalue(res, 0, PQfnumber(res, "aggdeserialfn"));
+ aggmtransfn = PQgetvalue(res, 0, PQfnumber(res, "aggmtransfn"));
+ aggminvtransfn = PQgetvalue(res, 0, PQfnumber(res, "aggminvtransfn"));
+ aggmfinalfn = PQgetvalue(res, 0, PQfnumber(res, "aggmfinalfn"));
+ aggfinalextra = (PQgetvalue(res, 0, PQfnumber(res, "aggfinalextra"))[0] == 't');
+ aggmfinalextra = (PQgetvalue(res, 0, PQfnumber(res, "aggmfinalextra"))[0] == 't');
+ aggfinalmodify = PQgetvalue(res, 0, PQfnumber(res, "aggfinalmodify"))[0];
+ aggmfinalmodify = PQgetvalue(res, 0, PQfnumber(res, "aggmfinalmodify"))[0];
+ aggsortop = PQgetvalue(res, 0, PQfnumber(res, "aggsortop"));
+ aggkind = PQgetvalue(res, 0, PQfnumber(res, "aggkind"))[0];
+ aggtranstype = PQgetvalue(res, 0, PQfnumber(res, "aggtranstype"));
+ aggtransspace = PQgetvalue(res, 0, PQfnumber(res, "aggtransspace"));
+ aggmtranstype = PQgetvalue(res, 0, PQfnumber(res, "aggmtranstype"));
+ aggmtransspace = PQgetvalue(res, 0, PQfnumber(res, "aggmtransspace"));
agginitval = PQgetvalue(res, 0, i_agginitval);
aggminitval = PQgetvalue(res, 0, i_aggminitval);
+ proparallel = PQgetvalue(res, 0, PQfnumber(res, "proparallel"));
if (fout->remoteVersion >= 80400)
{
@@ -14123,11 +13950,6 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
aggsig_tag = format_aggregate_signature(agginfo, fout, false);
- if (i_proparallel != -1)
- proparallel = PQgetvalue(res, 0, PQfnumber(res, "proparallel"));
- else
- proparallel = NULL;
-
/* identify default modify flag for aggkind (must match DefineAggregate) */
defaultfinalmodify = (aggkind == AGGKIND_NORMAL) ? AGGMODIFY_READ_ONLY : AGGMODIFY_READ_WRITE;
/* replace omitted flags for old versions */
@@ -14246,7 +14068,7 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
if (aggkind == AGGKIND_HYPOTHETICAL)
appendPQExpBufferStr(details, ",\n HYPOTHETICAL");
- if (proparallel != NULL && proparallel[0] != PROPARALLEL_UNSAFE)
+ if (proparallel[0] != PROPARALLEL_UNSAFE)
{
if (proparallel[0] == PROPARALLEL_SAFE)
appendPQExpBufferStr(details, ",\n PARALLEL = safe");
--
2.27.0
Re: some more pg_dump refactoring
On 2020-Jul-09, Peter Eisentraut wrote:
On 2020-07-08 06:42, Fabien COELHO wrote:
What do you think about this patch to reorganize the existing code from that
old commit?I think it is a definite further improvement.
Patch applies cleanly, compiles, global & pg_dump tap test ok, looks ok to
me.Thanks. I have committed that, and attached is my original patch adjusted
to this newer style.
Thanks, I too prefer the style where queries are split in lines instead
of a single very long one, at least when looking at log_statement=all
lines generated by pg_dump runs. It's not a *huge* usability
improvement, but I see no reason to make things worse.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: some more pg_dump refactoring
On 2020-07-09 16:14, Peter Eisentraut wrote:
On 2020-07-08 06:42, Fabien COELHO wrote:
What do you think about this patch to reorganize the existing code from that
old commit?I think it is a definite further improvement.
Patch applies cleanly, compiles, global & pg_dump tap test ok, looks ok to
me.Thanks. I have committed that, and attached is my original patch
adjusted to this newer style.
committed
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services