Re: Skipping schema changes in publication

On Tue, Mar 17, 2026 at 4:31 PM shveta malik <shveta.malik@gmail.com> wrote:
>
> 2)
> +bool
> +is_include_relation_publication(Oid pubid)
>
> The name 'is_include_relation_publication' looks slightly odd to me.
> Few options are: is_explicit_table_publication,
> is_table_list_publication, is_table_publication. Or anything better if
> you can think of?
>

+1 for is_table_publication() as it matches the existing function
is_schema_publication().

Few other comments:
=================
*
-   The <literal>ADD TABLES IN SCHEMA</literal> and
-   <literal>SET TABLES IN SCHEMA</literal> to a publication requires the
+   The <literal>ADD TABLES IN SCHEMA</literal>,
+   <literal>SET TABLES IN SCHEMA</literal>, and
+   <literal>SET ALL TABLES</literal> to a publication requires the
    invoking user to be a superuser.

Do we need to mention SET ALL SEQUENCES variant as well?

*
AlterPublicationTables(stmt, tup, relations, pstate->p_sourcetext,
     schemaidlist != NIL);
  AlterPublicationSchemas(stmt, tup, schemaidlist);
+
+ if (stmt->for_all_tables || stmt->for_all_sequences)
+ {
+ bool nulls[Natts_pg_publication];
+ bool replaces[Natts_pg_publication];
+ Datum values[Natts_pg_publication];
+ bool dirty = false;
+
+ memset(values, 0, sizeof(values));
+ memset(nulls, false, sizeof(nulls));
+ memset(replaces, false, sizeof(replaces));
+
+ if (stmt->for_all_tables != pubform->puballtables)
+ {
+ values[Anum_pg_publication_puballtables - 1] =
+ BoolGetDatum(stmt->for_all_tables);
+ replaces[Anum_pg_publication_puballtables - 1] = true;
+ dirty = true;
+ }
+
+ if (stmt->for_all_sequences != pubform->puballsequences)
+ {
+ values[Anum_pg_publication_puballsequences - 1] =
+ BoolGetDatum(stmt->for_all_sequences);
+ replaces[Anum_pg_publication_puballsequences - 1] = true;
+ dirty = true;
+ }
+
+ if (dirty)
+ {
+ tup = heap_modify_tuple(tup, RelationGetDescr(rel), values,
+ nulls, replaces);
+ CatalogTupleUpdate(rel, &tup->t_self, tup);
+ CommandCounterIncrement();
+
+ /* For ALL TABLES, we must invalidate all relcache entries */
+ if (replaces[Anum_pg_publication_puballtables - 1])
+ CacheInvalidateRelcacheAll();
+ }
+ }

Can we move this new code to a separate function, say
AlterPublicationAllFlags()?

-- 
With Regards,
Amit Kapila.