Re: Improve the error message for logical replication of regular column to generated column.

On Fri, Nov 15, 2024 at 9:06 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> On Fri, Nov 15, 2024 at 2:07 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > > A better solution may be just to *combine* everything, so the user
> > > only has to deal with one error. IIUC that's what is already happening
> > > in master code, so this patch doesn't need to do anything except make
> > > a quite trivial change to the wording of the existing error message.
> > >
> > > For example:
> > > BEFORE
> > > errmsg_plural("logical replication target relation \"%s.%s\" is
> > > missing replicated column: %s",
> > > "logical replication target relation \"%s.%s\" is
> > > missing replicated columns: %s",
> > > SUGGESTION
> > > errmsg_plural("logical replication target relation \"%s.%s\" has
> > > missing or generated replicated column: %s",
> > > "logical replication target relation \"%s.%s\" has
> > > missing or generated replicated columns: %s",
> > >
> >
> > With this, we can combine two different ERRORs into one but it won't
> > be evident if the column name referred in the message is generated or
> > missing. I see your point but combining two different errors into one
> > is also confusing. We can try to add more checks to make this
> > distinction clear but it doesn't seem worth the effort and complexity.
> > Also, it is not clear whether combining different ERRORs is a good
> > idea in the first place.
> >
>
> I don't know if it needs to be spelled out explicitly in the message
> which is which because the user will surely know their own subscriber
> table definition, so it will be quite obvious to them if a named
> column is missing or generated.
>

The separate messages in this case would be clearer and better.

--
With Regards,
Amit Kapila.