Re: A doubt about a newly added errdetail

2022-09-28 Thread Kyotaro Horiguchi
At Wed, 28 Sep 2022 10:46:41 +0200, Alvaro Herrera wrote in > On 2022-Sep-28, Amit Kapila wrote: > > > On Wed, Sep 28, 2022 at 11:30 AM Kyotaro Horiguchi > > wrote: > > > > It looks tome that the errmsg and errordetail are reversed. Isn't the > > > following order common? > > > > > > >

Re: A doubt about a newly added errdetail

2022-09-28 Thread Kyotaro Horiguchi
At Wed, 28 Sep 2022 13:47:25 +0530, Amit Kapila wrote in > On Wed, Sep 28, 2022 at 11:30 AM Kyotaro Horiguchi > wrote: > > I'm fine with that. By the way, related to the area, I found the > > following error messages. > > > > >errmsg("publication \"%s\" is defined as FOR ALL TABLES", >

Re: A doubt about a newly added errdetail

2022-09-28 Thread Alvaro Herrera
On 2022-Sep-28, Amit Kapila wrote: > On Wed, Sep 28, 2022 at 11:30 AM Kyotaro Horiguchi > wrote: > > It looks tome that the errmsg and errordetail are reversed. Isn't the > > following order common? > > > > >errmsg("schemas cannot be added to or dropped from publication > > > \"%s\".",

Re: A doubt about a newly added errdetail

2022-09-28 Thread Amit Kapila
On Wed, Sep 28, 2022 at 11:30 AM Kyotaro Horiguchi wrote: > > At Tue, 27 Sep 2022 12:19:35 +0200, Alvaro Herrera > wrote in > > Yeah, since you're changing another word in that line, it's ok to move > > the parameter line off-string. (If you were only changing the parameter > > to %s and there

Re: A doubt about a newly added errdetail

2022-09-27 Thread Kyotaro Horiguchi
At Tue, 27 Sep 2022 12:19:35 +0200, Alvaro Herrera wrote in > Yeah, since you're changing another word in that line, it's ok to move > the parameter line off-string. (If you were only changing the parameter > to %s and there was no message duplication, I would reject the patch as > useless.) I

Re: A doubt about a newly added errdetail

2022-09-27 Thread Amit Kapila
On Tue, Sep 27, 2022 at 6:12 PM Alvaro Herrera wrote: > > While reading this code, I noticed that function expr_allowed_in_node() > has a very strange API: it doesn't have any return convention at all > other than "if we didn't modify errdetail_str then all is good". I was > tempted to add an "As

Re: A doubt about a newly added errdetail

2022-09-27 Thread Alvaro Herrera
While reading this code, I noticed that function expr_allowed_in_node() has a very strange API: it doesn't have any return convention at all other than "if we didn't modify errdetail_str then all is good". I was tempted to add an "Assert(*errdetail_msg == NULL)" at the start of it, just to make su

Re: A doubt about a newly added errdetail

2022-09-27 Thread Alvaro Herrera
On 2022-Sep-27, houzj.f...@fujitsu.com wrote: > Thanks for reviewing! > > Just in case I misunderstand, it seems you mean the message style[1] is OK, > right ? > > [1] > errdetail("Column list cannot be specified for a partitioned table when %s is > false.", >"publish_via_partition_roo

RE: A doubt about a newly added errdetail

2022-09-27 Thread houzj.f...@fujitsu.com
On Tuesday, September 27, 2022 3:21 PM Alvaro Herrera wrote: > > On 2022-Sep-27, Kyotaro Horiguchi wrote: > > > By the way, this is not an issue caused by the proposed patch, I see > > the following message in the patch. > > > > -errdetail("Column list cannot

Re: A doubt about a newly added errdetail

2022-09-27 Thread Alvaro Herrera
On 2022-Sep-27, Kyotaro Horiguchi wrote: > By the way, this is not an issue caused by the proposed patch, I see > the following message in the patch. > > - errdetail("Column list cannot be used > for a partitioned table when %s is false.", > +

Re: A doubt about a newly added errdetail

2022-09-26 Thread Kyotaro Horiguchi
At Mon, 26 Sep 2022 17:33:46 +0530, Amit Kapila wrote in > On Mon, Sep 26, 2022 at 4:45 PM houzj.f...@fujitsu.com > wrote: > > > > > > Attach the patch. (The patch can apply on both HEAD and PG15) > > > > The patch looks good to me. > > * > - errmsg("cannot add schema to the publication"), >

Re: A doubt about a newly added errdetail

2022-09-26 Thread Amit Kapila
On Mon, Sep 26, 2022 at 4:45 PM houzj.f...@fujitsu.com wrote: > > > Attach the patch. (The patch can apply on both HEAD and PG15) > The patch looks good to me. * - errmsg("cannot add schema to the publication"), + errmsg("cannot add schema to publication \"%s\"", +stmt->pubname), I see that

RE: A doubt about a newly added errdetail

2022-09-26 Thread houzj.f...@fujitsu.com
On Monday, September 26, 2022 5:03 PM houzj.f...@fujitsu.com wrote: > > On Monday, September 26, 2022 4:57 PM Amit Kapila > > > > > On Mon, Sep 26, 2022 at 2:03 PM Alvaro Herrera > > > > wrote: > > > > > > On 2022-Sep-26, Amit Kapila wrote: > > > > > > > On Mon, Sep 26, 2022 at 1:10 PM Alvaro He

RE: A doubt about a newly added errdetail

2022-09-26 Thread houzj.f...@fujitsu.com
On Monday, September 26, 2022 4:57 PM Amit Kapila > > On Mon, Sep 26, 2022 at 2:03 PM Alvaro Herrera > wrote: > > > > On 2022-Sep-26, Amit Kapila wrote: > > > > > On Mon, Sep 26, 2022 at 1:10 PM Alvaro Herrera > wrote: > > > > > > ERROR: cannot use column list for relation "%s.%s" in publicati

Re: A doubt about a newly added errdetail

2022-09-26 Thread Amit Kapila
On Mon, Sep 26, 2022 at 2:03 PM Alvaro Herrera wrote: > > On 2022-Sep-26, Amit Kapila wrote: > > > On Mon, Sep 26, 2022 at 1:10 PM Alvaro Herrera > > wrote: > > > > ERROR: cannot use column list for relation "%s.%s" in publication "%s" > > > DETAIL: Column lists cannot be specified in publicat

Re: A doubt about a newly added errdetail

2022-09-26 Thread Alvaro Herrera
On 2022-Sep-26, Amit Kapila wrote: > On Mon, Sep 26, 2022 at 1:10 PM Alvaro Herrera > wrote: > > ERROR: cannot use column list for relation "%s.%s" in publication "%s" > > DETAIL: Column lists cannot be specified in publications containing FOR > > TABLES IN SCHEMA elements. > > This looks m

Re: A doubt about a newly added errdetail

2022-09-26 Thread Amit Kapila
On Mon, Sep 26, 2022 at 1:10 PM Alvaro Herrera wrote: > > On 2022-Sep-26, Kyotaro Horiguchi wrote: > > > I saw the following message recently added to publicationcmds.c. > > > > (ERROR: cannot use publication column list for relation "%s.%s"") > > > DETAIL: Column list cannot be specified if any s

Re: A doubt about a newly added errdetail

2022-09-26 Thread Alvaro Herrera
On 2022-Sep-26, Kyotaro Horiguchi wrote: > I saw the following message recently added to publicationcmds.c. > > (ERROR: cannot use publication column list for relation "%s.%s"") > > DETAIL: Column list cannot be specified if any schema is part of the > > publication or specified in the list. >