I am hesitant to write a function like AllocateDirWithFilePattern if the pattern is simply ignored on non-Windows. In my patch, the pattern underspecified the files, and the ad-hoc matching code applied to all the returned files tightened that up. But a person could just as well overspecify the pattern and then they would get different behavior on Windows vs. non-Windows, with fewer files returned by FindNextFile() than would have matched the ad-hoc pattern.
In this particular instance, AllocateDirWithFilePrefix would work, and could be applied on all platforms, using something like: temp_dir = AllocateDirWithFilePrefix(tmpdirname, PG_TEMP_FILE_PREFIX); and that could be converted to PG_TEMP_FILE_PREFIX "*" on Windows, and on non-Windows the function itself could apply the prefix check easily enough. Is AllocateDirWithFilePrefix overly specific? Clearly we could also take a Suffix argument, but if we go too far down this road we just reinvent regular expressions.... mark ________________________________ From: Tom Lane <t...@sss.pgh.pa.us> To: Mark Dilger <markdil...@yahoo.com> Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org> Sent: Tuesday, May 29, 2012 2:30 PM Subject: Re: [HACKERS] Performance patch for Win32 Mark Dilger <markdil...@yahoo.com> writes: > 4) Other places in the PostgreSQL sources where directory > iteration is needed should probably use a pattern if possible > when running on Windows. Thus, it might make more > sense to have a version of ReadDir that explicitly takes a > pattern, and use that version of ReadDir elsewhere in the > codebase. Yeah, I think a separate argument passed to an AllocateDir variant would be a less ugly way to deal with this. For example, in place of your first #ifdef block just write temp_dir = AllocateDirWithFilePattern(tmpdirname, PG_TEMP_FILE_PREFIX "*"); What is not immediately obvious to me is whether we should try to make the pattern argument do something useful on non-Windows platforms (and thus allow removal of the ad-hoc pattern match code in the loops where this is used); versus just treating it as a no-op on non-Windows. If we did that, we'd have to consider that Windows gets to define what the pattern language is and try to emulate that; which is likely to be expensive enough to not be a win, not to mention that non-Windows developers might not be terribly familiar with the finer points of the pattern language. I'm kind of inclined to consider that we should just treat the pattern option as a Windows-specific wart and keep the ad-hoc matching code as it is. regards, tom lane
