On Fri, Sep 30, 2022 at 6:46 AM Thomas Munro <thomas.munro@gmail.com> wrote:
>
> +1 for just doing it always, with a one-liner comment like
> "pg_pwritev*() might move the file position". No reason to spam the
> source tree with more explanations of the exact reason.
+1 for resetting the file position in a platform-independent manner.
But, a description there won't hurt IMO and it saves time for the
hackers who spend time there and think why it's that way.
> If someone
> ever comes up with another case where p- and non-p- I/O functions are
> intermixed and it's really worth saving a system call (don't get me
> wrong, we call lseek() an obscene amount elsewhere and I'd like to fix
> that, but this case isn't hot?) then I like your idea of a macro to
> tell you whether you need to.
I don't think we go that route as the code isn't a hot path and an
extra system call wouldn't hurt performance much, a comment there
should work.
> Earlier I wondered why we'd want to include "pg_pwritev" in the name
> of this zero-filling function (pwritev being an internal
> implementation detail), but now it seems like maybe a good idea
> because it highlights the file position portability problem by being a
> member of that family of similarly-named functions.
Hm.
On Thu, Sep 29, 2022 at 10:57 PM Nathan Bossart
<nathandbossart@gmail.com> wrote:
>
> + iov[0].iov_base = zbuffer.data;
>
> This seems superfluous, but I don't think it's hurting anything.
Yes, I removed it. Adding a comment, something like [1], would make it
more verbose, hence I've not added.
I'm attaching the v6 patch set, please review it further.
[1]
/*
* Use the first vector buffer to write the remaining size. Note that
* zero buffer was already pointed to it above, hence just specifying
* the size is enough here.
*/
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
