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?
- parse_filename_for_nontemp_relation() is also too similar to
isRelFileName().
At the end, do we really need to do anything more than adding some
checks on PG_TEMP_FILE_PREFIX? I am not sure that there is much need
for a global API like isChecksummedFile for only those two places. I
have already a patch doing the work of moving isRelFileName() into
src/common/. Adding one check on PG_TEMP_FILE_PREFIX is not much work
on top of it.
--
Michael
