On Wed, Apr 8, 2026 at 3:25 PM shveta malik <[email protected]> wrote: > > On Wed, Apr 8, 2026 at 10:24 AM Peter Smith <[email protected]> wrote: > > > > On Wed, Apr 8, 2026 at 1:45 PM vignesh C <[email protected]> wrote: > > > > > > On Tue, 7 Apr 2026 at 12:32, Peter Smith <[email protected]> wrote: > > > > > > > > Hi, after confirming my understanding of pg_publication_rel [1], I > > > > revisited some logical replication internal functions. > > > > > > > > Specifically. > > > > * The `is_table_publication` function is for checking if the > > > > publication has a clause like "FOR TABLE t1". > > > > * The `is_schema_publication` function is for checking if the > > > > publication has a clause like "FOR TABLES IN SCHEMA s1". > > > > > > > > Notice that neither of these ("FOR TABLE", "FOR TABLES IN SCHEMA") > > > > clauses are possible simultaneously with "FOR ALL TABLES". > > > > > > > > And we can readily discover if "FOR ALL TABLES" (aka `puballtables`) > > > > is present from the pubform. > > > > > > > > We can use this to optimise and simplify the implementations of the > > > > `is_schema_publication` and `is_table_publication` functions. > > > > > > > > PSA patch v1. > > > > > > > > AFAICT, the result is: > > > > - less code + simpler logic. e.g. is_table_publication does not check > > > > 'prexcept' anymore > > > > - more efficient. e.g. skips unnecessary scanning when puballtables is > > > > true. > > > > - more consistent. e.g., both functions are now almost identical. > > > > > > > > Thoughts? > > > > > > > Hi Vignesh. Thanks for reviewing! > > > > > I'm not sure if this additional check is sufficient in case of > > > is_schema_publication. Checking only puballtables can exclude FOR ALL > > > TABLES, but it still cannot distinguish regular table publications, > > > empty publications, or sequence publications. In all of those cases, > > > we still need to check pg_publication_namespace. > > > > Yes, this condition is only an optimisation for FOR ALL TABLES, as the > > comment says. > > > > IMO, the overhead of 1 additional boolean check for cases where it > > doesn't help is an insignificant trade-off for the savings when it can > > return false. > > > > > And also why just check for puballtables why not to check for > > > puballsequences > > > > I think function is_schema_publication() is unrelated to 'puballsequences'. > > > > e.g. all the following will still need to check > > pg_publication_namespace, regardless of the 'puballsequences' value. > > > > ex1. CREATE PUBLICATION ... FOR ALL SEQUENCES; > > ex2. CREATE PUBLICATION ... FOR ALL SEQUENCES, FOR TABLES IN SCHEMA s1; > > ex3. CREATE PUBLICATION ... FOR TABLES IN SCHEMA s1; > > > > IIUC, we don't support mix of ALL SEQUENCES and TABLES IN SCHEMA s1. > So I could not understand your point, why FOR ALL SEQ still need to > check pg_publication_namespace? >
Oh! You are right. (Sorry, Vignesh, I did not recognise that combination as unsupported). I'll post a patch update to handle it. ====== Kind Regards, Peter Smith. Fujitsu Australia
