Re: [Patch] Check file type before calling AllocateFile() for filesout of pg data directory to avoid potential issues (e.g. hang).

Поиск
Список
Период
Сортировка
От Paul Guo
Тема Re: [Patch] Check file type before calling AllocateFile() for filesout of pg data directory to avoid potential issues (e.g. hang).
Дата
Msg-id CAEET0ZH29mMNzmV=UWGSnmZ2m0dqS-du_hs_VfX=8_LN4KT-iw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: [Patch] Check file type before calling AllocateFile() for files out of pg data directory to avoid potential issues (e.g. hang).  (Tom Lane <tgl@sss.pgh.pa.us>)
Ответы Re: [Patch] Check file type before calling AllocateFile() for files out of pg data directory to avoid potential issues (e.g. hang).  (Tom Lane <tgl@sss.pgh.pa.us>)
Список pgsql-hackers
Дерево обсуждения


On Wed, Apr 24, 2019 at 10:36 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2019-Apr-24, Paul Guo wrote:
>> On Wed, Apr 24, 2019 at 12:49 PM Andres Freund <andres@anarazel.de> wrote:
>>> This seems like a bad idea to me. IMO we want to support using a pipe
>>> etc here. If the admin creates a fifo like this without attaching a
>>> program it seems like it's their fault.

>> Oh, I never know this application scenario before. So yes, for this, we
>> need to keep the current code logic in copy code.

> But the pgstat.c patch seems reasonable to me.

Nah, I don't buy that one either.  Nobody has any business creating any
non-Postgres files in the stats directory ... and if somebody does want
to stick a FIFO in there, perhaps for debug purposes, why should we stop
them?

For the pgstat case, the files for AllocateFile() are actually temp files which
are soon renamed to other file names. Users might not want to set them as fifo files.
For developers 'tail -f' might be sufficient for debugging purpose.

      const char *tmpfile = permanent ? PGSTAT_STAT_PERMANENT_TMPFILE : pgstat_stat_tmpname;
      fpout = AllocateFile(tmpfile, PG_BINARY_W);
      fwrite(fpout, ...);
      rename(tmpfile, statfile);

I'm not sure if those hardcoded temp filenames (not just those in pgstat) are used across postgres reboot.
If no, we should instead call glibc function to create unique temp files and also remove those hardcode temp
filename variables, else we also might want them to be regular files.


The case with COPY is a bit different, since there it's reasonable to be
worried about collisions with other users' files --- but I agree with
Andres that this change would eliminate too many valid use-cases.

                        regards, tom lane

В списке pgsql-hackers по дате отправления:

Предыдущее
От: alex lock
Дата:
Сообщение: Re: Help to review the with X cursor option.
Следующее
От: Amit Kapila
Дата:
Сообщение: Re: Unhappy about API changes in the no-fsm-for-small-rels patch