Re: Fixing findDependentObjects()'s dependency on scan order (regressions in DROP diagnostic messages) - Mailing list pgsql-hackers

I wrote:
>>> I've got much of the code for it already (in the wreckage of my failed
>>> attempts), so I'll go back and finish that up.

So I've been poking at that for most of the day, and I've despaired of
being able to fix this in v11.  The problem is that somebody was rolling
dice while deciding which dependencies of what types to insert for
partitioning cases.  In particular, try this (extracted from
foreign_key.sql):

CREATE TABLE fk_notpartitioned_pk (a int, b int, PRIMARY KEY (a, b));

CREATE TABLE fk_partitioned_fk (b int, a int) PARTITION BY RANGE (a, b);
ALTER TABLE fk_partitioned_fk ADD FOREIGN KEY (a, b) REFERENCES fk_notpartitioned_pk;

CREATE TABLE fk_partitioned_fk_2 PARTITION OF fk_partitioned_fk FOR VALUES FROM (1,1) TO (10,10);

At this point the dependencies of the child table's FK constraint look
like

select
  pg_describe_object(classid, objid, objsubid) as obj,
  pg_describe_object(refclassid, refobjid, refobjsubid) as ref,
  deptype, objid, refobjid
from pg_depend where objid =
  (select oid from pg_constraint where conrelid = 'fk_partitioned_fk_2'::regclass);

                               obj                                |                              ref
          | deptype | objid | refobjid  

------------------------------------------------------------------+----------------------------------------------------------------+---------+-------+----------
 constraint fk_partitioned_fk_a_fkey on table fk_partitioned_fk_2 | column a of table fk_partitioned_fk_2
          | a       | 52787 |    52784 
 constraint fk_partitioned_fk_a_fkey on table fk_partitioned_fk_2 | column b of table fk_partitioned_fk_2
          | a       | 52787 |    52784 
 constraint fk_partitioned_fk_a_fkey on table fk_partitioned_fk_2 | column a of table fk_notpartitioned_pk
          | n       | 52787 |    37209 
 constraint fk_partitioned_fk_a_fkey on table fk_partitioned_fk_2 | column b of table fk_notpartitioned_pk
          | n       | 52787 |    37209 
 constraint fk_partitioned_fk_a_fkey on table fk_partitioned_fk_2 | index fk_notpartitioned_pk_pkey
          | n       | 52787 |    37215 
 constraint fk_partitioned_fk_a_fkey on table fk_partitioned_fk_2 | constraint fk_partitioned_fk_a_fkey on table
fk_partitioned_fk| I       | 52787 |    52781 
(6 rows)

Now, v11 will let you drop the fk_partitioned_fk_2 table, which
recurses to dropping this constraint, and it does not then complain
that you should've dropped the fk_partitioned_fk_a_fkey constraint.
Given these dependencies, that's just *wrong*.  This dependency set
is no different from the bug case I exhibited upthread: arriving at
this object via an auto dependency should not be enough to let it
be dropped, if there's an 'I' dependency for it.

It's also fair to wonder what we're doing applying a single 'I'
dependency to an object in the first place; why not use a regular
internal dependency in that case?

Another problem I came across can be exhibited like this:

CREATE TABLE prt1 (a int, b int, c int) PARTITION BY RANGE(a);
CREATE INDEX ON prt1(c);

CREATE TABLE prt1_p3 PARTITION OF prt1 FOR VALUES FROM (500) TO (600);

alter table prt1 DETACH PARTITION prt1_p3;

At this point we have

select
  pg_describe_object(classid, objid, objsubid) as obj,
  pg_describe_object(refclassid, refobjid, refobjsubid) as ref,
  deptype, objid, refobjid
from pg_depend where 'prt1_p3_c_idx'::regclass in (objid,refobjid);

         obj         |      ref      | deptype | objid | refobjid
---------------------+---------------+---------+-------+----------
 index prt1_p3_c_idx | table prt1_p3 | a       | 52797 |    52794
(1 row)

This is also flat wrong, because it wil let you do

alter table prt1_p3 drop column c;

and the index still exists, though in a broken state:

\d prt1_p3

              Table "public.prt1_p3"
 Column |  Type   | Collation | Nullable | Default
--------+---------+-----------+----------+---------
 a      | integer |           |          |
 b      | integer |           |          |
Indexes:
    "prt1_p3_c_idx" btree ("........pg.dropped.3........" int4_ops)

The reason for that is that IndexSetParentIndex thinks that if it's
unlinking an index from a partition index, all it needs to put back
is an auto dependency on the whole table.  This has exactly nothing
to do with the dependencies that the index would normally have;
those are usually column-level not table-level.  As this example shows,
you do not get to take shortcuts.

I believe that we need to establish the following principles:

* The terminology "internal_auto" is disastrously unhelpful.
I propose calling these things "partition" dependencies instead.

* Partition dependencies are not singletons.  They should *always*
come in pairs, one on the parent partitioned object (partitioned
index, constraint, trigger, etc) and one on the child partitioned
table.  (Have I mentioned that our partition/partitioned terminology
is also a disaster?)  Maybe someday there'll be a reason to have
three or more of these for the same dependent object, but there's
no reason to have just one --- you should use an internal dependency
instead for that case.

* Partition dependencies are made *in addition to*, not instead of,
the dependencies the object would normally have.  In this way,
for example, the unlink action in IndexSetParentIndex would just
delete the partition dependencies and not have to worry about putting
back the index's usual dependencies.  Consistent with that, it's
simply wrong that index_create sometimes marks the "usual"
dependencies as internal_auto rather than auto.  (It wasn't even
doing that consistently; expression and predicate column dependencies
were still "auto", which makes no sense at all.)  They should go
back to being plain auto with the partition dependencies being added
separately.  That will work properly given the changes that say that
arriving at a partition-dependent object via an auto dependency is
not sufficient license to delete the object.

I have a mostly-working patch along these lines that I hope to
finish up and post tomorrow.  But there's no hope of applying it
to v11 --- if presented with the dependencies that v11 is currently
storing, the patch would refuse deletion of partitioned tables altogether
in many common cases.  That cure's certainly worse than the disease.

            regards, tom lane