pgsql: Redesign the partition dependency mechanism. - Mailing list pgsql-committers

From Tom Lane
Subject pgsql: Redesign the partition dependency mechanism.
Date
Msg-id E1gtHSO-0007BI-WA@gemulon.postgresql.org
Whole thread Raw
List pgsql-committers
Redesign the partition dependency mechanism.

The original setup for dependencies of partitioned objects had
serious problems:

1. It did not verify that a drop cascading to a partition-child object
also cascaded to at least one of the object's partition parents.  Now,
normally a child object would share all its dependencies with one or
another parent (e.g. a child index's opclass dependencies would be shared
with the parent index), so that this oversight is usually harmless.
But if some dependency failed to fit this pattern, the child could be
dropped while all its parents remain, creating a logically broken
situation.  (It's easy to construct artificial cases that break it,
such as attaching an unrelated extension dependency to the child object
and then dropping the extension.  I'm not sure if any less-artificial
cases exist.)

2. Management of partition dependencies during ATTACH/DETACH PARTITION
was complicated and buggy; for example, after detaching a partition
table it was possible to create cases where a formerly-child index
should be dropped and was not, because the correct set of dependencies
had not been reconstructed.

Less seriously, because multiple partition relationships were
represented identically in pg_depend, there was an order-of-traversal
dependency on which partition parent was cited in error messages.
We also had some pre-existing order-of-traversal hazards for error
messages related to internal and extension dependencies.  This is
cosmetic to users but causes testing problems.

To fix #1, add a check at the end of the partition tree traversal
to ensure that at least one partition parent got deleted.  To fix #2,
establish a new policy that partition dependencies are in addition to,
not instead of, a child object's usual dependencies; in this way
ATTACH/DETACH PARTITION need not cope with adding or removing the
usual dependencies.

To fix the cosmetic problem, distinguish between primary and secondary
partition dependency entries in pg_depend, by giving them different
deptypes.  (They behave identically except for having different
priorities for being cited in error messages.)  This means that the
former 'I' dependency type is replaced with new 'P' and 'S' types.

This also fixes a longstanding bug that after handling an internal
dependency by recursing to the owning object, findDependentObjects
did not verify that the current target was now scheduled for deletion,
and did not apply the current recursion level's objflags to it.
Perhaps that should be back-patched; but in the back branches it
would only matter if some concurrent transaction had removed the
internal-linkage pg_depend entry before the recursive call found it,
or the recursive call somehow failed to find it, both of which seem
unlikely.

Catversion bump because the contents of pg_depend change for
partitioning relationships.

Patch HEAD only.  It's annoying that we're not fixing #2 in v11,
but there seems no practical way to do so given that the problem
is exactly a poor choice of what entries to put in pg_depend.
We can't really fix that while staying compatible with what's
in pg_depend in existing v11 installations.

Discussion: https://postgr.es/m/CAH2-Wzkypv1R+teZrr71U23J578NnTBt2X8+Y=Odr4pOdW1rXg@mail.gmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/1d92a0c9f7dd547ad14cd8a3974289c5ec7f04ce

Modified Files
--------------
doc/src/sgml/catalogs.sgml             | 100 +++++++++++-----
src/backend/catalog/dependency.c       | 202 +++++++++++++++++++++++++++------
src/backend/catalog/index.c            |  36 +++---
src/backend/catalog/pg_constraint.c    |  25 ++--
src/backend/commands/indexcmds.c       |  36 +++---
src/backend/commands/tablecmds.c       |  24 ++--
src/backend/commands/trigger.c         |  16 ++-
src/include/catalog/catversion.h       |   2 +-
src/include/catalog/dependency.h       |  63 +---------
src/include/catalog/pg_constraint.h    |   3 +-
src/test/regress/expected/indexing.out |  24 ++++
src/test/regress/sql/indexing.sql      |  10 ++
12 files changed, 361 insertions(+), 180 deletions(-)


pgsql-committers by date:

Previous
From: Alvaro Herrera
Date:
Subject: pgsql: Fix misleading PG_RE_THROW commentary
Next
From: Thomas Munro
Date:
Subject: Re: pgsql: Use better comment marker in Autoconf input