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
From eca6fd80b6ca850a0f5e40c40d7cb1be48ab2e2a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@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
