Hi Zsolt, Thanks for reviewing the patch. On Thu, 28 May 2026 at 03:22, Zsolt Parragi <[email protected]> wrote: > > Hello! > > + if (footers[0] == NULL) > + footers[0] = > pg_strdup(tmpbuf.data); > + else if (footers[1] == NULL) > + footers[1] = > pg_strdup(tmpbuf.data); > + else > + footers[2] = > pg_strdup(tmpbuf.data); > + resetPQExpBuffer(&tmpbuf); > > Shouldn't the matching free calls also updated, this now leaks footers[2]? > Yes, correct. Made the change for the same.
> + "\n AND NOT EXISTS
> (\n"
> + " SELECT 1\n"
> + " FROM
> pg_catalog.pg_publication_rel pr\n"
> + " WHERE
> pr.prpubid = p.oid AND\n"
> + " pr.prrelid =
> '%s')"
>
> Isn't a pr.prexcept check missing from this? It is included in other queries.
>
pg_publication_rel has an entry for table/sequence if we specifically
include/exclude it in a publication.
So, we can say, pg_publication_rel contains an entry for a
Table/Sequence for a ALL TABLES/ ALL SEQUENCE publication, only if it
is specified in the EXCEPT clause.
So, the above query will give the expected results even if we donot
use the ' pr.prexcept' flag.
For the case: _("Get publications that publish this table"))
Here as well we use a similar query without prexcept:
"WHERE p.puballtables AND pg_catalog.pg_relation_is_publishable('%s')\n"
" AND NOT EXISTS (\n"
" SELECT 1\n"
" FROM
pg_catalog.pg_publication_rel pr\n"
" WHERE pr.prpubid = p.oid AND\n"
" (pr.prrelid = '%s' OR
pr.prrelid = pg_catalog.pg_partition_root('%s')))\n"
"ORDER BY 1;",
> - /* EXCEPT clause is not supported with ALTER PUBLICATION */
> - Assert(exceptseqs == NIL);
> -
>
> Would it be worth it to keep a more restricted assert, saying that
> EXCEPT clause is only supported for ALTER PUBLICATION ... SET?
>
>
Added the Assert in 0002 patch.
I have addressed the comments and attached the v8 patch.
Thanks,
Shlok Kyal
v8-0001-Support-EXCEPT-for-ALL-SEQUENCES-in-CREATE-PUBLIC.patch
Description: Binary data
v8-0002-Support-EXCEPT-for-ALL-SEQUENCES-in-ALTER-PUBLICA.patch
Description: Binary data
