Hi, On 2018-10-20 13:30:46 +0900, Michael Paquier wrote: > On Fri, Oct 19, 2018 at 08:50:04PM -0700, Andres Freund wrote: > > On 2018-10-20 12:39:55 +0900, Michael Paquier wrote: > >> So what I think we ought to do is the following: > >> - Start a new thread, this one about TAP tests is not adapted. > >> - Add in src/common/relpath.c the API from d55241af called > >> isRelFileName(), make use of it in the base backup code, and basically > >> remove is_checksummed_file() and the checksum skip list. > > > > I think it probably shouldn't quite be that as an API. The code should > > not just check whether the file matches a pattern, but also importantly > > needs to exclude files that are in a temp tablespace. isRelFileName() > > doesn't quite describe an API meaning that. I assume we should keep > > something like isRelFileName() but use an API ontop of that that also > > exclude temp files / relations. > > From what I can see we would need to check for a couple of patterns if > we go to this extent: > - Look for PG_TEMP_FILE_PREFIX and exclude those. > - looks_like_temp_rel_name() which checks for temp files names. This is > similar to isRelFileName except that it works on temporary files. > Moving it to relpath.c and renaming it IsTempRelFileName is an idea. > But this one would not be necessary as isRelFileName discards temporary > relations, no?
I think it's not good to have those necessary intermingled in an exposed function. > At the end, do we really need to do anything more than adding some > checks on PG_TEMP_FILE_PREFIX? I think we also need the exclusion list basebackup.c has that generally skips files. They might be excluded anyway, but I think it'd be safer to make sure. > I am not sure that there is much need for a global API like > isChecksummedFile for only those two places. Seems likely that other tools would want to have access too. Greetings, Andres Freund
