On Tue, May 9, 2017 at 3:51 PM, Ashutosh Bapat <ashutosh.ba...@enterprisedb.com> wrote: > On Tue, May 9, 2017 at 2:59 PM, Amit Langote > <langote_amit...@lab.ntt.co.jp> wrote: >> 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 == 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. > > I think this patch too has the same problem I described in my reply to > Amul; it fires a query to server for every partitioned table it > encounters, that's not very efficient. > Agree with Ashutosh, If such information is already available then we need to leverage of it.
>> >> 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. > > unless the partition is default partition or a hash partition. > >> 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. >> > > I am not sure if it's worth considering partitions and partitioned > table as different entities as far as dump is concerned. We should > probably dump the whole partitioned table or none of it. There's merit > in dumping just a single partition to transfer that data to some other > server, but I am not sure how much the feature would be used. > but won't be useless. > In order to avoid any such potential anomalies, we should copy dump > flag for a partition from that of the parent as I have described in my > reply to Amul. > >> 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. > > If we go about dumping a partition without its parent table, we should > dump CREATE TABLE with proper list of columns and constraints without > PARTITION OF clause. But I am not sure whether we should go that > route. IMO, I think it's worth to dump CREATE TABLE without PARTITION OF clause. Regards, Amul -- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers