Re: why can't a table be part of the same publication as its schema

On 9/19/22 4:52 PM, Jonathan S. Katz wrote:
> On 9/19/22 11:16 AM, Alvaro Herrera wrote:

>> This seems a pretty arbitrary restriction.  It feels like you're adding
>> this restriction precisely so that you don't have to write the code to
>> reject the ALTER .. SET SCHEMA if an incompatible configuration is
>> detected.  But we already have such checks in several cases, so I don't
>> see why this one does not seem a good idea.
>>
>> The whole FOR ALL TABLES IN SCHEMA thing seems pretty weird in several
>> aspects.  Others have already commented about the syntax, which is
>> unlike what GRANT uses; I'm also surprised that we've gotten away with
>> it being superuser-only.  Why are we building more superuser-only
>> features in this day and age?  I think not even FOR ALL TABLES should
>> require superuser.
>
> FYI, I've added this to the PG15 open items as there are some open
> questions to resolve in this thread.

(Replying personally, not RMT).

I wanted to enumerate the concerns raised in this thread in the context
of the open item to understand what needs to be addressed, and also give
an opinion. I did read up on the original thread to better understand
context around decisions.

I believe the concerns are these 3 things:

1. Allowing calls that have "ALL TABLES IN SCHEMA" that include calls to
specific tables in schema
2. The syntax of the "ALL TABLES IN SCHEMA" and comparing it to similar
behaviors in PostgreSQL
3. Adding on an additional "superuser-only" feature

For #1 (allowing calls that have schema/table overlap...), there appears
to be both a patch that allows this (reversing[8]), and a suggestion for
dealing with a corner-case that is reasonable, i.e. disallowing adding
schemas to a publication when specifying column-lists. Do we think we
can have consensus on this prior to the RC1 freeze?

For #2 ("ALL TABLES IN SCHEMA" syntax), this was heavily discussed on
the original thread[1][3][4][5][7]. I thought Tom's proposal on the
syntax[3] was reasonable as it "future proofs" for when we allow other
schema-scoped objects to be published and give control over which ones
can be published.

The bigger issue seems to be around the behavior in regards to the
syntax. The current behavior is that when one specifies "ALL TABLES IN
SCHEMA", any future tables created in that schema are added to the
publication. While folks tried to find parallels to GRANT[6], I think
this actually resembles how we handle partitions that are
published[9][10], i.e.:

"When a partitioned table is added to a publication, all of its existing
and future partitions are implicitly considered to be part of the
publication."[10]

Additionally, this is the behavior that is already present in "FOR ALL
TABLES":

"Marks the publication as one that replicates changes for all tables in
the database, including tables created in the future."[10]

I don't think we should change this behavior that's already in logical
replication. While I understand the reasons why "GRANT ... ALL TABLES IN
SCHEMA" has a different behavior (i.e. it's not applied to future
objects) and do not advocate to change it, I have personally been
affected where I thought a permission would be applied to all future
objects, only to discover otherwise. I believe it's more intuitive to
think that "ALL" applies to "everything, always."

For #3 (more superuser-only), in general I do agree that we shouldn't be
adding more of these. However, we have in this release, and not just to
this feature. ALTER SUBSCRIPTION ... SKIP[11] requires superuser. I
think it's easier for us to "relax" privileges (e.g. w/predefined roles)
than to make something "superuser-only" in the future, so I don't see
this being a blocker for v15. The feature will continue to work for
users even if we remove "superuser-only" in the future.

To summarize:

1. I do think we should fix the issue that Peter originally brought up
in this thread before v15. That is an open item.
2. I don't see why we need to change the syntax/behavior, I think that
will make this feature much harder to use.
3. I don't think we need to change the superuser requirement right now,
but we should do that for a future release.

Thanks,

Jonathan

[1]
https://www.postgresql.org/message-id/CAFiTN-u_m0cq7Rm5Bcu9EW4gSHG94WaLuxLfibwE-o7%2BLea2GQ%40mail.gmail.com
[2]
https://www.postgresql.org/message-id/C4D04B90-AC4D-42A7-B93C-4799CEDDDD96%40enterprisedb.com
[3] https://www.postgresql.org/message-id/155565.1628954580%40sss.pgh.pa.us
[4]
https://www.postgresql.org/message-id/CAHut%2BPvNwzp-EdtsDNazwrNrV4ziqCovNdLywzOJKSy52LvRjw%40mail.gmail.com
[5]
https://www.postgresql.org/message-id/CAHut%2BPt6Czj0KsE0ip6nMsPf4FatHgNDni-wSu2KkYNYF9mDAw%40mail.gmail.com
[6]
https://www.postgresql.org/message-id/CAA4eK1Lwtea0St1MV5nfSg9FrFeU04YKpHvhQ0i4W-tOBw%3D9Qw%40mail.gmail.com
[7]
https://www.postgresql.org/message-id/202109241325.eag5g6mpvoup@alvherre.pgsql
[8]
https://www.postgresql.org/message-id/CALDaNm1BEXtvg%3Dfq8FzM-FoYvETTEuvA_Gf8rCAjFr1VrB5aBA%40mail.gmail.com
[9]
https://www.postgresql.org/message-id/CAJcOf-fyM3075t9%2B%3DB-BSFz2FG%3D5BnDSPX4YtL8k1nnK%3DwjgWA%40mail.gmail.com
[10] https://www.postgresql.org/docs/current/sql-createpublication.html
[11] https://www.postgresql.org/docs/15/sql-altersubscription.html