There is a lot of version dependent code in pg_dump now, especially
per-version queries. The way they were originally built was that we
just have an entirely separate query per version. But as the number of
versions and the size of the query grows, this becomes unwieldy.
So I tried, as an example, to write the queries in getTableAttrs()
differently. 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 and is easier
to deal with. I think this patch is useful in its own right, but there
is also the larger question of whether people think this is a better
style going forward, for pg_dump and psql. (It's already in use in psql
in some places.) It won't always work, if the query structure is very
different between versions, but as this example shows, it can be useful.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 7aad405482cc70958cdba41e20ff1c2e4ffd3baf Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Tue, 28 Aug 2018 22:59:07 +0200
Subject: [PATCH] pg_dump: Reorganize getTableAttrs()
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 | 189 ++++++++++----------------------------
1 file changed, 47 insertions(+), 142 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 6763a899d8..9a2ca37475 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -8162,150 +8162,56 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int
numTables)
resetPQExpBuffer(q);
+ appendPQExpBuffer(q,
+ "SELECT\n"
+ "a.attnum,\n"
+ "a.attname,\n"
+ "a.atttypmod,\n"
+ "a.attstattarget,\n"
+ "a.attstorage,\n"
+ "t.typstorage,\n"
+ "a.attnotnull,\n"
+ "a.atthasdef,\n"
+ "a.attisdropped,\n"
+ "a.attlen,\n"
+ "a.attalign,\n");
if (fout->remoteVersion >= 110000)
- {
- /* atthasmissing and attmissingval are new in 11 */
- appendPQExpBuffer(q, "SELECT a.attnum, a.attname,
a.atttypmod, "
- "a.attstattarget,
a.attstorage, t.typstorage, "
- "a.attnotnull,
a.atthasdef, a.attisdropped, "
- "a.attlen,
a.attalign, a.attislocal, "
-
"pg_catalog.format_type(t.oid,a.atttypmod) AS atttypname, "
-
"array_to_string(a.attoptions, ', ') AS attoptions, "
- "CASE WHEN
a.attcollation <> t.typcollation "
- "THEN a.attcollation
ELSE 0 END AS attcollation, "
- "a.attidentity, "
-
"pg_catalog.array_to_string(ARRAY("
- "SELECT
pg_catalog.quote_ident(option_name) || "
- "' ' ||
pg_catalog.quote_literal(option_value) "
- "FROM
pg_catalog.pg_options_to_table(attfdwoptions) "
- "ORDER BY option_name"
- "), E',\n ') AS
attfdwoptions ,"
+ appendPQExpBuffer(q,
"CASE WHEN
a.atthasmissing AND NOT a.attisdropped "
- "THEN a.attmissingval
ELSE null END AS attmissingval "
- "FROM
pg_catalog.pg_attribute a LEFT JOIN pg_catalog.pg_type t "
- "ON a.atttypid =
t.oid "
- "WHERE a.attrelid =
'%u'::pg_catalog.oid "
- "AND a.attnum >
0::pg_catalog.int2 "
- "ORDER BY a.attnum",
-
tbinfo->dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 100000)
- {
- /*
- * attidentity is new in version 10.
- */
- appendPQExpBuffer(q, "SELECT a.attnum, a.attname,
a.atttypmod, "
- "a.attstattarget,
a.attstorage, t.typstorage, "
- "a.attnotnull,
a.atthasdef, a.attisdropped, "
- "a.attlen,
a.attalign, a.attislocal, "
-
"pg_catalog.format_type(t.oid,a.atttypmod) AS atttypname, "
-
"array_to_string(a.attoptions, ', ') AS attoptions, "
- "CASE WHEN
a.attcollation <> t.typcollation "
- "THEN a.attcollation
ELSE 0 END AS attcollation, "
- "a.attidentity, "
-
"pg_catalog.array_to_string(ARRAY("
- "SELECT
pg_catalog.quote_ident(option_name) || "
- "' ' ||
pg_catalog.quote_literal(option_value) "
- "FROM
pg_catalog.pg_options_to_table(attfdwoptions) "
- "ORDER BY option_name"
- "), E',\n ') AS
attfdwoptions ,"
- "NULL as
attmissingval "
- "FROM
pg_catalog.pg_attribute a LEFT JOIN pg_catalog.pg_type t "
- "ON a.atttypid =
t.oid "
- "WHERE a.attrelid =
'%u'::pg_catalog.oid "
- "AND a.attnum >
0::pg_catalog.int2 "
- "ORDER BY a.attnum",
-
tbinfo->dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 90200)
- {
- /*
- * attfdwoptions is new in 9.2.
- */
- appendPQExpBuffer(q, "SELECT a.attnum, a.attname,
a.atttypmod, "
- "a.attstattarget,
a.attstorage, t.typstorage, "
- "a.attnotnull,
a.atthasdef, a.attisdropped, "
- "a.attlen,
a.attalign, a.attislocal, "
-
"pg_catalog.format_type(t.oid,a.atttypmod) AS atttypname, "
-
"array_to_string(a.attoptions, ', ') AS attoptions, "
- "CASE WHEN
a.attcollation <> t.typcollation "
- "THEN a.attcollation
ELSE 0 END AS attcollation, "
+ "THEN a.attmissingval
ELSE null END AS attmissingval,\n");
+ if (fout->remoteVersion >= 100000)
+ appendPQExpBuffer(q,
+ "a.attidentity,\n");
+ if (fout->remoteVersion >= 90200)
+ appendPQExpBuffer(q,
"pg_catalog.array_to_string(ARRAY("
"SELECT
pg_catalog.quote_ident(option_name) || "
"' ' ||
pg_catalog.quote_literal(option_value) "
"FROM
pg_catalog.pg_options_to_table(attfdwoptions) "
"ORDER BY option_name"
- "), E',\n ') AS
attfdwoptions, "
- "NULL as
attmissingval "
- "FROM
pg_catalog.pg_attribute a LEFT JOIN pg_catalog.pg_type t "
- "ON a.atttypid =
t.oid "
- "WHERE a.attrelid =
'%u'::pg_catalog.oid "
- "AND a.attnum >
0::pg_catalog.int2 "
- "ORDER BY a.attnum",
-
tbinfo->dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 90100)
- {
+ "), E',\n ') AS
attfdwoptions,\n");
+ if (fout->remoteVersion >= 90100)
/*
- * attcollation is new in 9.1. 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.
+ * 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.
*/
- appendPQExpBuffer(q, "SELECT a.attnum, a.attname,
a.atttypmod, "
- "a.attstattarget,
a.attstorage, t.typstorage, "
- "a.attnotnull,
a.atthasdef, a.attisdropped, "
- "a.attlen,
a.attalign, a.attislocal, "
-
"pg_catalog.format_type(t.oid,a.atttypmod) AS atttypname, "
-
"array_to_string(a.attoptions, ', ') AS attoptions, "
+ appendPQExpBuffer(q,
"CASE WHEN
a.attcollation <> t.typcollation "
- "THEN a.attcollation
ELSE 0 END AS attcollation, "
- "NULL AS
attfdwoptions, "
- "NULL as
attmissingval "
- "FROM
pg_catalog.pg_attribute a LEFT JOIN pg_catalog.pg_type t "
- "ON a.atttypid =
t.oid "
- "WHERE a.attrelid =
'%u'::pg_catalog.oid "
- "AND a.attnum >
0::pg_catalog.int2 "
- "ORDER BY a.attnum",
-
tbinfo->dobj.catId.oid);
- }
- else if (fout->remoteVersion >= 90000)
- {
- /* attoptions is new in 9.0 */
- appendPQExpBuffer(q, "SELECT a.attnum, a.attname,
a.atttypmod, "
- "a.attstattarget,
a.attstorage, t.typstorage, "
- "a.attnotnull,
a.atthasdef, a.attisdropped, "
- "a.attlen,
a.attalign, a.attislocal, "
-
"pg_catalog.format_type(t.oid,a.atttypmod) AS atttypname, "
-
"array_to_string(a.attoptions, ', ') AS attoptions, "
- "0 AS attcollation, "
- "NULL AS
attfdwoptions, "
- "NULL as
attmissingval "
- "FROM
pg_catalog.pg_attribute a LEFT JOIN pg_catalog.pg_type t "
- "ON a.atttypid =
t.oid "
- "WHERE a.attrelid =
'%u'::pg_catalog.oid "
- "AND a.attnum >
0::pg_catalog.int2 "
- "ORDER BY a.attnum",
-
tbinfo->dobj.catId.oid);
- }
- else
- {
- /* need left join here to not fail on dropped columns
... */
- appendPQExpBuffer(q, "SELECT a.attnum, a.attname,
a.atttypmod, "
- "a.attstattarget,
a.attstorage, t.typstorage, "
- "a.attnotnull,
a.atthasdef, a.attisdropped, "
- "a.attlen,
a.attalign, a.attislocal, "
-
"pg_catalog.format_type(t.oid,a.atttypmod) AS atttypname, "
- "'' AS attoptions, 0
AS attcollation, "
- "NULL AS
attfdwoptions, "
- "NULL as
attmissingval "
- "FROM
pg_catalog.pg_attribute a LEFT JOIN pg_catalog.pg_type t "
- "ON a.atttypid =
t.oid "
- "WHERE a.attrelid =
'%u'::pg_catalog.oid "
- "AND a.attnum >
0::pg_catalog.int2 "
- "ORDER BY a.attnum",
-
tbinfo->dobj.catId.oid);
- }
+ "THEN a.attcollation
ELSE 0 END AS attcollation,\n");
+ if (fout->remoteVersion >= 90000)
+ appendPQExpBuffer(q,
+
"array_to_string(a.attoptions, ', ') AS attoptions,\n");
+ appendPQExpBuffer(q,
+ "a.attislocal,\n"
+
"pg_catalog.format_type(t.oid, a.atttypmod) AS atttypname\n"
+ /* need left join here to not
fail on dropped columns ... */
+ "FROM pg_catalog.pg_attribute
a LEFT JOIN pg_catalog.pg_type t "
+ "ON a.atttypid = t.oid\n"
+ "WHERE a.attrelid =
'%u'::pg_catalog.oid "
+ "AND a.attnum >
0::pg_catalog.int2\n"
+ "ORDER BY a.attnum",
+ tbinfo->dobj.catId.oid);
res = ExecuteSqlQuery(fout, q->data, PGRES_TUPLES_OK);
@@ -8370,10 +8276,10 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int
numTables)
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->attoptions[j] = (i_attoptions >= 0 ?
pg_strdup(PQgetvalue(res, j, i_attoptions)) : NULL);
+ tbinfo->attcollation[j] = (i_attcollation >= 0 ?
atooid(PQgetvalue(res, j, i_attcollation)) : InvalidOid);
+ tbinfo->attfdwoptions[j] = (i_attfdwoptions >= 0 ?
pg_strdup(PQgetvalue(res, j, i_attfdwoptions)) : NULL);
+ tbinfo->attmissingval[j] = (i_attmissingval >= 0 ?
pg_strdup(PQgetvalue(res, j, i_attmissingval)) : NULL);
tbinfo->attrdefs[j] = NULL; /* fix below */
if (PQgetvalue(res, j, i_atthasdef)[0] == 't')
hasdefaults = true;
@@ -15753,7 +15659,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
{
for (j = 0; j < tbinfo->numatts; j++)
{
- if (tbinfo->attmissingval[j][0] != '\0')
+ if (tbinfo->attmissingval[j][0])
{
appendPQExpBufferStr(q, "\n-- set
missing value.\n");
appendPQExpBufferStr(q,
@@ -16023,7 +15929,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
/*
* Dump per-column attributes.
*/
- if (tbinfo->attoptions[j] && tbinfo->attoptions[j][0]
!= '\0')
+ if (tbinfo->attoptions[j])
{
appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
qualrelname);
@@ -16037,8 +15943,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
* Dump per-column fdw options.
*/
if (tbinfo->relkind == RELKIND_FOREIGN_TABLE &&
- tbinfo->attfdwoptions[j] &&
- tbinfo->attfdwoptions[j][0] != '\0')
+ tbinfo->attfdwoptions[j])
{
appendPQExpBuffer(q, "ALTER FOREIGN TABLE %s ",
qualrelname);
--
2.18.0
