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. 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.
