On Wed, Sep 23, 2026 at 10:08 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, Sep 23, 2026 at 5:12 AM Manu <manuelreyesbravo@gmail.com> 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.
