Re: Using defines for protocol characters

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