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