public inbox for [email protected]
help / color / mirror / Atom feedFrom: Peter Eisentraut <[email protected]>
To: Thomas Munro <[email protected]>
To: Heikki Linnakangas <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Japin Li <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Cannot find a working 64-bit integer type on Illumos
Date: Sun, 14 Jul 2024 15:47:50 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <CA+hUKG+rYp-=fTP5kjRmnfPq7YgK-e=J+gJxBvKarkPVzLwQmQ@mail.gmail.com>
References: <ME3P282MB3166F9D1F71F787929C0C7E7B6312@ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM>
<[email protected]>
<ME3P282MB3166D876E9CB83009727A5ECB6312@ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM>
<[email protected]>
<CA+hUKG+CR05ACFujUbeAd-yM+nNJBSpnyB=q3vbriCJuoSu9Ag@mail.gmail.com>
<[email protected]>
<CA+hUKG+ZwX-reVMHak3yo-63rrh9AJwEVGW4rCbsoLeSKO54Pg@mail.gmail.com>
<[email protected]>
<CA+hUKGKXEdVSfR6UHXfR3tSQ+TfBNk-ssYmO2F0wCdw7qcx86Q@mail.gmail.com>
<[email protected]>
<CA+hUKG+rYp-=fTP5kjRmnfPq7YgK-e=J+gJxBvKarkPVzLwQmQ@mail.gmail.com>
On 04.07.24 03:55, Thomas Munro wrote:
> Unfortunately, that theory turned out to be wrong. The usual suspect,
> Windows, uses something else: "I64" or something like that. We could
> teach our snprintf to grok that, but I don't like the idea anymore.
> So let's go back to INT64_MODIFIER, with just a small amount of
> configure time work to pick the right value. I couldn't figure out
> any header-only way to do that.
src/port/snprintf.c used to support I64 in the past, but it was later
removed. We could probably put it back.
>> Personally, I find "PRId64" pretty unreadable. "INT64_MODIFIER" wasn't
>> nice either, though, and following standards is good, so I'm sure I'll
>> get used to it.
Using PRId64 would be very beneficial because gettext understands it,
and so we would no longer need the various workarounds for not putting
INT64_FORMAT into the middle of a translated string.
But this could be a separate patch. What you have works for now.
Here are some other comments on this patch set:
* v3-0001-Use-int64_t-support-from-stdint.h.patch
- src/include/c.h:
Maybe add a comment that above all the int8_t -> int8 etc. typedefs
that these are for backward compatibility or something like that.
Actually, just move the comment
+/* Historical names for limits in stdint.h. */
up a bit to it covers the types as well.
Also, these /* == 8 bits */ comments could be removed, I think.
- src/include/port/pg_bitutils.h:
- src/port/pg_bitutils.c:
- src/port/snprintf.c:
These changes look functionally correct, but I think I like the old
code layout better, like
#if (using long)
...
#elif (using long long)
...
#else
#error
#endif
rather than
#if (using long)
...
#else
static assertion
... // long long
#endif
which seems a bit more complicated. I think you could leave the code
mostly alone and just change
defined(HAVE_LONG_INT_64) to SIZEOF_LONG == 8
defined(HAVE_LONG_LONG_INT_64) to SIZEOF_LONG_LONG == 8
in each case.
- src/include/postgres_ext.h:
-#define OID_MAX UINT_MAX
-/* you will need to include <limits.h> to use the above #define */
+#define OID_MAX UINT32_MAX
If the type Oid is unsigned int, then the OID_MAX should be UINT_MAX.
So this should not be changed. Also, is the comment about <limits.h>
no longer applicable?
- src/interfaces/ecpg/ecpglib/typename.c:
- src/interfaces/ecpg/include/sqltypes.h:
- .../test/expected/compat_informix-sqlda.c:
-#ifdef HAVE_LONG_LONG_INT_64
+#if SIZEOF_LONG < 8
These changes alter the library behavior unnecessarily. The old code
would always prefer to report back long long (ECPGt_long_long etc.),
but the new code will report back long (ECPGt_long etc.) if it is
64-bit. I don't know the impact of these changes, but it seems
preferable to keep the existing behavior.
- src/interfaces/ecpg/include/ecpg_config.h.in:
- src/interfaces/ecpg/include/meson.build:
In the past, we have kept obsolete symbols as always defined in
ecpg_config.h. We ought to do the same here.
* v3-0002-Remove-traces-of-BeOS.patch
This looks ok. This could also be committed before 0001.
* v3-0003-Allow-tzcode-to-use-stdint.h-and-inttypes.h.patch
- src/timezone/localtime.c:
Addition of #include <stdint.h> is unnecessary, since it's already
included in c.h, and it's also not in the upstream code.
This looks like a typo:
- * UTC months are at least 28 days long
(minus 1 second for a
+ * UTC months are at least 2 days long
(minus 1 second for a
-getsecs(const char *strp, int32 *const secsp)
+getsecs(const char *strp, int_fast32_t * const secsp)
Need to add int_fast32_t (and maybe the other types) to typedefs.list?
- src/timezone/zic.c:
+#include <inttypes.h>
+#include <stdint.h>
We don't need both of these. Also, this is not in the upstream code
AFAICT.
view thread (25+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Cannot find a working 64-bit integer type on Illumos
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox