Re: why can't a table be part of the same publication as its schema - Mailing list pgsql-hackers

On 9/19/22 11:16 AM, Alvaro Herrera wrote:
> 
>> diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
>> index 1ae3287..0ab768d 100644
>> --- a/doc/src/sgml/logical-replication.sgml
>> +++ b/doc/src/sgml/logical-replication.sgml
>> @@ -1120,6 +1120,11 @@ test_sub=# SELECT * FROM child ORDER BY a;
>>     </para>
>>   
>>     <para>
>> +   Specifying a column list when the publication also publishes
>> +   <literal>FOR ALL TABLES IN SCHEMA</literal> is not supported.
>> +  </para>
>> +
>> +  <para>
>>      For partitioned tables, the publication parameter
>>      <literal>publish_via_partition_root</literal> determines which column list
>>      is used. If <literal>publish_via_partition_root</literal> is
>>
>> diff --git a/doc/src/sgml/ref/create_publication.sgml b/doc/src/sgml/ref/create_publication.sgml
>> index 0a68c4b..0ced7da 100644
>> --- a/doc/src/sgml/ref/create_publication.sgml
>> +++ b/doc/src/sgml/ref/create_publication.sgml
>> @@ -103,17 +103,17 @@ CREATE PUBLICATION <replaceable class="parameter">name</replaceable>
>>        </para>
>>   
>>        <para>
>> +      Specifying a column list when the publication also publishes
>> +      <literal>FOR ALL TABLES IN SCHEMA</literal> is not supported.
>> +     </para>
>>
>> @@ -733,6 +694,24 @@ CheckPubRelationColumnList(List *tables, const char *queryString,
>>               continue;
>>   
>>           /*
>> +         * Disallow specifying column list if any schema is in the
>> +         * publication.
>> +         *
>> +         * XXX We could instead just forbid the case when the publication
>> +         * tries to publish the table with a column list and a schema for that
>> +         * table. However, if we do that then we need a restriction during
>> +         * ALTER TABLE ... SET SCHEMA to prevent such a case which doesn't
>> +         * seem to be a good idea.
>> +         */
>> +        if (publish_schema)
>> +            ereport(ERROR,
>> +                    errcode(ERRCODE_INVALID_PARAMETER_VALUE),
>> +                    errmsg("cannot use publication column list for relation \"%s.%s\"",
>> +                           get_namespace_name(RelationGetNamespace(pri->relation)),
>> +                           RelationGetRelationName(pri->relation)),
>> +                    errdetail("Column list cannot be specified if any schema is part of the publication or
specifiedin the list."));
 
>> +
> 
> 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.

Jonathan