On Tue, Jun 14, 2022 at 1:40 PM Mark Wong <markwkm@gmail.com> wrote:
> Checking in for quick feedback to see if this refactor makes sense.
>
> I've created a function for each data type with the idea that an example
> for handling a specific data type can be more easily reviewed by looking
> in a single place.
>
> I've added examples for REAL, TIMESTAMP WITHOUT TIME ZONE, and BOOLEAN
> to try to illustrate how testlibpq3.sql and testlibpq3.c will grow if
> this is a good way to go.
I think this could be a reasonable kind of thing to do, but I think
you left the "ts" output out of the example output in the comments,
and also your code's apparently not portable, because my compiler is
OK with testlibpq3 right now, but with your patch it emits lengthy
unhappy moaning, starting with:
testlibpq3.c:88:10: error: expected ')'
uint64_t ntohll(uint64_t);
^
/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/include/sys/_endian.h:140:25:
note: expanded from macro 'ntohll'
#define ntohll(x) __DARWIN_OSSwapInt64(x)
^
Apparently macOS defines ntohll as a macro, and it's not amused by
your attempt to include a function prototype for it.
I'm not sure that we want to let these test programs grow very large.
In particular, I don't think we should let ourselves get sucked into
adding an example for every data type under the sun -- if that's
wanted, the solution is perhaps to add documentation for the binary
formats, not hide impromptu documentation inside a test program. But
doing this much seems OK to me.
--
Robert Haas
EDB: http://www.enterprisedb.com