From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | Peter Smith <smithpb2250(at)gmail(dot)com> |
Cc: | Shubham Khanna <khannashubham1197(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Improve the error message for logical replication of regular column to generated column. |
Date: | 2024-11-15 03:06:52 |
Message-ID: | CAA4eK1+HBEpUNH2gq=UsezPfmN2q+5RTZw4RPXgrZQsOXWgdRg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Nov 15, 2024 at 6:10 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> 3. A different approach?
>
> TBH, is introducing a whole new error message even a good idea?
>
> Now there are going to be two separate error messages where previously
> there was only one. So if the table has multiple problems at the same
> time then still only one of them can "win". i.e. you have to either
> report the "generated columns" problem 1st or the "missing columns"
> problem 1st -- either way that might not be a good user experience
> because they might be unaware of multiple problems until they try the
> CREATE SUBSCRIPTION a 2nd time and then it fails a 2nd time with the
> other kind of error! That could be annoying.
>
I don't know why the user needs to perform CREATE SUBSCRIPTION
multiple times to see this. IIUC, this error will happen in the apply
worker and after fixing the first, the user should see the second. I
think this can happen in other ways in apply worker as well.
> 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.
--
With Regards,
Amit Kapila.
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Smith | 2024-11-15 03:06:54 | Re: Skip collecting decoded changes of already-aborted transactions |
Previous Message | Pavan Deolasee | 2024-11-15 03:05:48 | Re: Potential ABI breakage in upcoming minor releases |