Re: [HACKERS] dropping partitioned tables without CASCADE

Hi Ashutosh,

Thanks for taking a look at the patch.

On 2017/02/20 21:49, Ashutosh Bapat wrote:
> Thanks for working on all the follow on work for partitioning feature.
> 
> May be you should add all those patches in the next commitfest, so
> that we don't forget those.

I think adding these as one of the PostgreSQL 10 Open Items [0] might be
better.  I've done that.

> On Mon, Feb 20, 2017 at 7:46 AM, Amit Langote wrote:
>> So I count more than a few votes saying that we should be able to DROP
>> partitioned tables without specifying CASCADE.
>>
>> I tried to implement that using the attached patch by having
>> StoreCatalogInheritance1() create DEPENDENCY_AUTO dependency between
>> parent and child if the child is a partition, instead of DEPENDENCY_NORMAL
>> that would otherwise be created.  Now it seems that that is one way of
>> making sure that partitions are dropped when the root partitioned table is
>> dropped, not sure if the best; why create the pg_depend entries at all one
>> might ask.  I chose it for now because that's the one with fewest lines of
>> change.  Adjusted regression tests as well, since we recently tweaked
>> tests [1] to work around the irregularities of test output when using CASCADE.
> 
> The patch applies cleanly and regression does not show any failures.
> 
> Here are some comments
> 
> For the sake of readability you may want reverse the if and else order.
> -    recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
> +    if (!child_is_partition)
> +        recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);
> +    else
> +        recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO);
> like
> +    if (child_is_partition)
> +        recordDependencyOn(&childobject, &parentobject, DEPENDENCY_AUTO);
> +    else
> +        recordDependencyOn(&childobject, &parentobject, DEPENDENCY_NORMAL);

Sure, done.

> It's weird that somebody can perform DROP TABLE on the partition without
> referring to its parent. That may be a useful feature as it allows one to
> detach the partition as well as remove the table in one command. But it looks
> wierd for someone familiar with partitioning features of other DBMSes. But then
> our partition creation command is CREATE TABLE .... So may be this is expected
> difference.

There is a line on the CREATE TABLE page in the description of PARTITION
OF clause:

"Note that dropping a partition with DROP TABLE requires taking an ACCESS
EXCLUSIVE lock on the parent table."

In earlier proposals I had included the ALTER TABLE parent ADD/DROP
PARTITION commands, but CRAETE TABLE PARTITION OF / DROP TABLE prevailed.

> --- cleanup: avoid using CASCADE
> -DROP TABLE list_parted, part_1;
> -DROP TABLE list_parted2, part_2, part_5, part_5_a;
> -DROP TABLE range_parted, part1, part2;
> +-- cleanup
> +DROP TABLE list_parted, list_parted2, range_parted;
> Testcases usually drop one table at a time, I guess, to reduce the differences
> when we add or remove tables from testcases. All such blocks should probably
> follow same policy.

Hmm, I see this in src/test/regress/sql/inherit.sql:141

DROP TABLE firstparent, secondparent, jointchild, thirdparent, otherchild;

>  drop table list_parted cascade;
> -NOTICE:  drop cascades to 3 other objects
> -DETAIL:  drop cascades to table part_ab_cd
> probably we should remove cascade from there, unless you are testing CASCADE
> functionality. Similarly for other blocks like
>  drop table range_parted cascade;
> 
> BTW, I noticed that although we are allowing foreign tables to be
> partitions, there are no tests in foreign_data.sql for testing it. If
> there would have been we would tests DROP TABLE on a partitioned table
> with foreign partitions as well. That file has testcases for testing
> foreign table inheritance, and should have tests for foreign table
> partitions.

That makes sense.  Patch 0002 is for that (I'm afraid this should be
posted separately though).  I didn't add/repeat all the tests that were
added by the foreign table inheritance patch again for foreign partitions
(common inheritance rules apply to both cases), only added those for the
new partitioning commands and certain new rules.

Thanks,
Amit

[0] https://wiki.postgresql.org/wiki/PostgreSQL_10_Open_Items

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers