Re: Error in pg_restore (could not close data file: Success) - Mailing list pgsql-hackers

From Andrii Tkach
Subject Re: Error in pg_restore (could not close data file: Success)
Date
Msg-id CAN2Q-+inoXsbGKx9LomeSWCTr09N+Hz4M8V1-3XCXVempHH9Og@mail.gmail.com
Whole thread Raw
In response to Re: Error in pg_restore (could not close data file: Success)  (Kyotaro Horiguchi <horikyota.ntt@gmail.com>)
List pgsql-hackers
Maybe it would be better to commit this patches to mainstream, but I don't realy know.

ср, 21 окт. 2020 г. в 09:20, Kyotaro Horiguchi <horikyota.ntt@gmail.com>:
At Wed, 21 Oct 2020 13:45:15 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in
> https://www.postgresql.org/message-id/flat/20200416.181945.759179589924840062.horikyota.ntt%40gmail.com#ed85c5fda64873c45811be4e3027a2ea
>
> Me> Hmm. Sounds reasonable.  I'm going to do that.  Thanks!
>
> But somehow that haven't happened, I'll come up with a new version.

pg_restore shows the following error instead of "Success" for broken
compressed file.

pg_restore: error: could not close data file "d/3149.dat": zlib error: error reading or writing compressed file


0001:

cfclose() calls fatal() instead of returning the result to the callers
on error, which isobviously okay for all existing callers that are
handling errors from the function. Other callers ignored the returned
value but we should fatal() on error of the function.

At least for me, gzerror doesn't return a message (specifically,
returns NULL) after gzclose failure so currently cfclose shows its own
messages for erros of gzclose().  Am I missing something?

0002:

cfread has the same code with get_cfp_error() and cfgetc uses
sterror() after gzgetc(). It would be suitable for a separate patch,
but 0002 fixes those bugs.  I changed _EndBlob() to show the cause of
an error.

Did not do in this patch:

We could do further small refactoring to remove temporary variables in
pg_backup_directory.c for _StartData(), InitArchiveFmt_Directory,
_LoadBlobs(), _StartBlobs() and _CloseArchive(), but I left them as is
for the ease of back-patching.

Now that we have the file name in the context variable so we could
show the file name in all error messages, but that change was large
and there's a part where that change is a bit more complex so I didn't
do that.


regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

pgsql-hackers by date:

Previous
From: Julien Rouhaud
Date:
Subject: Re: Online checksums verification in the backend
Next
From: Heikki Linnakangas
Date:
Subject: Re: [doc] remove reference to pg_dump pre-8.1 switch behaviour