On Wed, Sep 23, 2026 at 10:08 AM Amit Kapila <[email protected]> wrote: > > On Wed, Sep 23, 2026 at 5:12 AM Manu <[email protected]> wrote: > > > > 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. > > > > I am not against adding the tests for code coverage in general but for > negative tests we need to be careful to not add too many. I am content > with the current coverage of the negative cases but if you still want > to pursue, I suggest making a case in a separate email with a > HEAD-only patch.
+1. > If we see that others see a value in the proposal > then we can consider it. > > > Sorry for the noise. > > > > No issues, thanks for the detailed analysis. > > -- > With Regards, > Amit Kapila.
