Re: Dropping partitioned table drops a previously detached partition

I reviewed and tested
0001-Dependency-between-partitioned-table-and-partition_v1.patch
It applies cleanly on master and make check passes.

Following are few comments:

>/*
> * Drop the dependency created by StoreCatalogInheritance1 (CREATE TABLE
> * INHERITS/ALTER TABLE INHERIT -- refclassid will be RelationRelationId)
or
> * heap_create_with_catalog (CREATE TABLE OF/ALTER TABLE OF -- refclassid
will
> * be TypeRelationId). There's no convenient way to do this, so go
trawling
> * through pg_depend.
> */
>static void
>drop_parent_dependency(Oid relid, Oid refclassid, Oid refobjid,
DependencyType deptype)

This is not directly related to this patch but still would like to mention.
drop_parent_dependency() is being used to drop the dependency created
by CREATE TABLE PARTITION OF command also, so probably the comment
needs to be modified to reflect that.

>+/*
>+ * Partition tables are expected to be dropped when the parent partitioned
>+ * table gets dropped. Hence for partitioning we use AUTO dependency.
>+ * Otherwise, for regular inheritance use NORMAL dependency.
A minor cosmetic correction:
+ Hence for *declarative* partitioning we use AUTO dependency
+ * Otherwise, for regular inheritance *we* use NORMAL dependency.

I have added tests to the
0001-Dependency-between-partitioned-table-and-partition_v1.patch. Please
find attached the v2 patch.

On Tue, Jun 13, 2017 at 10:25 AM, Amit Langote <
Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:

> Hi Ashutosh,
>
> On 2017/06/12 20:56, Ashutosh Bapat wrote:
> > Hi,
> > If we detach a partition and drop the corresponding partitioned table,
> > it drops the once-partition now-standalone table as well.
>
> Thanks for the report. Looks like 8b4d582d279d78 missed the bit about
> drop_parent_dependency() that you describe in your analysis below.
>
> > The reason for this is as follows
> > An AUTO dependency is recorded between a partitioned table and
> partition. In
> > case of inheritance we record a NORMAL dependency between parent
> > and child. While
> > detaching a partition, we call RemoveInheritance(), which should
> have taken
> > care of removing this dependency. But it removes the dependencies
> which are
> > marked as NORMAL. When we changed the dependency for partitioned
> case from
> > NORMAL to AUTO by updating StoreCatalogInheritance1(), this function
> was not
> > updated. This caused the dependency to linger behind even after
> > detaching the
> > partition, thus causing the now standalone table (which was once a
> > partition)
> > to be dropped when the parent partitioned table is dropped. This
> patch fixes
> > RemoveInheritance() to choose appropriate dependency.
> >
> > Attached patch 0001 to fix this.
>
> Looks good to me. Perhaps, the example in your email could be added as a
> test case.
>
> > I see a similar issue-in-baking in case we change the type of
> > dependency recorded between a table and the composite type associated
> > with using CREATE TABLE ... OF command. 0002 patch addresses the
> > potential issue by using a macro while creating and dropping the
> > dependency in corresponding functions. There might be more such
> > places, but I haven't searched for those.
>
> Might be a good idea too.
>
> Adding this to the open items list.
>
> Thanks,
> Amit
>
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>