Hi Amul,
On 2017/05/09 16:13, amul sul wrote:
> Hi,
>
> Current pg_dump --exclude-table option excludes partitioned relation
> and dumps all its child relations and vice versa for --table option, which
> I think is incorrect.
I agree that `pg_dump -t partitioned_table` should dump all of its
partitions too and that `pg_dump -T partitioned_table` should exclude
partitions. Your patch achieves that. Some comments:
I think we should update doc/src/sgml/ref/pg_dump.sgml to mention this
behavior.
+static void
+find_partition_by_relid(Archive *fout, Oid parentId, SimpleOidList *oids)
Is expand_partitioned_table() a slightly better name?
+ appendPQExpBuffer(query, "SELECT relkind "
+ "FROM pg_catalog.pg_class "
+ "WHERE oid = %u", partid);
+
+ res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+
+ if (PQntuples(res) == 0)
+ exit_horribly(NULL, "no matching partition tables were
");
+
+ relkind = PQgetvalue(res, 0, 0);
+
+ if (relkind[0] == RELKIND_PARTITIONED_TABLE)
+ find_partition_by_relid(fout, partid, oids);
Instead of recursing like this requiring to send one query for every
partition, why not issue just one query such that all the partitions in
the inheritance tree are returned. For example, as follows:
+ appendPQExpBuffer(query, "WITH RECURSIVE partitions (partoid) AS ("
+ " SELECT inhrelid"
+ " FROM pg_inherits"
+ " WHERE inhparent = %u"
+ " UNION ALL"
+ " SELECT inhrelid"
+ " FROM pg_inherits, partitions"
+ " WHERE inhparent = partitions.partoid )"
+ " SELECT partoid FROM partitions",
+ parentId);
I included your patch with the above modifications in the attached 0001
patch, which also contains the documentation updates. Please take a look.
When testing the patch, I found that if --table specifies the parent and
--exclude specifies one of its partitions, the latter won't be forcefully
be included due to the partitioned table expansion, which seems like an
acceptable behavior. On the other hand, if --table specifies a partition
and --exclude specifies the partition's parent, the latter makes partition
to be excluded as well as part of the expansion, which seems fine too.
One more thing I am wondering about is whether `pg_dump -t partition`
should be dumped as a partition definition (that is, as CREATE TABLE
PARTITION OF) which is what happens currently or as a regular table (just
CREATE TABLE)? I'm thinking the latter would be better, but would like to
confirm before writing any code for that.
I will add this as an open item. Thanks for working on it.
Thanks,
Amit
>From 3b3103d6a23aab78592d7547e11f7e6414cfe0ec Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Tue, 9 May 2017 16:39:14 +0900
Subject: [PATCH] Expand partitioned table specified using pg_dump's -t or -T
options
Report and patch by Amul Sul.
---
doc/src/sgml/ref/pg_dump.sgml | 12 +++++----
src/bin/pg_dump/pg_dump.c | 57 +++++++++++++++++++++++++++++++++++++++++--
2 files changed, 62 insertions(+), 7 deletions(-)
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 6cf7e570ef..c90a3298ca 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -520,10 +520,11 @@ PostgreSQL documentation
<replaceable class="parameter">table</replaceable>.
For this purpose, <quote>table</> includes views, materialized views,
sequences, and foreign tables. Multiple tables
- can be selected by writing multiple <option>-t</> switches. Also, the
- <replaceable class="parameter">table</replaceable> parameter is
- interpreted as a pattern according to the same rules used by
- <application>psql</>'s <literal>\d</> commands (see <xref
+ can be selected by writing multiple <option>-t</> switches. If the
+ specified table is a partitioned table, its partitions are dumped
+ too. Also, the <replaceable class="parameter">table</replaceable>
+ parameter is interpreted as a pattern according to the same rules used
+ by <application>psql</>'s <literal>\d</> commands (see <xref
linkend="APP-PSQL-patterns" endterm="APP-PSQL-patterns-title">),
so multiple tables can also be selected by writing wildcard characters
in the pattern. When using wildcards, be careful to quote the pattern
@@ -572,7 +573,8 @@ PostgreSQL documentation
class="parameter">table</replaceable> pattern. The pattern is
interpreted according to the same rules as for <option>-t</>.
<option>-T</> can be given more than once to exclude tables
- matching any of several patterns.
+ matching any of several patterns. If the specified table is a
+ partitioned table, its partitions are not dumped either.
</para>
<para>
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index af84c25093..00146786e3 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1250,6 +1250,43 @@ expand_schema_name_patterns(Archive *fout,
}
/*
+ * Expand partitioned table and append the OIDs of its partitions to the list
+ * of tables to be dumped.
+ */
+static void
+expand_partitioned_table(Archive *fout, Oid parentId, SimpleOidList *oids)
+{
+
+ PGresult *partitions;
+ int partition;
+ PQExpBuffer query = createPQExpBuffer();
+
+ /* Get the OIDs of partitions of all levels */
+ appendPQExpBuffer(query, "WITH RECURSIVE partitions (partoid) AS ("
+ " SELECT inhrelid"
+ " FROM pg_inherits"
+ " WHERE inhparent = %u"
+ " UNION ALL"
+ " SELECT inhrelid"
+ " FROM pg_inherits, partitions"
+ " WHERE inhparent = partitions.partoid )"
+ " SELECT partoid FROM partitions",
+ parentId);
+
+ partitions = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+
+ for (partition = 0; partition < PQntuples(partitions); partition++)
+ {
+ Oid partid = atooid(PQgetvalue(partitions, partition, 0));
+
+ simple_oid_list_append(oids, partid);
+ }
+
+ PQclear(partitions);
+ destroyPQExpBuffer(query);
+}
+
+/*
* Find the OIDs of all tables matching the given list of patterns,
* and append them to the given OID list.
*/
@@ -1276,7 +1313,7 @@ expand_table_name_patterns(Archive *fout,
for (cell = patterns->head; cell; cell = cell->next)
{
appendPQExpBuffer(query,
- "SELECT c.oid"
+ "SELECT c.oid, c.relkind"
"\nFROM pg_catalog.pg_class c"
"\n LEFT JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace"
"\nWHERE c.relkind in ('%c', '%c', '%c', '%c', '%c', '%c')\n",
@@ -1293,7 +1330,23 @@ expand_table_name_patterns(Archive *fout,
for (i = 0; i < PQntuples(res); i++)
{
- simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
+ Oid relOid = atooid(PQgetvalue(res, i, 0));
+
+ simple_oid_list_append(oids, relOid);
+
+ /*
+ * Starting in PostgreSQL 10, we have partitioned tables, whose
+ * partitions we must include in the list of tables to be included
+ * or excluded along with the partitioned table named using the
+ * command-line option.
+ */
+ if (fout->remoteVersion >= 100000)
+ {
+ char relkind = *(PQgetvalue(res, i, 1));
+
+ if (relkind == RELKIND_PARTITIONED_TABLE)
+ expand_partitioned_table(fout, relOid, oids);
+ }
}
PQclear(res);
--
2.11.0
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
