On Thu, Aug 17, 2023 at 09:31:55AM +0900, Michael Paquier wrote:
> Looks sensible seen from here.
Thanks for taking a look.
> This patch is missing the installation of protocol.h in
> src/tools/msvc/Install.pm for MSVC. For pqcomm.h, we are doing that:
> lcopy('src/include/libpq/pqcomm.h', $target . '/include/internal/libpq/')
> || croak 'Could not copy pqcomm.h';
>
> So adding two similar lines for protocol.h should be enough (I assume,
> did not test).
I added those lines in v7.
> In fe-exec.c, we still have a few things for the type of objects to
> work on:
> - 'S' for statement.
> - 'P' for portal.
> Should these be added to protocol.h? They are part of the extended
> protocol.
IMHO they should be added, but I've intentionally restricted this first
patch to only codes with existing names in protocol.sgml. I figured we
could work on naming other things in a follow-up discussion.
> The comment at the top of PQsendTypedCommand() mentions 'C' and 'D',
> but perhaps these should be updated to the object names instead?
Done.
> pqFunctionCall3(), for PQfn(), has a few more hardcoded characters for
> its status codes. I'm OK to do things incrementally so it's fine by
> me to not add them now, just noticing on the way what could be added
> to this new header.
Cool, thanks.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
