Re: dropping partitioned tables without CASCADE

From: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
To: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: dropping partitioned tables without CASCADE
Date: 2017-02-20 12:49:14
Message-ID: CAFjFpRcrdzBRj0cZ+JAQmfSa2Tv8wSEcWAeYtDpV-YZnNna2sA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

On Mon, Feb 20, 2017 at 7:46 AM, Amit Langote
<Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> Re-posting the patch I posted in a nearby thread [0].
>
> On 2017/02/16 2:08, Robert Haas wrote:
>> On Wed, Feb 15, 2017 at 11:34 AM, Alvaro Herrera
>> <alvherre(at)2ndquadrant(dot)com> wrote:
>>> I think new-style partitioning is supposed to consider each partition as
>>> an implementation detail of the table; the fact that you can manipulate
>>> partitions separately does not really mean that they are their own
>>> independent object. You don't stop to think "do I really want to drop
>>> the TOAST table attached to this main table?" and attach a CASCADE
>>> clause if so. You just drop the main table, and the toast one is
>>> dropped automatically. I think new-style partitions should behave
>>> equivalently.
>>
>> That's a reasonable point of view. I'd like to get some more opinions
>> on this topic. I'm happy to have us do whatever most people want, but
>> I'm worried that having table inheritance and table partitioning work
>> differently will be create confusion. I'm also suspicious that there
>> may be some implementation difficulties. On the hand, it does seem a
>> little silly to say that DROP TABLE partitioned_table should always
>> fail except in the degenerate case where there are no partitions, so
>> maybe changing it is for the best.
>
> 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);

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.

--- 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.

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.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Christoph Berg 2017-02-20 12:51:50 powerpc(32) point/polygon regression failures on Debian Jessie
Previous Message Tom Lane 2017-02-20 12:44:13 Re: 答复: [HACKERS] Adding new output parameter of pg_stat_statements toidentify operation of the query.(Internet mail)