Re: pgsql: Improve performance of timezone loading, especially pg_timezone_

On Sun, May 7, 2017 at 2:33 PM, David Rowley
<david(dot)rowley(at)2ndquadrant(dot)com> wrote:
> , On 7 May 2017 at 17:19, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> writes:
>>> On Wed, May 3, 2017 at 7:21 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>> Improve performance of timezone loading, especially pg_timezone_names view.
>>
>>> I am consistently getting below error after this commit in my Win7 machine:
>>> running bootstrap script ... 2017-05-07 05:09:13.383 GMT [7908] LOG:
>>> could not open directory "/installation/share/timezone/posixrules": No
>>> such file or directory
>>
>>> This occurs both during initdb and server start.
>>
>> You sure it wasn't there before, too?

Yes and that is confirmed by David as well.

>> That commit did not add any
>> file reads that didn't happen before.
>
> It didn't exist before either, but I don't think the error is what you
> think. Please note the error message indicated it's looking for a
> "directory" named posixrules, not the file.
>
> The error seems to be due to this code:
>
> if (canonname == NULL)
> {
> int result;
>
> fullname[fullnamelen] = '/';
> /* test above ensured this will fit: */
> strcpy(fullname + fullnamelen + 1, name);
> result = open(fullname, O_RDONLY | PG_BINARY, 0);
> if (result >= 0)
> return result;
> /* If that didn't work, fall through to do it the hard way */
> }
>
> where we append the name, then try to open the file, but since the
> file does not exist result is < 0 and we fall through to the for(;;)
> loop and try to do scan_directory_ci() on the filename rather than the
> directory.
>
>
> /* If that didn't work, fall through to do it the hard way */
> fullname[fullnamelen] = '\0';
>

Yes, that is one way of doing it, otherwise, we could save the
original fullname (before appending 'name' to it) and then use the
same in for loop.

> but I've not yet looked into why the file is missing in the first place.

I think it is better to consider that as a separate patch as that is a
different problem that exists before this commit as well.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com