Re: [UNVERIFIED SENDER] Re: Challenges preventing us moving to 64 bit transaction id (XID)?

On Tue, Sep 07, 2021 at 02:38:13PM +0800, Julien Rouhaud wrote:
> On Tue, Sep 7, 2021 at 12:20 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>> And a couple of months later, the development cycle of 15 has begun.
>> While re-reading the thread, I got the impression that there is no
>> reason to not move on with at least the 64-bit GUC part, and that it
>> could be useful as a piece to move forward with the rest. Am I
>> getting the wrong impression?
>
> That's also my understanding, so +1 to apply that soon

I have been looking at this part, and there are a couple of things
that the patch is doing wrong, while there are other cases that we had
better support for consistency with the 32b case:
- No checks after decimal '.', 'e' and 'E' after the initial parsing
phase, but I think that we should allow those cases and then go
through strtod() or just strtold(). For example 1.1 or just 1e1 are
supported values.
- No support for units, the code failing if there is any trailing
character. This should be flexible enough to allow any number of
spaces between the value and its units.
- No support for octal and hex values.

+ for (i = 0; int64RelOpts[i].gen.name; i++)
+ {
+ Assert(DoLockModesConflict(int64RelOpts[i].gen.lockmode,
+ int64RelOpts[i].gen.lockmode));
+ j++;
+ }
+ for (i = 0; int64RelOpts[i].gen.name; i++)
+ {
+ Assert(DoLockModesConflict(int64RelOpts[i].gen.lockmode,
+ int64RelOpts[i].gen.lockmode));
+ j++;
+ }
This loop is repeated twice, this is required only once.

+#ifdef _MSC_VER /* MSVC only */
+ val = _strtoi64(value, &endptr, 10);
+#elif defined(HAVE_STRTOLL) && SIZEOF_LONG < 8
+ val = strtoll(value, &endptr, 10);
+#else
+ val = strtol(value, &endptr, 10);
+#endif
This is a copy-paste of pg_strtouint64() in numutils.c. We could just
add a similar routine there, as this code path cannot generate elog()s
and similar reports by itself. This code had better use 0 for the
base number, to be able to handle the hex and octal cases.

On top of the tests for needed for custom GUCs, this needs tests for
the new int64 reloption. I would suggest to add something in
dummy_index_am, where we test all the reloption APIs.
--
Michael