Re: propagating replica identity to partitions

On 2019-Feb-20, Robert Haas wrote:

> On Tue, Feb 19, 2019 at 5:41 PM Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:
> > OK, let me concede that point -- it's not rewriting that makes things
> > act differently, but rather TABLESPACE (as well as some other things)
> > behave that way. ALTER TABLE ... SET DATA TYPE is the obvious
> > counterexample.
> >
> > The Notes section of ALTER TABLE says:
> >
> > : The actions for identity columns (ADD GENERATED, SET etc., DROP IDENTITY), as
> > : well as the actions TRIGGER, CLUSTER, OWNER, and TABLESPACE never recurse to
> > : descendant tables; that is, they always act as though ONLY were specified.
> > : Adding a constraint recurses only for CHECK constraints that are not marked NO
> > : INHERIT.
>
> I don't see that in the NOTES section for ALTER TABLE. Are you
> looking at some patch, or at master?

I was looking here:
https://www.postgresql.org/docs/11/sql-altertable.html

> More generally, our documentation in this area seems to be somewhat
> lacking. For instance, the TABLESPACE section of the ALTER TABLE
> documentation appears to make no mention of what exactly the behavior
> is when applied to a partition table. It doesn't seem sufficient to
> simply say "well, it doesn't recurse," because that would imply that
> it doesn't do anything at all, which isn't the case.

That's true. Maybe we can add a short blurb in the stanza for the SET
TABLESPACE form in the Description section. It currently says:

: This form changes the table's tablespace to the specified tablespace
: and moves the data file(s) associated with the table to the new
: tablespace. Indexes on the table, if any, are not moved; but they
: can be moved separately with additional SET TABLESPACE commands. All
: tables in the current database in a tablespace can be moved by using
: the ALL IN TABLESPACE form, which will lock all tables to be moved
: first and then move each one. This form also supports OWNED BY,
: which will only move tables owned by the roles specified. If the
: NOWAIT option is specified then the command will fail if it is
: unable to acquire all of the locks required immediately. Note that
: system catalogs are not moved by this command, use ALTER DATABASE or
: explicit ALTER TABLE invocations instead if desired. The
: information_schema relations are not considered part of the system
: catalogs and will be moved. See also CREATE TABLESPACE.

Maybe the ALL IN TABLESPACE and OWNED BY sub-forms should be split to a
separate para. I suggest:

: This form changes the table's tablespace to the specified tablespace
: and moves the data file(s) associated with the table to the new
: tablespace. Indexes on the table, if any, are not moved; but they
: can be moved separately with additional SET TABLESPACE commands.
: When applied to a partitioned table, nothing is moved, but any
: partitions created afterwards with CREATE TABLE PARTITION OF
: will use that tablespace.
:
: All
: tables in the current database in a tablespace can be moved by using
: the ALL IN TABLESPACE form, which will lock all tables to be moved
: first and then move each one. This form also supports OWNED BY,
: which will only move tables owned by the roles specified. If the
: NOWAIT option is specified then the command will fail if it is
: unable to acquire all of the locks required immediately. Note that
: system catalogs are not moved by this command, use ALTER DATABASE or
: explicit ALTER TABLE invocations instead if desired. The
: information_schema relations are not considered part of the system
: catalogs and will be moved. See also CREATE TABLESPACE.

> > Since REPLICA IDENTITY does not appear in this list, the documented
> > behavior is to recurse, per the description of the "name" parameter:
> >
> > : The name (optionally schema-qualified) of an existing table to
> > : alter. If ONLY is specified before the table name, only that table
> > : is altered. If ONLY is not specified, the table and all its
> > : descendant tables (if any) are altered. Optionally, * can be
> > : specified after the table name to explicitly indicate that
> > : descendant tables are included.
> >
> > I didn't come up with this on my own, as you imply.
>
> Fair enough. I don't think it's entirely useful to think about this
> in terms of which operations do and don't recurse; the question in my
> mind is WHY some things recurse and other things don't, and what will
> be easiest for users to understand.

I think the reason tablespace was made not to recurse was to avoid
acquiring access exclusive lock on too many tables at once, but I'm not
sure. This is very old behavior.

> Obviously any property where the
> parents and children have to match, like adding a column or changing a
> data type, has to always recurse; nothing else is sensible. For
> others, it seems we have a choice. It's unclear to me why we'd choose
> to make REPLICA IDENTITY recurse by default and, say, OWNER not
> recurse.

Ah. I did argue that OWNER should recurse:
https://postgr.es/m/20171017163203.uw7hmlqonidlfeqj@alvherre.pgsql
but it was too late then to change it for pg10. We can change it now,
surely.

> In both cases, the default assumption is presumably that the
> whole partitioning hierarchy should match, but in a particular case a
> user could want to do something else. Consequently, I don't
> understand why a user would want one of those operations to descend to
> children by default and the other not.

I agree that OWNER TO should recurse for partitioned tables. I'm -0 on
changing it for legacy inheritance, but if the majority is to change it
for both, let's do that.

> It feels like we're choosing the behavior in individual cases, as Tom
> would say, with the aid of a dartboard. Maybe there's some coherent
> principle here that I'm just missing.

I don't think that's a thing we're doing now. Rather we all did it as a
group years ago, and we're now finding that some of the darts landed in
the wrong spot of the board. I don't disagree with fixing all the ones
we know about (I volunteer to do that), but I don't want to conduct a
full audit of tablecmds.c.

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services