On Tue, Dec 10, 2024 at 3:02 PM Thomas Munro <thomas.mu...@gmail.com> wrote: > On Thu, Dec 5, 2024 at 12:16 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > > Now you already snuck the camel's nose under the > > tent by including stdint.h there, and maybe these additional headers > > wouldn't do any further damage. > > Even though we fixed the immediate issue (thanks), this comment stayed > with me. I did that because I didn't want to change any interfaces at > the same time as the <stdint.h> retrofit, but I agree that it feels a > bit odd hidden in there, and doesn't appear to conform to > postgres_ext.h's self-description. Stepping back, and I realise it's > difficult to answer with certainty, I wonder why anyone would ever > want to use postgres_ext.h directly for the definition of pg_int64 > *without* being a user of libpq-fe.h. I can't find any references to > pg_int64 (excluding forks of our code) on github; there are a few > things like proxies and suchlike that include postgres_ext.h for other > things, mostly bogusly (they also include libpq-fe.h, or they say they > want NAMEDATALEN, which isn't there anymore). > > We have just three lo_*64() functions using that type and then > pg_usec_time_t. Seems like a very narrow usage that hasn't spread, > likely only used to receive arguments, and really quite specific to > libpq-fe.h and not one of the "fundamental Postgres declarations". > Maybe we should consider moving #include <stdint.h> into libpq-fe.h? > > And if we included <stdint.h> overtly, rather than covertly in > postgres_ext.h, why would we still want a third name for int64_t? We > could change the three lo_*64() declarations to use the standard type > directly, but keep the historical typedef marked deprecated. > > > But I don't see a strong argument to > > change long-standing external APIs any more than we absolutely must. > > So perhaps you'll hate that idea then. I wonder if you'll hate it > more than keeping the #include in postgres_ext.h, hence putting the > idea forward!
Does anyone else have thoughts about this arguable leftover quirk from the <stdint.h> refactoring work? In brief, shouldn't libpq-fe.h include <stdint.h> directly, and use int64_t explicitly, instead of doing it "secretly" in another header that came about because of historical namespace pollution concerns, now gone? We require you to have a 64 bit integer type, we require C99, C99 requires int64_t to be defined if you have a 64 bit type, and there doesn't seem to be any reason to want pg_int64 other than to use these large object functions in libpq-fe.h. The current arrangement feels a bit obfuscated, leading to the patch in the previous email. Adding Ishii-san who introduced the three uses of pg_int64 to libpq-fe.h in 461ef73f.
