| From: | Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> |
|---|---|
| To: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> |
| Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: Support EXCEPT for ALL SEQUENCES publications |
| Date: | 2026-05-29 11:59:11 |
| Message-ID: | CANhcyEVCJOZTH9GL2wCOA+ea5uM_yqemUu2vAG-chNuA29HuJA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Zsolt,
Thanks for reviewing the patch.
On Thu, 28 May 2026 at 03:22, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> 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
| Attachment | Content-Type | Size |
|---|---|---|
| v8-0001-Support-EXCEPT-for-ALL-SEQUENCES-in-CREATE-PUBLIC.patch | application/octet-stream | 58.9 KB |
| v8-0002-Support-EXCEPT-for-ALL-SEQUENCES-in-ALTER-PUBLICA.patch | application/octet-stream | 26.9 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Matthias van de Meent | 2026-05-29 12:04:00 | Re: Key joins |
| Previous Message | Hannu Krosing | 2026-05-29 11:44:26 | some utf8 breaking substring(txt,1,3) but not substring(txt from '^.{4}') |