| From: | Manu <manuelreyesbravo(at)gmail(dot)com> |
|---|---|
| To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
| Cc: | vignesh C <vignesh21(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: Distinguish publication exclusions in object addresses |
| Date: | 2026-09-22 23:42:25 |
| Message-ID: | 179012054503.688004.12375093780155765376@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Correcting myself before you spend time on this: I wrote
> Of those six, the regression tests currently reach one.
and that is wrong. I had inferred it from grepping the expected output
for the EXCEPT message, without measuring the code. I have now built
with --enable-coverage and run make check. The ereport() call sites in
check_publication_add_relation() come out like this (gcov execution
counts, master):
incomplete detach never executed
individual partitions 2
relkind not supported 2
system tables 1
conflict log tables 1
temporary tables 1
unlogged tables 1
So five of the six already execute. They are reached through the
non-EXCEPT path: the same ereport() runs with the other errormsg, from
the existing "cannot add relation ... to publication" cases. Only the
incomplete-detach one is never reached at all.
What my patch actually changes, same measurement with it applied:
relkind not supported 2 -> 3
system tables 1 -> 2
temporary tables 1 -> 2
so it adds three executions, but not three uncovered lines.
That weakens the case I made. The honest version is narrower: line
coverage of those sites does not change, what is untested is the
EXCEPT variant of each of them - the message whose quoting vignesh
just fixed, and the schema-qualified name it prints. Today exactly one
of those six paths has an expected output containing "in the
publication EXCEPT clause", the partitions one.
Whether that is worth three test cases is your call, and I would
understand if the answer is no now that the coverage argument is off
the table. If you do want something from this, the incomplete-detach
site is the one genuinely never executed, though reaching it needs an
interrupted concurrent detach and I have not tried to build that.
Sorry for the noise.
Regards,
Manu
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Manu | 2026-09-23 00:00:41 | Re: Add a test for index_rebuild_count of REPACK (CONCURRENTLY) |
| Previous Message | Masahiko Sawada | 2026-09-22 23:35:19 | Re: [PATCH] Release replication slot on error in SQL-callable slot functions |