Re: Have better wording for snapshot file reading failure

2023-09-17 Thread Yogesh Sharma
On 9/14/23 20:33, Andres Freund wrote: I'd probably just go for something like "snapshot \"%s\" does not exist", similar to what we report for unknown tables etc. Arguably changing the errcode to ERRCODE_UNDEFINED_OBJECT would make this more precise? +1  better  informative message compare to t

Re: Have better wording for snapshot file reading failure

2023-09-14 Thread Michael Paquier
On Thu, Sep 14, 2023 at 05:33:35PM -0700, Andres Freund wrote: > I'd probably just go for something like "snapshot \"%s\" does not exist", > similar to what we report for unknown tables etc. Arguably changing the > errcode to ERRCODE_UNDEFINED_OBJECT would make this more precise? Good points. Upd

Re: Have better wording for snapshot file reading failure

2023-09-14 Thread Andres Freund
Hi, On 2023-09-14 16:29:22 +0900, Michael Paquier wrote: > On Thu, Sep 14, 2023 at 01:33:39PM +0900, Michael Paquier wrote: > > Ahem. This seems to be the only code path that tracks a failure on > > AllocateFile() where we don't show %m at all, while the error is > > misleading in basically all t

Re: Have better wording for snapshot file reading failure

2023-09-14 Thread Michael Paquier
On Thu, Sep 14, 2023 at 01:33:39PM +0900, Michael Paquier wrote: > Ahem. This seems to be the only code path that tracks a failure on > AllocateFile() where we don't show %m at all, while the error is > misleading in basically all the cases as errno holds the extra > information telling somebody t

Re: Have better wording for snapshot file reading failure

2023-09-13 Thread Michael Paquier
On Wed, Sep 13, 2023 at 07:09:32PM -0700, Andres Freund wrote: > On 2023-09-13 19:07:24 -0700, Andres Freund wrote: >> Huh. I don't think this is a good idea - and certainly not in the back >> branches. The prior message made more sense, imo. The fact that the snapshot >> identifier is a file is an

Re: Have better wording for snapshot file reading failure

2023-09-13 Thread Andres Freund
Hi, On 2023-09-13 19:07:24 -0700, Andres Freund wrote: > On 2023-09-14 10:33:33 +0900, Michael Paquier wrote: > > On Wed, Sep 13, 2023 at 01:19:38PM +0200, Daniel Gustafsson wrote: > > > +1. This errmsg is already present so it eases the translation burden as > > > well. > > > > I was thinking a

Re: Have better wording for snapshot file reading failure

2023-09-13 Thread Andres Freund
Hi, On 2023-09-14 10:33:33 +0900, Michael Paquier wrote: > On Wed, Sep 13, 2023 at 01:19:38PM +0200, Daniel Gustafsson wrote: > > +1. This errmsg is already present so it eases the translation burden as > > well. > > I was thinking about doing only that on HEAD, but there is an argument > that o

Re: Have better wording for snapshot file reading failure

2023-09-13 Thread Michael Paquier
On Wed, Sep 13, 2023 at 01:19:38PM +0200, Daniel Gustafsson wrote: > +1. This errmsg is already present so it eases the translation burden as well. I was thinking about doing only that on HEAD, but there is an argument that one could get confusing errors when dealing with snapshot imports on back-

Re: Have better wording for snapshot file reading failure

2023-09-13 Thread Daniel Gustafsson
> On 13 Sep 2023, at 08:18, Michael Paquier wrote: > f = AllocateFile(path, PG_BINARY_R); > if (!f) > ereport(ERROR, > -(errcode(ERRCODE_INVALID_PARAMETER_VALUE), > - errmsg("invalid snapshot identifier: \"%s\"", idstr))); > +(errcod

Re: Have better wording for snapshot file reading failure

2023-09-13 Thread Bharath Rupireddy
On Wed, Sep 13, 2023 at 3:32 PM Yogesh Sharma wrote: > > On 9/13/23 02:10, Bharath Rupireddy wrote: > > Hi, > > > > When a snapshot file reading fails in ImportSnapshot(), it errors out > > with "invalid snapshot identifier". This message better suits for > > snapshot identifier parsing errors whi

Re: Have better wording for snapshot file reading failure

2023-09-13 Thread Yogesh Sharma
On 9/13/23 02:10, Bharath Rupireddy wrote: Hi, When a snapshot file reading fails in ImportSnapshot(), it errors out with "invalid snapshot identifier". This message better suits for snapshot identifier parsing errors which is being done just before the file reading. The attached patch adds a ge

Re: Have better wording for snapshot file reading failure

2023-09-12 Thread Michael Paquier
On Wed, Sep 13, 2023 at 11:40:25AM +0530, Bharath Rupireddy wrote: > When a snapshot file reading fails in ImportSnapshot(), it errors out > with "invalid snapshot identifier". This message better suits for > snapshot identifier parsing errors which is being done just before the > file reading. The

Have better wording for snapshot file reading failure

2023-09-12 Thread Bharath Rupireddy
Hi, When a snapshot file reading fails in ImportSnapshot(), it errors out with "invalid snapshot identifier". This message better suits for snapshot identifier parsing errors which is being done just before the file reading. The attached patch adds a generic file reading error message with path to