From: | Rahila Syed <rahilasyed90(at)gmail(dot)com> |
---|---|
To: | Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> |
Cc: | Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Dropping partitioned table drops a previously detached partition |
Date: | 2017-06-13 13:44:12 |
Message-ID: | CAH2L28um0v5srZcAVwOo4ysVcL5a1gWobXhFLys9ce8iWGudvA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
>
Attachment | Content-Type | Size |
---|---|---|
0001-Dependency-between-partitioned-table-and-partition_v2.patch | text/x-patch | 5.9 KB |
0002-Macro-for-dendency-between-table-and-its-composite-t_v1.patch | text/x-patch | 2.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2017-06-13 13:53:45 | Re: pgrowlocks relkind check |
Previous Message | Tom Lane | 2017-06-13 13:37:59 | Re: Transactional sequence stuff breaks pg_upgrade |