Re: Add pg_file_sync() to adminpack
Atsushi Torikoshi <[email protected]> Wed, 15 Jan 2020 00:08:06 +0900
| Newsgroups | gmane.comp.db.postgresql.devel.general |
|---|---|
| Message-ID | <CACZ0uYGAkKeA-tsX82Ypuu1HkWem8=VeBpK0XNOD51ackKD1+w@mail.gmail.com> |
--0000000000003dbab0059c1af6bd Content-Type: text/plain; charset="UTF-8" Hello, > On Sut, Jan 11, 2020 at 2:12 Fujii Masao <[email protected]>: > I'm not sure if returning false with WARNING only in some error cases > is really good idea or not. At least for me, it's more intuitive to > return true on success and emit an ERROR otherwise. I'd like to hear > more opinions about this. +1. As a user, I expect these adminpack functions to do similar behaviors to the corresponding system calls. System calls for flushing data to disk(fsync on Linux and FlushFileBuffers on Windows) return different codes on success and failure, and when it fails we can get error messages. So it seems straightforward for me to 'return true on success and emit an ERROR otherwise'. > > On Thu, Jan 9, 2020 at 10:39 PM Julien Rouhaud <[email protected]> wrote: > > > > > > I think that pg_write_server_files should be allowed to call that > > > function by default. > > > > But pg_write_server_files users are not allowed to execute > > other functions like pg_file_write() by default. So doing that > > change only for pg_file_sync() looks strange to me. > Ah indeed. I'm wondering if that's an oversight of the original > default role patch or voluntary. It's not directly related to the patch, but as far as I read the manual below, I expected pg_write_server_files users could execute adminpack functions. | Table 21.1 Default Roles | pg_write_server_files: Allow writing to files in any location the database can access on the server with COPY and other file-access functions. -- Atsushi Torikoshi --0000000000003dbab0059c1af6bd Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr">Hello,<br><br>> On Sut, Jan 11, 2020 at =C2=A02:12 Fuji= i Masao <<a href=3D"mailto:[email protected]">[email protected]<= /a>>:<br>> I'm not sure if returning false with WARNING only in s= ome error cases<br>> is really good idea or not. At least for me, it'= ;s more intuitive to<br>> return true on success and emit an ERROR other= wise. I'd like to hear<br>> more opinions about this.<br><br>+1.<br>= As a user, I expect these adminpack functions to do similar behaviors<br>to= the corresponding system calls.<br>System calls for flushing data to disk(= fsync on Linux and FlushFileBuffers<br>=C2=A0on Windows) return different c= odes on success and failure, and when it<br>fails we can get error messages= . So it seems straightforward for me to<br>=C2=A0'return true on succes= s and emit an ERROR otherwise'.<br><br><br>> > On Thu, Jan 9, 202= 0 at 10:39 PM Julien Rouhaud <<a href=3D"mailto:[email protected]">rjuj= [email protected]</a>> wrote:<br>> > ><br>> > > I think t= hat pg_write_server_files should be allowed to call that<br>> > > = function by default.<br>> ><br>> > But pg_write_server_files us= ers are not allowed to execute<br>> > other functions like pg_file_wr= ite() by default. So doing that<br>> > change only for pg_file_sync()= looks strange to me.<br><br>> Ah indeed.=C2=A0 I'm wondering if tha= t's an oversight of the original<br>> default role patch or voluntar= y.<br><br>It's not directly related to the patch, but as far as I read = the<br>manual below, I expected pg_write_server_files users could execute<b= r>=C2=A0adminpack functions.<br><br>=C2=A0 | Table 21.1 Default Roles<br>= =C2=A0 | pg_write_server_files: Allow writing to files in any location the = database can access on the server with COPY and other file-access functions= .<br><br><br>--<br>Atsushi Torikoshi</div> --0000000000003dbab0059c1af6bd--