From: | vignesh C <vignesh21(at)gmail(dot)com> |
---|---|
To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
Cc: | Peter Smith <smithpb2250(at)gmail(dot)com>, 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-27 07:15:15 |
Message-ID: | CALDaNm3TywSSMuLJiZWxZHEvUm9fWVgniVSojh369pigP5Fs3w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, 27 Nov 2024 at 12:15, vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Wed, 27 Nov 2024 at 08:50, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Wed, Nov 27, 2024 at 3:30 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> > >
> > > Hi, here are some review comments for patch v7-0001.
> > >
> > > ======
> > > src/backend/replication/logical/relation.c
> > >
> > > logicalrep_report_missing_or_gen_attrs:
> > >
> > > 1.
> > > +/*
> > > + * If attempting to replicate missing or generated columns, report an error.
> > > + * Prioritize 'missing' errors if both occur though the prioritization is
> > > + * random.
> > > + */
> > >
> > > That part "though the prioritization is random" seems wrongly worded
> > > because there is nothing random here.
> > >
> > > I guess the intention was something like "This prioritization design
> > > choice was arbitrary.", but TBH it may be better not to give any
> > > reason at all.
> > >
> >
> > I think we should give a reason so that if we come across any scenario
> > or add another one in the future, it will be easier to make the
> > decision. I'll change the patch to use 'arbitrary' instead of random.
>
> There is a buildfarm failure in [1]. One of the new tests added to
> verify the log for the "incompatible generated columns" issue was
> incorrect. Specifically, the check qr/ERROR: ( [A-Z0-9]:) should have
> been updated to qr/ERROR: ( [A-Z0-9]+:), which is consistent with
> similar checks elsewhere in the codebase. The attached patch contains
> the necessary changes to address this issue.
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=prion&dt=2024-11-27%2004%3A17%3A03
The issue occurs specifically on the prion machine, which is
configured with log_error_verbosity = verbose, causing error messages
to include the sqlerrcode alongside the error description, as shown
below from [1]:
2024-11-27 05:41:13.966 UTC [2990900:3] ERROR: 55000: logical
replication target relation "public.t1" has incompatible generated
columns: "c2", "c3"
In contrast, other buildfarm machines do not include the sqlerrcode in
the error messages, as seen here from [2]:
2024-11-27 07:19:45.975 CET [38683:2] ERROR: logical replication
target relation "public.t1" has incompatible generated columns: "c2",
"c3"
The problem arises only when the sqlerrcode is present, as the error
code matching was not correct. I have confirmed that the patch
referenced in [3] resolves the issue when log_error_verbosity =
verbose is enabled.
[1]: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=prion&dt=2024-11-27%2004%3A17%3A03
[2]: https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=loach&dt=2024-11-27%2006%3A07%3A55&stg=subscription-check
[3]: https://www.postgresql.org/message-id/CALDaNm0C5LPiTxkdqsxiyeaL%3DnuUP8t6ne81sp9jE0%3DMFz%3D-ew%40mail.gmail.com
Regards,
Vignesh
From | Date | Subject | |
---|---|---|---|
Next Message | Sergey Dudoladov | 2024-11-27 07:28:25 | Re: Add connection active, idle time to pg_stat_activity |
Previous Message | John Naylor | 2024-11-27 07:02:23 | Re: ECPG cleanup and fix for clang compile-time problem |