Обсуждение: Should we add xid_current() or a int8->xid cast?

Hi,

we have txid_current(), which returns an int8. But there's no convenient
way to convert that to type 'xid'. Which is fairly inconvenient, given
that we expose xids in various places.

My current need for this was just a regression test to make sure that
system columns (xmin/xmax in particular) don't get broken again for ON
CONFLICT. But I've needed this before in other scenarios - e.g. age(xid)
can be useful to figure out how old a transaction is, but age() doesn't
work with txid_current()'s return value.

Seems easiest to just add xid_current(), or add a cast from int8 to xid
(probably explicit?) that handles the wraparound logic correctly?

Greetings,

Andres Freund

On Thu, Jul 25, 2019 at 12:06 PM Andres Freund <andres@anarazel.de> wrote:
> we have txid_current(), which returns an int8. But there's no convenient
> way to convert that to type 'xid'. Which is fairly inconvenient, given
> that we expose xids in various places.
>
> My current need for this was just a regression test to make sure that
> system columns (xmin/xmax in particular) don't get broken again for ON
> CONFLICT. But I've needed this before in other scenarios - e.g. age(xid)
> can be useful to figure out how old a transaction is, but age() doesn't
> work with txid_current()'s return value.
>
> Seems easiest to just add xid_current(), or add a cast from int8 to xid
> (probably explicit?) that handles the wraparound logic correctly?

Yeah, I was wondering about that.  int8 isn't really the right type,
since FullTransactionId is unsigned.  If we had a SQL type for 64 bit
xids, it should be convertible to xid, and the reverse conversion
should require a more complicated dance.  Of course we can't casually
change txid_current() without annoying people who are using it, so
perhaps if we invent a new SQL type we should also make a new function
that returns it.

-- 
Thomas Munro
https://enterprisedb.com

Hi,

On 2019-07-25 12:20:58 +1200, Thomas Munro wrote:
> On Thu, Jul 25, 2019 at 12:06 PM Andres Freund <andres@anarazel.de> wrote:
> > we have txid_current(), which returns an int8. But there's no convenient
> > way to convert that to type 'xid'. Which is fairly inconvenient, given
> > that we expose xids in various places.
> >
> > My current need for this was just a regression test to make sure that
> > system columns (xmin/xmax in particular) don't get broken again for ON
> > CONFLICT. But I've needed this before in other scenarios - e.g. age(xid)
> > can be useful to figure out how old a transaction is, but age() doesn't
> > work with txid_current()'s return value.
> >
> > Seems easiest to just add xid_current(), or add a cast from int8 to xid
> > (probably explicit?) that handles the wraparound logic correctly?
> 
> Yeah, I was wondering about that.  int8 isn't really the right type,
> since FullTransactionId is unsigned.

For now that doesn't seem that big an impediment...


> If we had a SQL type for 64 bit xids, it should be convertible to xid,
> and the reverse conversion should require a more complicated dance.
> Of course we can't casually change txid_current() without annoying
> people who are using it, so perhaps if we invent a new SQL type we
> should also make a new function that returns it.

Possibly we could add a fullxid or xid8, xid64, pg_xid64, ... type, and
have an implicit cast to int8?

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> On 2019-07-25 12:20:58 +1200, Thomas Munro wrote:
>> On Thu, Jul 25, 2019 at 12:06 PM Andres Freund <andres@anarazel.de> wrote:
>>> Seems easiest to just add xid_current(), or add a cast from int8 to xid
>>> (probably explicit?) that handles the wraparound logic correctly?

>> Yeah, I was wondering about that.  int8 isn't really the right type,
>> since FullTransactionId is unsigned.

> For now that doesn't seem that big an impediment...

Yeah, I would absolutely NOT recommend that you open that can of worms
right now.  We have looked at adding unsigned integer types in the past
and it looked like a mess.

I think an explicit cast is a reasonable thing to add, though.

            regards, tom lane

Hi,

On 2019-07-24 20:34:39 -0400, Tom Lane wrote:
> Yeah, I would absolutely NOT recommend that you open that can of worms
> right now.  We have looked at adding unsigned integer types in the past
> and it looked like a mess.

I assume Thomas was thinking more of another bespoke type like xid, just
wider.  There's some notational advantage in not being able to
immediately do math etc on xids.

- Andres

Andres Freund <andres@anarazel.de> writes:
> On 2019-07-24 20:34:39 -0400, Tom Lane wrote:
>> Yeah, I would absolutely NOT recommend that you open that can of worms
>> right now.  We have looked at adding unsigned integer types in the past
>> and it looked like a mess.

> I assume Thomas was thinking more of another bespoke type like xid, just
> wider.  There's some notational advantage in not being able to
> immediately do math etc on xids.

Well, we could invent an xid8 type if we want, just don't try to make
it part of the numeric hierarchy (as indeed xid isn't).

            regards, tom lane

On Thu, Jul 25, 2019 at 12:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2019-07-24 20:34:39 -0400, Tom Lane wrote:
> >> Yeah, I would absolutely NOT recommend that you open that can of worms
> >> right now.  We have looked at adding unsigned integer types in the past
> >> and it looked like a mess.
>
> > I assume Thomas was thinking more of another bespoke type like xid, just
> > wider.  There's some notational advantage in not being able to
> > immediately do math etc on xids.
>
> Well, we could invent an xid8 type if we want, just don't try to make
> it part of the numeric hierarchy (as indeed xid isn't).

Yeah, I meant an xid64/xid8/fxid/pg_something/... type that isn't a
kind of number.

-- 
Thomas Munro
https://enterprisedb.com

On Thu, Jul 25, 2019 at 1:11 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> On Thu, Jul 25, 2019 at 12:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Andres Freund <andres@anarazel.de> writes:
> > > On 2019-07-24 20:34:39 -0400, Tom Lane wrote:
> > >> Yeah, I would absolutely NOT recommend that you open that can of worms
> > >> right now.  We have looked at adding unsigned integer types in the past
> > >> and it looked like a mess.
> >
> > > I assume Thomas was thinking more of another bespoke type like xid, just
> > > wider.  There's some notational advantage in not being able to
> > > immediately do math etc on xids.
> >
> > Well, we could invent an xid8 type if we want, just don't try to make
> > it part of the numeric hierarchy (as indeed xid isn't).
>
> Yeah, I meant an xid64/xid8/fxid/pg_something/... type that isn't a
> kind of number.

I played around with an xid8 type over here (not tested much yet, in
particular not tested on 32 bit box):

https://www.postgresql.org/message-id/CA%2BhUKGKbQtX8E5TEdcZaYhTxqLqrvcpN1Vjb7eCu2bz5EACZbw%40mail.gmail.com

-- 
Thomas Munro
https://enterprisedb.com

On Fri, Aug 2, 2019 at 10:42 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> On Thu, Jul 25, 2019 at 1:11 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> > On Thu, Jul 25, 2019 at 12:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > > Andres Freund <andres@anarazel.de> writes:
> > > > On 2019-07-24 20:34:39 -0400, Tom Lane wrote:
> > > >> Yeah, I would absolutely NOT recommend that you open that can of worms
> > > >> right now.  We have looked at adding unsigned integer types in the past
> > > >> and it looked like a mess.
> > >
> > > > I assume Thomas was thinking more of another bespoke type like xid, just
> > > > wider.  There's some notational advantage in not being able to
> > > > immediately do math etc on xids.
> > >
> > > Well, we could invent an xid8 type if we want, just don't try to make
> > > it part of the numeric hierarchy (as indeed xid isn't).
> >
> > Yeah, I meant an xid64/xid8/fxid/pg_something/... type that isn't a
> > kind of number.

I thought about how to deal with the transition to xid8 for the
txid_XXX() family of functions.  The best idea I've come up with so
far is to create a parallel xid8_XXX() family of functions, and
declare the bigint-based functions to be deprecated, and threaten to
drop them from a future release.  The C code for the two families can
be the same (it's a bit of a dirty trick, but only until the
txid_XXX() variants go away).  Here's a PoC patch demonstrating that.
Not tested much, yet, probably needs some more work, but I wanted to
see if others thought the idea was terrible first.

I wonder if there is a better way to share hash functions than the
hack in check_hash_func_signature(), which I had to extend to cover
xid8.

Adding to CF.

-- 
Thomas Munro
https://enterprisedb.com

On Sun, Sep 1, 2019 at 5:04 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> Adding to CF.

Rebased.  An OID clashed so re-roll the dice.  Also spotted a typo.

-- 
Thomas Munro
https://enterprisedb.com

Thomas Munro <thomas.munro@gmail.com> writes:
> On Sun, Sep 1, 2019 at 5:04 PM Thomas Munro <thomas.munro@gmail.com> wrote:
>> Adding to CF.

> Rebased.  An OID clashed so re-roll the dice.  Also spotted a typo.

FWIW, I'd move *all* the OIDs added by this patch up to >= 8000.
I don't feel a strong need to fill in the gaps in the low-numbered
OIDs, and people who do try that are likely to hit problems of the
sort you just did.

            regards, tom lane

> 
> Thomas Munro <thomas.munro@gmail.com> writes:
>> On Sun, Sep 1, 2019 at 5:04 PM Thomas Munro <thomas.munro@gmail.com> 
>> wrote:
>>> Adding to CF.
> 
>> Rebased.  An OID clashed so re-roll the dice.  Also spotted a typo.
> 

I have some questions in this code.

First,
"FullTransactionIdPrecedes(xmax, val)" is not equal to "val >= xmax" of 
the previous code.  "FullTransactionIdPrecedes(xmax, val)" expresses 
"val > xmax". Is it all right?

@@ -384,15 +324,17 @@ parse_snapshot(const char *str)
      while (*str != '\0')
      {
          /* read next value */
-        val = str2txid(str, &endp);
+        val = FullTransactionIdFromU64(pg_strtouint64(str, &endp, 10));
          str = endp;

          /* require the input to be in order */
-        if (val < xmin || val >= xmax || val < last_val)
+        if (FullTransactionIdPrecedes(val, xmin) ||
+            FullTransactionIdPrecedes(xmax, val) ||
+            FullTransactionIdPrecedes(val, last_val))

In addition to it, as to current TransactionId(not FullTransactionId) 
comparison, when we express ">=" of TransactionId, we use 
"TransactionIdFollowsOrEquals". this method is referred by some methods. 
On the other hand, FullTransactionIdFollowsOrEquals has not implemented 
yet. So, how about implementing this method?

Second,
About naming rule, "8" of xid8 means 8 bytes, but "8" has different 
meaning in each situation. For example, int8 of PostgreSQL means 8 
bytes, int8 of C language means 8 bits. If 64 is used, it just means 64 
bits. how about xid64()?

regards,

Takao Fujii

On Tue, Oct 29, 2019 at 5:23 PM btfujiitkp <btfujiitkp@oss.nttdata.com> wrote:
> > Thomas Munro <thomas.munro@gmail.com> writes:
> >> On Sun, Sep 1, 2019 at 5:04 PM Thomas Munro <thomas.munro@gmail.com>
> >> wrote:
> >>> Adding to CF.
> >
> >> Rebased.  An OID clashed so re-roll the dice.  Also spotted a typo.
> >
>
> I have some questions in this code.

Thanks for looking at the patch.

> First,
> "FullTransactionIdPrecedes(xmax, val)" is not equal to "val >= xmax" of
> the previous code.  "FullTransactionIdPrecedes(xmax, val)" expresses
> "val > xmax". Is it all right?
>
> @@ -384,15 +324,17 @@ parse_snapshot(const char *str)
>         while (*str != '\0')
>         {
>                 /* read next value */
> -               val = str2txid(str, &endp);
> +               val = FullTransactionIdFromU64(pg_strtouint64(str, &endp, 10));
>                 str = endp;
>
>                 /* require the input to be in order */
> -               if (val < xmin || val >= xmax || val < last_val)
> +               if (FullTransactionIdPrecedes(val, xmin) ||
> +                       FullTransactionIdPrecedes(xmax, val) ||
> +                       FullTransactionIdPrecedes(val, last_val))
>
> In addition to it, as to current TransactionId(not FullTransactionId)
> comparison, when we express ">=" of TransactionId, we use
> "TransactionIdFollowsOrEquals". this method is referred by some methods.
> On the other hand, FullTransactionIdFollowsOrEquals has not implemented
> yet. So, how about implementing this method?

Good idea.  I added the missing variants:

+#define FullTransactionIdPrecedesOrEquals(a, b) ((a).value <= (b).value)
+#define FullTransactionIdFollows(a, b) ((a).value > (b).value)
+#define FullTransactionIdFollowsOrEquals(a, b) ((a).value >= (b).value)

> Second,
> About naming rule, "8" of xid8 means 8 bytes, but "8" has different
> meaning in each situation. For example, int8 of PostgreSQL means 8
> bytes, int8 of C language means 8 bits. If 64 is used, it just means 64
> bits. how about xid64()?

In C, the typenames use bits, by happy coincidence similar to the C99
stdint.h typenames (int32_t etc) that we should perhaps eventually
switch to.

In SQL, the types have names based on the number of bytes: int2, int4,
int8, float4, float8, not conforming to any standard but established
over 3 decades ago and also understood by a few other SQL systems.

That's unfortunate, but I can't see that ever changing.  I thought
that it would make most sense for the SQL type to be called xid8,
though admittedly it doesn't quite fit the pattern because xid is not
called xid4.  There is another example a bit like that: macaddr (6
bytes) and macaccdr8 (8 bytes).  As for the C type, we use
TransactionId and FullTransactionId (rather than, say, xid32 and
xid64).

In the attached I also took Tom's advice and used unused_oids script
to pick random OIDs >= 8000 for all new objects (ignoring nearby
comments about the range of OIDs used in different sections of the
file).

> On Tue, Oct 29, 2019 at 5:23 PM btfujiitkp <btfujiitkp@oss.nttdata.com> 
> wrote:
>> > Thomas Munro <thomas.munro@gmail.com> writes:
>> >> On Sun, Sep 1, 2019 at 5:04 PM Thomas Munro <thomas.munro@gmail.com>
>> >> wrote:
>> >>> Adding to CF.
>> >
>> >> Rebased.  An OID clashed so re-roll the dice.  Also spotted a typo.
>> >
>> 
>> I have some questions in this code.
> 
> Thanks for looking at the patch.
> 
>> First,
>> "FullTransactionIdPrecedes(xmax, val)" is not equal to "val >= xmax" 
>> of
>> the previous code.  "FullTransactionIdPrecedes(xmax, val)" expresses
>> "val > xmax". Is it all right?
>> 
>> @@ -384,15 +324,17 @@ parse_snapshot(const char *str)
>>         while (*str != '\0')
>>         {
>>                 /* read next value */
>> -               val = str2txid(str, &endp);
>> +               val = FullTransactionIdFromU64(pg_strtouint64(str, 
>> &endp, 10));
>>                 str = endp;
>> 
>>                 /* require the input to be in order */
>> -               if (val < xmin || val >= xmax || val < last_val)
>> +               if (FullTransactionIdPrecedes(val, xmin) ||
>> +                       FullTransactionIdPrecedes(xmax, val) ||
>> +                       FullTransactionIdPrecedes(val, last_val))
>> 
>> In addition to it, as to current TransactionId(not FullTransactionId)
>> comparison, when we express ">=" of TransactionId, we use
>> "TransactionIdFollowsOrEquals". this method is referred by some 
>> methods.
>> On the other hand, FullTransactionIdFollowsOrEquals has not 
>> implemented
>> yet. So, how about implementing this method?
> 
> Good idea.  I added the missing variants:
> 
> +#define FullTransactionIdPrecedesOrEquals(a, b) ((a).value <= 
> (b).value)
> +#define FullTransactionIdFollows(a, b) ((a).value > (b).value)
> +#define FullTransactionIdFollowsOrEquals(a, b) ((a).value >= 
> (b).value)
> 

Thank you for your patch.
It looks good.


>> Second,
>> About naming rule, "8" of xid8 means 8 bytes, but "8" has different
>> meaning in each situation. For example, int8 of PostgreSQL means 8
>> bytes, int8 of C language means 8 bits. If 64 is used, it just means 
>> 64
>> bits. how about xid64()?
> 
> In C, the typenames use bits, by happy coincidence similar to the C99
> stdint.h typenames (int32_t etc) that we should perhaps eventually
> switch to.
> 
> In SQL, the types have names based on the number of bytes: int2, int4,
> int8, float4, float8, not conforming to any standard but established
> over 3 decades ago and also understood by a few other SQL systems.
> 
> That's unfortunate, but I can't see that ever changing.  I thought
> that it would make most sense for the SQL type to be called xid8,
> though admittedly it doesn't quite fit the pattern because xid is not
> called xid4.  There is another example a bit like that: macaddr (6
> bytes) and macaccdr8 (8 bytes).  As for the C type, we use
> TransactionId and FullTransactionId (rather than, say, xid32 and
> xid64).

That makes sense.

Anyway,
In the pg_proc.dat, "xid_snapshot_xip" should be "xid8_snapshot_xip".
And some parts of 0002 patch are rejected when I patch 0002 after 
patching 0001.

regards

Hi Thomas,

Please let me ask something about wraparound problem.

+static FullTransactionId
+convert_xid(TransactionId xid, FullTransactionId next_fxid)
 {
-    uint64        epoch;
+    TransactionId next_xid = XidFromFullTransactionId(next_fxid);
+    uint32 epoch = EpochFromFullTransactionId(next_fxid);
 
...
 
-    /* xid can be on either side when near wrap-around */
-    epoch = (uint64) state->epoch;
-    if (xid > state->last_xid &&
-        TransactionIdPrecedes(xid, state->last_xid))
+    if (xid > next_xid)
         epoch--;
-    else if (xid < state->last_xid &&
-             TransactionIdFollows(xid, state->last_xid))
-        epoch++;
 
-    return (epoch << 32) | xid;
+    return FullTransactionIdFromEpochAndXid(epoch, xid);


ISTM codes for wraparound are deleted. Is that correct?
I couldn't have read all related threads about using FullTransactionId but
does using FullTransactionId avoid wraparound problem? 

If we consider below conditions, we can say it's difficult to see wraparound
with current disk like SSD (2GB/s) or memory DDR4 (34GB/s), but if we can use 
more high-spec hardware like HBM3 (2048GB/s), we can see wraparound. Or do
I say silly things?

* 10 year system ( < 2^4 )
* 1 year = 31536000 ( = 60 * 60 * 24 * 365) secs  ( < 2^25 )
* 2^35 ( = 2^64 / 2^4 / 2^25) transactions we can use in each seconds
* we can write at (2^5 * 2^30 * n) bytes/sec = (32 * n) GB/sec if we use 'n'
  bytes for each transactions.

Is there any agreement we can throw the wraparound problem away if we adopt
FullTransactionId?


Thanks
--
Yoshikazu Imai

On Wed, Nov 20, 2019 at 5:43 PM imai.yoshikazu@fujitsu.com
<imai.yoshikazu@fujitsu.com> wrote:
> Is there any agreement we can throw the wraparound problem away if we adopt
> FullTransactionId?

Here is one argument for why 64 bits ought to be enough: we use 64 bit
LSNs for the WAL, and it usually takes more than one byte of WAL to
consume a transaction.  If you write about 500MB of WAL per second,
your system will break in about a thousand years due to LSN
wraparound, that is, assuming the earth hasn't been destroyed to make
way for a hyperspace bypass, but either way you will probably still
have some spare full transaction IDs.

That's fun to think about, but unfortunately it's not easy to figure
out how to retrofit FullTransactionId into enough places to make
wraparounds go away in the traditional heap.  It's a goal of at least
a couple of ongoing new AM projects to not have that problem, and I
figured it was a good idea to lay down very basic facilities for that,
trivial as they might be, and see where else they can be useful...

On 11/3/19 2:43 PM, Thomas Munro wrote:
> On Tue, Oct 29, 2019 at 5:23 PM btfujiitkp <btfujiitkp@oss.nttdata.com> wrote:
>>> Thomas Munro <thomas.munro@gmail.com> writes:
>>>> On Sun, Sep 1, 2019 at 5:04 PM Thomas Munro <thomas.munro@gmail.com>
>>>> wrote:
>>>>> Adding to CF.
>>>
>>>> Rebased.  An OID clashed so re-roll the dice.  Also spotted a typo.
>>>
>>
>> I have some questions in this code.
> 
> Thanks for looking at the patch.
> 
>> First,
>> "FullTransactionIdPrecedes(xmax, val)" is not equal to "val >= xmax" of
>> the previous code.  "FullTransactionIdPrecedes(xmax, val)" expresses
>> "val > xmax". Is it all right?
>>
>> @@ -384,15 +324,17 @@ parse_snapshot(const char *str)
>>          while (*str != '\0')
>>          {
>>                  /* read next value */
>> -               val = str2txid(str, &endp);
>> +               val = FullTransactionIdFromU64(pg_strtouint64(str, &endp, 10));
>>                  str = endp;
>>
>>                  /* require the input to be in order */
>> -               if (val < xmin || val >= xmax || val < last_val)
>> +               if (FullTransactionIdPrecedes(val, xmin) ||
>> +                       FullTransactionIdPrecedes(xmax, val) ||
>> +                       FullTransactionIdPrecedes(val, last_val))
>>
>> In addition to it, as to current TransactionId(not FullTransactionId)
>> comparison, when we express ">=" of TransactionId, we use
>> "TransactionIdFollowsOrEquals". this method is referred by some methods.
>> On the other hand, FullTransactionIdFollowsOrEquals has not implemented
>> yet. So, how about implementing this method?
> 
> Good idea.  I added the missing variants:
> 
> +#define FullTransactionIdPrecedesOrEquals(a, b) ((a).value <= (b).value)
> +#define FullTransactionIdFollows(a, b) ((a).value > (b).value)
> +#define FullTransactionIdFollowsOrEquals(a, b) ((a).value >= (b).value)
> 
>> Second,
>> About naming rule, "8" of xid8 means 8 bytes, but "8" has different
>> meaning in each situation. For example, int8 of PostgreSQL means 8
>> bytes, int8 of C language means 8 bits. If 64 is used, it just means 64
>> bits. how about xid64()?
> 
> In C, the typenames use bits, by happy coincidence similar to the C99
> stdint.h typenames (int32_t etc) that we should perhaps eventually
> switch to.
> 
> In SQL, the types have names based on the number of bytes: int2, int4,
> int8, float4, float8, not conforming to any standard but established
> over 3 decades ago and also understood by a few other SQL systems.
> 
> That's unfortunate, but I can't see that ever changing.  I thought
> that it would make most sense for the SQL type to be called xid8,
> though admittedly it doesn't quite fit the pattern because xid is not
> called xid4.  There is another example a bit like that: macaddr (6
> bytes) and macaccdr8 (8 bytes).  As for the C type, we use
> TransactionId and FullTransactionId (rather than, say, xid32 and
> xid64).
> 
> In the attached I also took Tom's advice and used unused_oids script
> to pick random OIDs >= 8000 for all new objects (ignoring nearby
> comments about the range of OIDs used in different sections of the
> file).
> 

These two patches (v3) no longer apply cleanly.  Could you please
rebase?

-- 
Mark Dilger

On Sun, Dec 1, 2019 at 12:22 PM Mark Dilger <hornschnorter@gmail.com> wrote:
> These two patches (v3) no longer apply cleanly.  Could you please
> rebase?

Hi Mark,
Thanks.  Here's v4.

On 11/30/19 5:14 PM, Thomas Munro wrote:
> On Sun, Dec 1, 2019 at 12:22 PM Mark Dilger <hornschnorter@gmail.com> wrote:
>> These two patches (v3) no longer apply cleanly.  Could you please
>> rebase?
> 
> Hi Mark,
> Thanks.  Here's v4.

Thanks, Thomas.

The new patches apply cleanly and pass 'installcheck'.

-- 
Mark Dilger

On Tue, Dec 3, 2019 at 6:56 AM Mark Dilger <hornschnorter@gmail.com> wrote:
> On 11/30/19 5:14 PM, Thomas Munro wrote:
> > On Sun, Dec 1, 2019 at 12:22 PM Mark Dilger <hornschnorter@gmail.com> wrote:
> >> These two patches (v3) no longer apply cleanly.  Could you please
> >> rebase?
> >
> > Hi Mark,
> > Thanks.  Here's v4.
>
> Thanks, Thomas.
>
> The new patches apply cleanly and pass 'installcheck'.

I rebased, fixed the "xid_snapshot_xip" problem spotted by Takao Fujii
that I had missed earlier, updated a couple of error messages to refer
to the new names (even when using the old functions) and ran
check-world and some simple manual tests on an -m32 build just to be
paranoid.  Here are the versions of these patches I'd like to commit.
Does anyone want to object to the txid/xid8 type punning scheme or
long term txid-sunsetting plan?

On 2020/01/28 14:05, Thomas Munro wrote:
> On Tue, Dec 3, 2019 at 6:56 AM Mark Dilger <hornschnorter@gmail.com> wrote:
>> On 11/30/19 5:14 PM, Thomas Munro wrote:
>>> On Sun, Dec 1, 2019 at 12:22 PM Mark Dilger <hornschnorter@gmail.com> wrote:
>>>> These two patches (v3) no longer apply cleanly.  Could you please
>>>> rebase?
>>>
>>> Hi Mark,
>>> Thanks.  Here's v4.
>>
>> Thanks, Thomas.
>>
>> The new patches apply cleanly and pass 'installcheck'.
> 
> I rebased, fixed the "xid_snapshot_xip" problem spotted by Takao Fujii
> that I had missed earlier, updated a couple of error messages to refer
> to the new names (even when using the old functions) and ran
> check-world and some simple manual tests on an -m32 build just to be
> paranoid.  Here are the versions of these patches I'd like to commit.

Thanks for the patches! Here are my minor comments.

Isn't it better to add also xid8lt, xid8gt, xid8le, and xid8ge?

xid8 and xid8_snapshot should be documented in datatype.sgml like
txid_snapshot is?

logicaldecoding.sgml and monitoring.sgml still referred to txid_xxx.
They should be updated so that new xid8_xxx is used?

In func.sgml, the table "Snapshot Components" is described still based
on txid. It should be updated so that it uses xid8, instead?

+# xid_ops
+{ amopfamily => 'hash/xid8_ops', amoplefttype => 'xid8', amoprighttype => 'xid8',
+  amopstrategy => '1', amopopr => '=(xid8,xid8)', amopmethod => 'hash' },

"xid_ops" in the comment should be "xid8_ops".

+{ oid => '9558',
+  proname => 'xid8neq', proleakproof => 't', prorettype => 'bool',
+  proargtypes => 'xid8 xid8', prosrc => 'xid8neq' },

Basically the names of not-equal functions for most data types are
something like "xxxne" not "xxxneq". So IMO it's better to use the name
"xid8ne" instead of "xid8neq" here.

  /*
- * do a TransactionId -> txid conversion for an XID near the given epoch
+ * Do a TransactionId -> fxid conversion for an XID that is known to precede
+ * the given 'next_fxid'.
   */
-static txid
-convert_xid(TransactionId xid, const TxidEpoch *state)
+static FullTransactionId
+convert_xid(TransactionId xid, FullTransactionId next_fxid)

As the comment suggests, this function assumes that "xid" must
precede "next_fxid". But there is no check for the assumption.
Isn't it better to add, e.g., an assertion checking that?
Or convert_xid() should handle the case where "xid" follows
"next_fxid" like the orignal convert_xid() does. That is, don't
apply the following change.

-    if (xid > state->last_xid &&
-        TransactionIdPrecedes(xid, state->last_xid))
+    if (xid > next_xid)
          epoch--;
-    else if (xid < state->last_xid &&
-             TransactionIdFollows(xid, state->last_xid))
-        epoch++;

> Does anyone want to object to the txid/xid8 type punning scheme or
> long term txid-sunsetting plan?

No. +1 to retire txid someday.

Regards,

-- 
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters

Hello Fujii-san,

Thanks for your review!

On Wed, Jan 29, 2020 at 12:43 AM Fujii Masao
<masao.fujii@oss.nttdata.com> wrote:
> Isn't it better to add also xid8lt, xid8gt, xid8le, and xid8ge?

xid doesn't have these operators, probably to avoid confusion about
wraparound.  But you're right, we should add them for xid8, especially
since the xid_snapshot documentation mentions such comparisons (the
type used by the old functions was int8, so that worked).  Done.  I
also added the extra catalogue nuts and bolts required to use xid8 in
btree indexes and merge joins.

To test the operators, I added a new regression test for xid and xid8
types.  While doing that, I tried to add some range checks to validate
input, but I discovered that it's a bit tricky to do so portably with
strtoul().  I suspect '0x100000000'::xid already gives different
results on Windows and Unix today, and if better input validation is
desired, I think it should be tackled outside this project.

While working on those tests, I realised that we probably wanted two
sets of tests:

1.  txid.sql: The existing tests that show that the old txid_XXX()
functions continue to work correctly (with the only user-visible
difference being that in their error messages they sometimes mention
names including xid8).  This test will be dropped when the txid_XXX()
functions are dropped.

2.  xid.sql: A new set of tests that show that the new xid8_XXX()
functions work correctly.

To verify that the old tests and the new tests are exactly the same
except for typenames and some casts, use:

diff -u src/test/regress/expected/txid.out src/test/regress/expected/xid.out

> xid8 and xid8_snapshot should be documented in datatype.sgml like
> txid_snapshot is?

Done.

> logicaldecoding.sgml and monitoring.sgml still referred to txid_xxx.
> They should be updated so that new xid8_xxx is used?

Done.

> In func.sgml, the table "Snapshot Components" is described still based
> on txid. It should be updated so that it uses xid8, instead?

Done.

> +# xid_ops
> +{ amopfamily => 'hash/xid8_ops', amoplefttype => 'xid8', amoprighttype => 'xid8',
> +  amopstrategy => '1', amopopr => '=(xid8,xid8)', amopmethod => 'hash' },
>
> "xid_ops" in the comment should be "xid8_ops".

Fixed.

> +{ oid => '9558',
> +  proname => 'xid8neq', proleakproof => 't', prorettype => 'bool',
> +  proargtypes => 'xid8 xid8', prosrc => 'xid8neq' },
>
> Basically the names of not-equal functions for most data types are
> something like "xxxne" not "xxxneq". So IMO it's better to use the name
> "xid8ne" instead of "xid8neq" here.

Huh.  You are right, but the existing function xidneq is an exception.
It's not clear which one to follow.  I will take your advice and use
xid8ne.  We could potentially change xidneq to xidne too, but it's
user-visible.

>   /*
> - * do a TransactionId -> txid conversion for an XID near the given epoch
> + * Do a TransactionId -> fxid conversion for an XID that is known to precede
> + * the given 'next_fxid'.
>    */
> -static txid
> -convert_xid(TransactionId xid, const TxidEpoch *state)
> +static FullTransactionId
> +convert_xid(TransactionId xid, FullTransactionId next_fxid)
>
> As the comment suggests, this function assumes that "xid" must
> precede "next_fxid". But there is no check for the assumption.
> Isn't it better to add, e.g., an assertion checking that?
> Or convert_xid() should handle the case where "xid" follows
> "next_fxid" like the orignal convert_xid() does. That is, don't
> apply the following change.
>
> -       if (xid > state->last_xid &&
> -               TransactionIdPrecedes(xid, state->last_xid))
> +       if (xid > next_xid)
>                 epoch--;
> -       else if (xid < state->last_xid &&
> -                        TransactionIdFollows(xid, state->last_xid))
> -               epoch++;

I need to think about this part some more, but I wanted to share
responses to the rest of your review now.  I'll return to this point
next week.

> > Does anyone want to object to the txid/xid8 type punning scheme or
> > long term txid-sunsetting plan?
>
> No. +1 to retire txid someday.

Cool.  Let's do that in a couple of years.

On Fri, Feb 14, 2020 at 6:31 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> On Wed, Jan 29, 2020 at 12:43 AM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
> >   /*
> > - * do a TransactionId -> txid conversion for an XID near the given epoch
> > + * Do a TransactionId -> fxid conversion for an XID that is known to precede
> > + * the given 'next_fxid'.
> >    */
> > -static txid
> > -convert_xid(TransactionId xid, const TxidEpoch *state)
> > +static FullTransactionId
> > +convert_xid(TransactionId xid, FullTransactionId next_fxid)
> >
> > As the comment suggests, this function assumes that "xid" must
> > precede "next_fxid". But there is no check for the assumption.
> > Isn't it better to add, e.g., an assertion checking that?
> > Or convert_xid() should handle the case where "xid" follows
> > "next_fxid" like the orignal convert_xid() does. That is, don't
> > apply the following change.
> >
> > -       if (xid > state->last_xid &&
> > -               TransactionIdPrecedes(xid, state->last_xid))
> > +       if (xid > next_xid)
> >                 epoch--;
> > -       else if (xid < state->last_xid &&
> > -                        TransactionIdFollows(xid, state->last_xid))
> > -               epoch++;

I don't think it is reachable.  I have renamed the function to
widen_snapshot_xid() and rewritten the comments to explain the logic.

The other changes in this version:

* updated OIDs to avoid collisions
* added btequalimage to btree/xid8_ops

On Sat, Mar 21, 2020 at 11:14 AM Thomas Munro <thomas.munro@gmail.com> wrote:
> * updated OIDs to avoid collisions
> * added btequalimage to btree/xid8_ops

Here's the version I'm planning to commit tomorrow, if no one objects.  Changes:

* txid.c renamed to xid8funcs.c
* remaining traces of "txid" replaced various internal identifiers
* s/backwards compatible/backward compatible/ in funcs.sgml (en_GB -> en_US)

> On Apr 1, 2020, at 8:21 PM, Thomas Munro <thomas.munro@gmail.com> wrote:
>
> On Sat, Mar 21, 2020 at 11:14 AM Thomas Munro <thomas.munro@gmail.com> wrote:
>> * updated OIDs to avoid collisions
>> * added btequalimage to btree/xid8_ops
>
> Here's the version I'm planning to commit tomorrow, if no one objects.  Changes:
>
> * txid.c renamed to xid8funcs.c
> * remaining traces of "txid" replaced various internal identifiers
> * s/backwards compatible/backward compatible/ in funcs.sgml (en_GB -> en_US)
>
<v8-0001-Add-SQL-type-xid8-to-expose-FullTransactionId-to-.patch><v8-0002-Introduce-xid8_XXX-functions-to-replace-txid_XXX.patch>

Hi Thomas, Thanks for working on this.

I'm taking a quick look at your patches.  It's not a big deal, and certainly not a show stopper if you want to go ahead
withthe commit, but you've left some mentions of "txid_current" that might better be modified to use the new name
"xid8_current". At least one mention of "txid_current" is needed to check that the old name still works, but leaving
thismany in the regression test suite may lead other developers to follow that lead and use txid_current() in newly
developedcode.  ("xid8_current" is not exercised by name anywhere in the regression suite, that I can see.) 

> contrib/test_decoding/expected/ddl.out:SELECT txid_current() != 0; -- so no fixed xid apears in the outfile
> contrib/test_decoding/expected/decoding_in_xact.out:SELECT txid_current() = 0;
> contrib/test_decoding/expected/decoding_in_xact.out:SELECT txid_current() = 0;
> contrib/test_decoding/expected/decoding_in_xact.out:SELECT txid_current() = 0;
> contrib/test_decoding/expected/oldest_xmin.out:step s0_getxid: SELECT txid_current() IS NULL;
> contrib/test_decoding/expected/ondisk_startup.out:step s2txid: SELECT txid_current() IS NULL;
> contrib/test_decoding/expected/ondisk_startup.out:step s3txid: SELECT txid_current() IS NULL;
> contrib/test_decoding/expected/ondisk_startup.out:step s2txid: SELECT txid_current() IS NULL;
> contrib/test_decoding/expected/snapshot_transfer.out:step s0_log_assignment: SELECT txid_current() IS NULL;
> contrib/test_decoding/expected/snapshot_transfer.out:step s0_log_assignment: SELECT txid_current() IS NULL;
> contrib/test_decoding/specs/oldest_xmin.spec:step "s0_getxid" { SELECT txid_current() IS NULL; }
> contrib/test_decoding/specs/ondisk_startup.spec:step "s2txid" { SELECT txid_current() IS NULL; }
> contrib/test_decoding/specs/ondisk_startup.spec:step "s3txid" { SELECT txid_current() IS NULL; }
> contrib/test_decoding/specs/snapshot_transfer.spec:step "s0_log_assignment" { SELECT txid_current() IS NULL; }
> contrib/test_decoding/sql/ddl.sql:SELECT txid_current() != 0; -- so no fixed xid apears in the outfile
> contrib/test_decoding/sql/decoding_in_xact.sql:SELECT txid_current() = 0;
> contrib/test_decoding/sql/decoding_in_xact.sql:SELECT txid_current() = 0;
> contrib/test_decoding/sql/decoding_in_xact.sql:SELECT txid_current() = 0;
>
> src/test/modules/commit_ts/t/004_restart.pl:    SELECT txid_current();
> src/test/modules/commit_ts/t/004_restart.pl:            EXECUTE 'SELECT txid_current()';
> src/test/modules/commit_ts/t/004_restart.pl:    SELECT txid_current();
> src/test/recovery/t/003_recovery_targets.pl:    "SELECT pg_current_wal_lsn(), txid_current();");
> src/test/recovery/t/011_crash_recovery.pl:SELECT txid_current();
> src/test/recovery/t/011_crash_recovery.pl:cmp_ok($node->safe_psql('postgres', 'SELECT txid_current()'),
> src/test/regress/expected/alter_table.out:        where transactionid = txid_current()::integer)
> src/test/regress/expected/alter_table.out:        where transactionid = txid_current()::integer)
> src/test/regress/expected/hs_standby_functions.out:select txid_current();
> src/test/regress/expected/hs_standby_functions.out:ERROR:  cannot execute txid_current() during recovery
> src/test/regress/expected/hs_standby_functions.out:select length(txid_current_snapshot()::text) >= 4;
> src/test/regress/expected/txid.out:select txid_current() >= txid_snapshot_xmin(txid_current_snapshot());
> src/test/regress/expected/txid.out:select txid_visible_in_snapshot(txid_current(), txid_current_snapshot());
> src/test/regress/expected/txid.out:-- test txid_current_if_assigned
> src/test/regress/expected/txid.out:SELECT txid_current_if_assigned() IS NULL;
> src/test/regress/expected/txid.out:SELECT txid_current() \gset
> src/test/regress/expected/txid.out:SELECT txid_current_if_assigned() IS NOT DISTINCT FROM BIGINT :'txid_current';
> src/test/regress/expected/txid.out:SELECT txid_current() AS committed \gset
> src/test/regress/expected/txid.out:SELECT txid_current() AS rolledback \gset
> src/test/regress/expected/txid.out:SELECT txid_current() AS inprogress \gset
> src/test/regress/expected/update.out:CREATE FUNCTION xid_current() RETURNS xid LANGUAGE SQL AS $$SELECT
(txid_current()% ((1::int8<<32)))::text::xid;$$; 
> src/test/regress/sql/alter_table.sql:        where transactionid = txid_current()::integer)
> src/test/regress/sql/alter_table.sql:        where transactionid = txid_current()::integer)
> src/test/regress/sql/hs_standby_functions.sql:select txid_current();
> src/test/regress/sql/hs_standby_functions.sql:select length(txid_current_snapshot()::text) >= 4;
> src/test/regress/sql/txid.sql:select txid_current() >= txid_snapshot_xmin(txid_current_snapshot());
> src/test/regress/sql/txid.sql:select txid_visible_in_snapshot(txid_current(), txid_current_snapshot());
> src/test/regress/sql/txid.sql:-- test txid_current_if_assigned
> src/test/regress/sql/txid.sql:SELECT txid_current_if_assigned() IS NULL;
> src/test/regress/sql/txid.sql:SELECT txid_current() \gset
> src/test/regress/sql/txid.sql:SELECT txid_current_if_assigned() IS NOT DISTINCT FROM BIGINT :'txid_current';
> src/test/regress/sql/txid.sql:SELECT txid_current() AS committed \gset
> src/test/regress/sql/txid.sql:SELECT txid_current() AS rolledback \gset
> src/test/regress/sql/txid.sql:SELECT txid_current() AS inprogress \gset
> src/test/regress/sql/update.sql:CREATE FUNCTION xid_current() RETURNS xid LANGUAGE SQL AS $$SELECT (txid_current() %
((1::int8<<32)))::text::xid;$$;

A reasonable argument could be made for treating txid_current as the preferred form, and xid8_current merely as a
synonym,but then I can't make sense of the change your patch makes to the docs: 

+   <para>
+    In releases of <productname>PostgreSQL</productname> before 13 there was
+    no <type>xid8</type> type, so variants of these functions were provided
+    that used <type>bigint</type>.  The older functions with
+    <literal>txid</literal>
+    in the name are still supported for backward compatibility, but may be
+    removed from a future release.  The <type>bigint</type> variants are shown
+    in <xref linkend="functions-txid-snapshot"/>.
+   </para>

which looks like a txid deprecation warning to me.

Am I reading all this wrong?  If I'm reading this right, then FYI there are similar s/txid_(.*)/xid8_$1/g changes to be
madethat I didn't bother listing here by name. 

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

> On Apr 2, 2020, at 9:06 AM, Mark Dilger <mark.dilger@enterprisedb.com> wrote:
>
> ("xid8_current" is not exercised by name anywhere in the regression suite, that I can see.)

I spoke too soon.  That's exercised in the new xid.sql test file. It didn't show up in my 'git diff', because it's new.
Sorry about that. 

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2020-Apr-02, Thomas Munro wrote:

> On Sat, Mar 21, 2020 at 11:14 AM Thomas Munro <thomas.munro@gmail.com> wrote:
> > * updated OIDs to avoid collisions
> > * added btequalimage to btree/xid8_ops
> 
> Here's the version I'm planning to commit tomorrow, if no one objects.  Changes:
> 
> * txid.c renamed to xid8funcs.c
> * remaining traces of "txid" replaced various internal identifiers
> * s/backwards compatible/backward compatible/ in funcs.sgml (en_GB -> en_US)

Hmm, for some reason I had it in my head that we would make these use an
"epoch/val" output format rather than raw uint64 values.  Are we really
going to do it this way?  Myself I can convert values easily enough, but
I'm not sure this is user-friendly.  (If somebody were to tell me that
LSNs are going to be straight uint64 values, I would not be happy.)

Or maybe it's the other way around -- this is fine for everyone except
me -- and we should never expose the epoch as a separate quantity.  

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Hi,

On 2020-04-02 14:33:18 -0300, Alvaro Herrera wrote:
> On 2020-Apr-02, Thomas Munro wrote:
> 
> > On Sat, Mar 21, 2020 at 11:14 AM Thomas Munro <thomas.munro@gmail.com> wrote:
> > > * updated OIDs to avoid collisions
> > > * added btequalimage to btree/xid8_ops
> > 
> > Here's the version I'm planning to commit tomorrow, if no one objects.  Changes:
> > 
> > * txid.c renamed to xid8funcs.c
> > * remaining traces of "txid" replaced various internal identifiers
> > * s/backwards compatible/backward compatible/ in funcs.sgml (en_GB -> en_US)
> 
> Hmm, for some reason I had it in my head that we would make these use an
> "epoch/val" output format rather than raw uint64 values.

Why would we do that? IMO the goal should be to reduce awareness of the
32bitness of normal xids from as many places as possible, and treat them
as an internal space optimization.

Greetings,

Andres Freund

> On Apr 2, 2020, at 11:01 AM, Andres Freund <andres@anarazel.de> wrote:
>
>>
>> Hmm, for some reason I had it in my head that we would make these use an
>> "epoch/val" output format rather than raw uint64 values.
>
> Why would we do that? IMO the goal should be to reduce awareness of the
> 32bitness of normal xids from as many places as possible, and treat them
> as an internal space optimization.

I agree with transitioning to 64-bit xids with 32 bit xid/epoch pairs as an internal implementation and storage detail
only,but we still have user facing views that don't treat it that way.   pg_stat_get_activity still returns backend_xid
andbackend_xmin as 32-bit, not 64-bit.  Should this function change to be consistent?  I'm curious what the user
experiencewill be during the transitional period where some user facing xids are 64 bit and others (perhaps the same
xidsbut viewed elsewhere) will be 32 bit.  That might make it difficult for users to match them up. 

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Andres Freund <andres@anarazel.de> writes:
> On 2020-04-02 14:33:18 -0300, Alvaro Herrera wrote:
>> Hmm, for some reason I had it in my head that we would make these use an
>> "epoch/val" output format rather than raw uint64 values.

> Why would we do that? IMO the goal should be to reduce awareness of the
> 32bitness of normal xids from as many places as possible, and treat them
> as an internal space optimization.

If they're just int64s then you don't need special functions to do
things like finding the min or max in a column of them.

            regards, tom lane

On Thu, Apr 2, 2020 at 2:47 PM Mark Dilger <mark.dilger@enterprisedb.com> wrote:
>
>
>
> > On Apr 2, 2020, at 11:01 AM, Andres Freund <andres@anarazel.de> wrote:
> >
> >>
> >> Hmm, for some reason I had it in my head that we would make these use an
> >> "epoch/val" output format rather than raw uint64 values.
> >
> > Why would we do that? IMO the goal should be to reduce awareness of the
> > 32bitness of normal xids from as many places as possible, and treat them
> > as an internal space optimization.
>
> I agree with transitioning to 64-bit xids with 32 bit xid/epoch pairs as an internal implementation and storage
detailonly, but we still have user facing views that don't treat it that way.   pg_stat_get_activity still returns
backend_xidand backend_xmin as 32-bit, not 64-bit.  Should this function change to be consistent?  I'm curious what the
userexperience will be during the transitional period where some user facing xids are 64 bit and others (perhaps the
samexids but viewed elsewhere) will be 32 bit.  That might make it difficult for users to match them up. 


Agreed. The "benefit" (at least in the short term) of using the
epoch/value style is that it makes (visual, at least) comparison with
other (32-bit) xid values easier.

I'm not sure if that's worth it, or if it's worth making a change
depend on changing all of those views too.

James

Hi,

On 2020-04-02 11:47:32 -0700, Mark Dilger wrote:
> I agree with transitioning to 64-bit xids with 32 bit xid/epoch pairs
> as an internal implementation and storage detail only, but we still
> have user facing views that don't treat it that way.

Note that epochs are not really a thing internally anymore. The xid
counter is a FullTransactionId.


> pg_stat_get_activity still returns backend_xid and backend_xmin as
> 32-bit, not 64-bit.  Should this function change to be consistent?  I'm
> curious what the user experience will be during the transitional period
> where some user facing xids are 64 bit and others (perhaps the same xids
> but viewed elsewhere) will be 32 bit.  That might make it difficult for
> users to match them up.

I think we probably should switch them over at some point, but I would
strongly advise against coupling that with Thomas' patch. That patch
doesn't make the current situation around 32bit / 64bit any worse, as
far as I can tell.

Given that txid_current() "always" has been a plain 64 bit integer, and
the various txid_* functions always have returned 64 bit integers, I
really don't think arguing for some 32bit/32bit situation now makes
sense.

Greetings,

Andres Freund

> On Apr 2, 2020, at 2:13 PM, Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2020-04-02 11:47:32 -0700, Mark Dilger wrote:
>> I agree with transitioning to 64-bit xids with 32 bit xid/epoch pairs
>> as an internal implementation and storage detail only, but we still
>> have user facing views that don't treat it that way.
>
> Note that epochs are not really a thing internally anymore. The xid
> counter is a FullTransactionId.
>
>
>> pg_stat_get_activity still returns backend_xid and backend_xmin as
>> 32-bit, not 64-bit.  Should this function change to be consistent?  I'm
>> curious what the user experience will be during the transitional period
>> where some user facing xids are 64 bit and others (perhaps the same xids
>> but viewed elsewhere) will be 32 bit.  That might make it difficult for
>> users to match them up.
>
> I think we probably should switch them over at some point, but I would
> strongly advise against coupling that with Thomas' patch. That patch
> doesn't make the current situation around 32bit / 64bit any worse, as
> far as I can tell.

I agree with that.

> Given that txid_current() "always" has been a plain 64 bit integer, and
> the various txid_* functions always have returned 64 bit integers, I
> really don't think arguing for some 32bit/32bit situation now makes
> sense.

Yeah, I'm not arguing for that, though I can see how my email might have been ambiguous on that point.

Since Thomas's patch is really just focused on transitioning from txid -> xid8, I think this conversation is a bit
beyondscope for this patch, except that "xid8" sounds an awful lot like the new type that all user facing xid output
willtransition to.  Maybe I'm wrong about that.   Are we going to change the definition of the "xid" type to 8 bytes?
Thatsounds dangerous, from a compatibility standpoint. 

On balance, I'd rather have xid8in and xid8out work just as Thomas has it.  I'm not asking for any change there.  But
I'mcurious if the whole community is on the same page regarding where this is all heading. 

I'm contemplating the statement that "the goal should be to reduce awareness of the 32bitness of normal xids from as
manyplaces as possible", which I support, and what that means for the eventual signatures of functions like
pg_stat_get_activity,including: 

    (..., backend_xid XID, backend_xminxid XID, ...) pg_stat_get_activity(pid INTEGER)

    (..., transactionid XID, ...) pg_lock_status()

    (transaction XID, ...) pg_prepared_xact()

    timestamptz pg_xact_commit_timestamp(XID)

    (xid XID, ...) pg_last_committed_xact()

    (..., xmin XID, catalog_xmin XID, ...) pg_get_replication_slots()

    ... more that I'm too lazy to copy-and-paste just now ...

I would expect that, eventually, these would be upgraded to xid8.  If that happened seemlessly in one release, then
therewould be no problem with some functions returning 4-byte xids and some returning 8-byte xids, but otherwise there
wouldbe a transition period where some have been reworked to return xid8 but others not, and users during that
transitionperiod might be happier with Alvaro's suggestion of treating epoch/xid as two fields in xid8in and xid8out.
I'malso doubtful that these functions would be "upgraded".  It seems far more likely that alternate versions, perhaps
namedsomething with /xid8/ in them, would exist side-by-side with the originals. 

So I'm really just wondering where others on this list think all this is heading, and if there are any arguments
brewingabout that which could be avoided by making assumptions clear right up front. 


—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi,

On 2020-04-02 14:26:41 -0700, Mark Dilger wrote:
> Since Thomas's patch is really just focused on transitioning from txid
> -> xid8, I think this conversation is a bit beyond scope for this
> patch, except that "xid8" sounds an awful lot like the new type that
> all user facing xid output will transition to.  Maybe I'm wrong about
> that.

Several at least. For me it'd e.g. make no sense to change pageinspect
etc.


> Are we going to change the definition of the "xid" type to 8 bytes?
> That sounds dangerous, from a compatibility standpoint.

No, I can't see that happening.


> On balance, I'd rather have xid8in and xid8out work just as Thomas has
> it.  I'm not asking for any change there.  But I'm curious if the
> whole community is on the same page regarding where this is all
> heading.
>
> I'm contemplating the statement that "the goal should be to reduce
> awareness of the 32bitness of normal xids from as many places as
> possible", which I support, and what that means for the eventual
> signatures of functions like pg_stat_get_activity, including:

Maybe. Aiming to do things like this all-at-once just makes it less
likely for anything to ever happen.


> but otherwise there would be a transition period where some have been
> reworked to return xid8 but others not, and users during that
> transition period might be happier with Alvaro's suggestion of
> treating epoch/xid as two fields in xid8in and xid8out.

-countless

I can only restate my point that we've had 8 byte xids exposed for many
years. We've had very few epoch/xid values exposed. I think it'd be
insane to now start to expose that more widely.

It's just about impossible for normal users to compare xids. Once one
wrapped around, it's just too hard/mindbending. Yes, an accompanying
epoch makes it easier, but it still can be quite confusing.

Greetings,

Andres Freund

On Fri, Apr 3, 2020 at 10:37 AM Andres Freund <andres@anarazel.de> wrote:
> On 2020-04-02 14:26:41 -0700, Mark Dilger wrote:
> > On balance, I'd rather have xid8in and xid8out work just as Thomas has
> > it.  I'm not asking for any change there.  But I'm curious if the
> > whole community is on the same page regarding where this is all
> > heading.
> >
> > I'm contemplating the statement that "the goal should be to reduce
> > awareness of the 32bitness of normal xids from as many places as
> > possible", which I support, and what that means for the eventual
> > signatures of functions like pg_stat_get_activity, including:
>
> Maybe. Aiming to do things like this all-at-once just makes it less
> likely for anything to ever happen.

Agreed.  Let's just keep chipping away at this stuff.

> > but otherwise there would be a transition period where some have been
> > reworked to return xid8 but others not, and users during that
> > transition period might be happier with Alvaro's suggestion of
> > treating epoch/xid as two fields in xid8in and xid8out.
>
> -countless
>
> I can only restate my point that we've had 8 byte xids exposed for many
> years. We've had very few epoch/xid values exposed. I think it'd be
> insane to now start to expose that more widely.
>
> It's just about impossible for normal users to compare xids. Once one
> wrapped around, it's just too hard/mindbending. Yes, an accompanying
> epoch makes it easier, but it still can be quite confusing.

Just by the way, any xid8 values can be sliced with ::xid, so that
should help with comparisons.  I'm not keen to allow users to convert
in the other direction though, due to the hard-to-understand
interlocking requirements of modulo xids (as belaboured elsewhere).

As Mark noted, I'd left a few uses of txid_XXX stuff in other tests.
So here's a 0003 patch that upgrades all of those too, so that the
only remaining usage is in the txid.sql tests (= the tests that the
backwards compat functions still work).  No change to 0001 and 0002,
other than a commit message tweak (reviewer email address change).

> On Apr 2, 2020, at 7:39 PM, Thomas Munro <thomas.munro@gmail.com> wrote:
>
>
<v9-0001-Add-SQL-type-xid8-to-expose-FullTransactionId-to-.patch><v9-0002-Introduce-xid8_XXX-functions-to-replace-txid_XXX.patch><v9-0003-Replace-all-txid_XXX-usage-in-tests-with-xid8_XXX.patch>

These apply cleanly, build and pass check-world on mac, and the documentation and regression test changes surrounding
txidlook good to me. 

FYI, (not the responsibility of this patch), we never quite define what the abbreviation "xip" stands for.  If "Active
xid8sat the time of the snapshot." were rewritten as "In progress xid8s at the time of the snapshot", it might be
slightlyeasier for the reader to figure out that "xip" = "Xid8s In Progress".  As it stands, nothing in the docs seems
toexplain the abbrevation.  See doc/src/sgml/func.sgml 

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Sat, Apr 4, 2020 at 4:45 AM Mark Dilger <mark.dilger@enterprisedb.com> wrote:
> FYI, (not the responsibility of this patch), we never quite define what the abbreviation "xip" stands for.  If
"Activexid8s at the time of the snapshot." were rewritten as "In progress xid8s at the time of the snapshot", it might
beslightly easier for the reader to figure out that "xip" = "Xid8s In Progress".  As it stands, nothing in the docs
seemsto explain the abbrevation.  See doc/src/sgml/func.sgml 

You're right.  Done.

I also noticed that "xid8s" didn't flow very well here, so I changed
it to "transaction IDs" in a couple of places like that (which I think
is fine in these English sentences, to mean xid, xid8 or bigint
depending on the context).

I also removed a sentence about values being "extended with an epoch",
because that's not really how we want people to think about this stuff
anymore.  It's rather the other way around: transaction IDs begin life
as 64 bit numbers and then get sliced.

I noticed that the description of xmax was flat out wrong (it didn't
know about ancient commit 6bd4f401), so I rewrote it.  And then while
exercising my backspace key, it bothered me that the description of
xip_list said almost the same thing twice so I kept only the more
accurate of two sentences.

However, I am getting cold feet about the new function names.  The
existing naming structure made sense when all this stuff originated in
a contrib module with "txid_" as a prefix all over the place, but now
that 64 bit IDs are a core concept, I wonder if we shouldn't aim for
something that looks a little more like core functionality and doesn't
have those "xid8_" warts in the names.  Here's what I now propose:

Transaction ID functions, using names that fit with others (cf
pg_xact_commit_timestamp()):

  pg_current_xact_id()
  pg_current_xact_id_if_assigned()
  pg_xact_status(xid8)

Snapshot functions (cf pg_export_snapshot()):

  pg_current_snapshot()
  pg_snapshot_xmin(pg_snapshot)
  pg_snapshot_xmax(pg_snapshot)
  pg_snapshot_xip(pg_snapshot)
  pg_visible_in_snapshot(xid8, pg_snapshot)

Here's a patch set like that (0003 has been squashed into 0002).

> On Apr 4, 2020, at 7:11 AM, Thomas Munro <thomas.munro@gmail.com> wrote:
>
> On Sat, Apr 4, 2020 at 4:45 AM Mark Dilger <mark.dilger@enterprisedb.com> wrote:
>> FYI, (not the responsibility of this patch), we never quite define what the abbreviation "xip" stands for.  If
"Activexid8s at the time of the snapshot." were rewritten as "In progress xid8s at the time of the snapshot", it might
beslightly easier for the reader to figure out that "xip" = "Xid8s In Progress".  As it stands, nothing in the docs
seemsto explain the abbrevation.  See doc/src/sgml/func.sgml 
>
> You're right.  Done.

Thanks!

> However, I am getting cold feet about the new function names.  The
> existing naming structure made sense when all this stuff originated in
> a contrib module with "txid_" as a prefix all over the place, but now
> that 64 bit IDs are a core concept, I wonder if we shouldn't aim for
> something that looks a little more like core functionality and doesn't
> have those "xid8_" warts in the names.

The "xid8_" warts are partly motivated by having a type named "xid8", which is a bit of a wart in itself.

> Here's what I now propose:
>
> Transaction ID functions, using names that fit with others (cf
> pg_xact_commit_timestamp()):
>
>  pg_current_xact_id()
>  pg_current_xact_id_if_assigned()
>  pg_xact_status(xid8)
>
> Snapshot functions (cf pg_export_snapshot()):
>
>  pg_current_snapshot()
>  pg_snapshot_xmin(pg_snapshot)
>  pg_snapshot_xmax(pg_snapshot)
>  pg_snapshot_xip(pg_snapshot)
>  pg_visible_in_snapshot(xid8, pg_snapshot)

I like some aspects of this, but not others.  Function pg_stat_get_activity(), which gets exposed through view
pg_stat_activityexposes both "backend_xid" and "backend_xmin" as (32-bit) xid.  Your new function names are not
sufficientlydistinct from these older names for users to easily remember the difference: 

select pg_snapshot_xmax(st.snap)
    from snapshot_test st, pg_stat_activity sa
    where pg_snapshot_xmin(st.snap) = sa.backend_xmin;
ERROR:  operator does not exist: xid8 = xid

SELECT * FROM pg_stat_activity s WHERE backend_xid = pg_current_xact_id();
ERROR:  operator does not exist: xid = xid8

SELECT pg_xact_status(backend_xmin), * FROM pg_stat_activity;
ERROR:  function pg_xact_status(xid) does not exist

It's not the end of the world, and users can figure out to put a cast on those, but it's kind of ugly.

It was cleaner before v10, when "backend_xid = xid8_current()" clearly had a xid vs. xid8 mismatch. On the other hand,
ifthe xid columns in pg_stat_activity and elsewhere eventually get upgraded to xid8 fields, then the new naming
conventionin v10 will be cleaner. 

As such, I'm ±0 for the change.


—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Sun, Apr 5, 2020 at 10:34 AM Mark Dilger
<mark.dilger@enterprisedb.com> wrote:
> > On Apr 4, 2020, at 7:11 AM, Thomas Munro <thomas.munro@gmail.com> wrote:
> > However, I am getting cold feet about the new function names.  The
> > existing naming structure made sense when all this stuff originated in
> > a contrib module with "txid_" as a prefix all over the place, but now
> > that 64 bit IDs are a core concept, I wonder if we shouldn't aim for
> > something that looks a little more like core functionality and doesn't
> > have those "xid8_" warts in the names.
>
> The "xid8_" warts are partly motivated by having a type named "xid8", which is a bit of a wart in itself.

Just a thought for the future, not sure if it's a good one: would it
seem less warty in years to come if we introduced xid4 as an alias for
xid, and preferred the name xid4?  Then it wouldn't look so much like
xid is the "real" transaction ID type and xid8 is some kind of freaky
extended version; instead it would look like xid4 and xid8 are narrow
and wide transaction IDs, and xid is just a historical name for xid4.

> > Here's what I now propose:
> >
> > Transaction ID functions, using names that fit with others (cf
> > pg_xact_commit_timestamp()):
> >
> >  pg_current_xact_id()
> >  pg_current_xact_id_if_assigned()
> >  pg_xact_status(xid8)
> >
> > Snapshot functions (cf pg_export_snapshot()):
> >
> >  pg_current_snapshot()
> >  pg_snapshot_xmin(pg_snapshot)
> >  pg_snapshot_xmax(pg_snapshot)
> >  pg_snapshot_xip(pg_snapshot)
> >  pg_visible_in_snapshot(xid8, pg_snapshot)
>
> I like some aspects of this, but not others.  Function pg_stat_get_activity(), which gets exposed through view
pg_stat_activityexposes both "backend_xid" and "backend_xmin" as (32-bit) xid.  Your new function names are not
sufficientlydistinct from these older names for users to easily remember the difference: 
>
> select pg_snapshot_xmax(st.snap)
>     from snapshot_test st, pg_stat_activity sa
>     where pg_snapshot_xmin(st.snap) = sa.backend_xmin;
> ERROR:  operator does not exist: xid8 = xid
>
> SELECT * FROM pg_stat_activity s WHERE backend_xid = pg_current_xact_id();
> ERROR:  operator does not exist: xid = xid8

It's quite tempting to go and widen pg_stat_activity etc...  but in
any case I'm sure it'll happen for PG14.

> SELECT pg_xact_status(backend_xmin), * FROM pg_stat_activity;
> ERROR:  function pg_xact_status(xid) does not exist
>
> It's not the end of the world, and users can figure out to put a cast on those, but it's kind of ugly.

That particular one can't really be fixed with a cast, either before
or after this patch (I mean, if you add the right casts you can get
the query to run with this function or its txid ancestor, but it'll
only give the right answers during epoch 0 so I would call this
friction a good case of the type system doing its job during the
transition).

> It was cleaner before v10, when "backend_xid = xid8_current()" clearly had a xid vs. xid8 mismatch. On the other
hand,if the xid columns in pg_stat_activity and elsewhere eventually get upgraded to xid8 fields, then the new naming
conventionin v10 will be cleaner. 

Yeah.  Well, my cold feet with the v9 names came from thinking about
how all this is going to look in a couple of years as xid8 flows into
more administration interfaces.  It seems inevitable that there will
be some friction along the way, but it seems like a nice goal to have
wider values everywhere possible from functions and views with
non-warty names, and use cast to get narrow values if needed for some
reason.

> As such, I'm ±0 for the change.

I'll let this sit for another day and see if some more reactions appear.

On Sun, Apr 5, 2020 at 11:31 AM Thomas Munro <thomas.munro@gmail.com> wrote:
> On Sun, Apr 5, 2020 at 10:34 AM Mark Dilger
> <mark.dilger@enterprisedb.com> wrote:
> > The "xid8_" warts are partly motivated by having a type named "xid8", which is a bit of a wart in itself.
>
> Just a thought for the future, not sure if it's a good one: would it
> seem less warty in years to come if we introduced xid4 as an alias for
> xid, and preferred the name xid4?  Then it wouldn't look so much like
> xid is the "real" transaction ID type and xid8 is some kind of freaky
> extended version; instead it would look like xid4 and xid8 are narrow
> and wide transaction IDs, and xid is just a historical name for xid4.

I'll look into proposing that for PG14.  One reason I like that idea
is that system view names like backend_xid could potentially retain
their names while switching to xid8 type, (maybe?) breaking fewer
queries and avoiding ugly names, on the theory that _xid doesn't
specify whether it's xid4 or an xid8.

> > >  pg_current_xact_id()
> > >  pg_current_xact_id_if_assigned()
> > >  pg_xact_status(xid8)

> > >  pg_current_snapshot()
> > >  pg_snapshot_xmin(pg_snapshot)
> > >  pg_snapshot_xmax(pg_snapshot)
> > >  pg_snapshot_xip(pg_snapshot)
> > >  pg_visible_in_snapshot(xid8, pg_snapshot)

> > As such, I'm ±0 for the change.
>
> I'll let this sit for another day and see if some more reactions appear.

Hearing no objections, pushed.  Happy to reconsider these names before
release if someone finds a problem with this scheme.

> On Apr 6, 2020, at 5:14 PM, Thomas Munro <thomas.munro@gmail.com> wrote:
>
> On Sun, Apr 5, 2020 at 11:31 AM Thomas Munro <thomas.munro@gmail.com> wrote:
>> On Sun, Apr 5, 2020 at 10:34 AM Mark Dilger
>> <mark.dilger@enterprisedb.com> wrote:
>>> The "xid8_" warts are partly motivated by having a type named "xid8", which is a bit of a wart in itself.
>>
>> Just a thought for the future, not sure if it's a good one: would it
>> seem less warty in years to come if we introduced xid4 as an alias for
>> xid, and preferred the name xid4?  Then it wouldn't look so much like
>> xid is the "real" transaction ID type and xid8 is some kind of freaky
>> extended version; instead it would look like xid4 and xid8 are narrow
>> and wide transaction IDs, and xid is just a historical name for xid4.
>
> I'll look into proposing that for PG14.  One reason I like that idea
> is that system view names like backend_xid could potentially retain
> their names while switching to xid8 type, (maybe?) breaking fewer
> queries and avoiding ugly names, on the theory that _xid doesn't
> specify whether it's xid4 or an xid8.
>
>>>> pg_current_xact_id()
>>>> pg_current_xact_id_if_assigned()
>>>> pg_xact_status(xid8)
>
>>>> pg_current_snapshot()
>>>> pg_snapshot_xmin(pg_snapshot)
>>>> pg_snapshot_xmax(pg_snapshot)
>>>> pg_snapshot_xip(pg_snapshot)
>>>> pg_visible_in_snapshot(xid8, pg_snapshot)
>
>>> As such, I'm ±0 for the change.
>>
>> I'll let this sit for another day and see if some more reactions appear.
>
> Hearing no objections, pushed.  Happy to reconsider these names before
> release if someone finds a problem with this scheme.

Hmm, I should have spoken sooner...

src/backend/replication/walsender.c:static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
src/backend/utils/adt/xid8funcs.c:TransactionIdInRecentPast(FullTransactionId fxid, TransactionId *extracted_xid)

I don't care much for having two different functions with the same name and related semantics but different argument
types.


—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, Apr 17, 2020 at 3:44 AM Mark Dilger
<mark.dilger@enterprisedb.com> wrote:
> Hmm, I should have spoken sooner...
>
> src/backend/replication/walsender.c:static bool TransactionIdInRecentPast(TransactionId xid, uint32 epoch);
> src/backend/utils/adt/xid8funcs.c:TransactionIdInRecentPast(FullTransactionId fxid, TransactionId *extracted_xid)
>
> I don't care much for having two different functions with the same name and related semantics but different argument
types.

Maybe that's not ideal, but it's not because of this patch.  Those
functions are from 5737c12df05 and 857ee8e391f.

On Thu, Apr 2, 2020 at 5:13 PM Andres Freund <andres@anarazel.de> wrote:
> Given that txid_current() "always" has been a plain 64 bit integer, and
> the various txid_* functions always have returned 64 bit integers, I
> really don't think arguing for some 32bit/32bit situation now makes
> sense.

I'm not sure what the best thing to do is here, but the reality is
that there are many places where 32-bit XIDs are going to be showing
up for years to come. With the format printed as a raw 64-bit
quantity, people troubleshooting stuff are going to spend a lot of
time figuring what x%2^32 is. And I can't do that in my head. So I
think saying that the proposal does not makes sense is a gross
overstatement. It may not be what we want to do. But it definitely
would make sense.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi,

On 2020-04-17 13:33:53 -0400, Robert Haas wrote:
> On Thu, Apr 2, 2020 at 5:13 PM Andres Freund <andres@anarazel.de> wrote:
> > Given that txid_current() "always" has been a plain 64 bit integer, and
> > the various txid_* functions always have returned 64 bit integers, I
> > really don't think arguing for some 32bit/32bit situation now makes
> > sense.
> 
> I'm not sure what the best thing to do is here, but the reality is
> that there are many places where 32-bit XIDs are going to be showing
> up for years to come. With the format printed as a raw 64-bit
> quantity, people troubleshooting stuff are going to spend a lot of
> time figuring what x%2^32 is. And I can't do that in my head. So I
> think saying that the proposal does not makes sense is a gross
> overstatement. It may not be what we want to do. But it definitely
> would make sense.

You seem to be entirely disregarding my actual point, namely that
txid_current(), as well as some other txid_* functions, have returned
64bit xids for many many years. txid_current() is the only function to
get the current xid in a reasonable way. I don't understand how a
proposal to add a 32/32 bit representation *in addition* to the existing
32 and 64bit representations is going to improve the situation. Nor do I
see changing txid_current()'s return format as something we're going to
go for.

I did not argue against a function to turn 64bit xids into epoch/32bit
xid or such.

?

Greetings,

Andres Freund

On Fri, Apr 17, 2020 at 1:45 PM Andres Freund <andres@anarazel.de> wrote:
> You seem to be entirely disregarding my actual point, namely that
> txid_current(), as well as some other txid_* functions, have returned
> 64bit xids for many many years. txid_current() is the only function to
> get the current xid in a reasonable way. I don't understand how a
> proposal to add a 32/32 bit representation *in addition* to the existing
> 32 and 64bit representations is going to improve the situation. Nor do I
> see changing txid_current()'s return format as something we're going to
> go for.
>
> I did not argue against a function to turn 64bit xids into epoch/32bit
> xid or such.

I thought we were talking about how the new xid8 type ought to behave.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi,

On 2020-04-17 14:07:07 -0400, Robert Haas wrote:
> On Fri, Apr 17, 2020 at 1:45 PM Andres Freund <andres@anarazel.de> wrote:
> > You seem to be entirely disregarding my actual point, namely that
> > txid_current(), as well as some other txid_* functions, have returned
> > 64bit xids for many many years. txid_current() is the only function to
> > get the current xid in a reasonable way. I don't understand how a
> > proposal to add a 32/32 bit representation *in addition* to the existing
> > 32 and 64bit representations is going to improve the situation. Nor do I
> > see changing txid_current()'s return format as something we're going to
> > go for.
> >
> > I did not argue against a function to turn 64bit xids into epoch/32bit
> > xid or such.
> 
> I thought we were talking about how the new xid8 type ought to behave.

Yes? But that type doesn't exist in isolation. Having yet another
significantly different representation of 64bit xids (as plain 64 bit
integers, and as some 32/32 epoch/xid split) would make an already
confusing situation even more complex.

Greetings,

Andres Freund

On 2020-Apr-17, Andres Freund wrote:

> Yes? But that type doesn't exist in isolation. Having yet another
> significantly different representation of 64bit xids (as plain 64 bit
> integers, and as some 32/32 epoch/xid split) would make an already
> confusing situation even more complex.

On the contrary -- I think it would clarify a confusing situation.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Tue, Apr 7, 2020 at 12:14 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> On Sun, Apr 5, 2020 at 11:31 AM Thomas Munro <thomas.munro@gmail.com> wrote:
> > On Sun, Apr 5, 2020 at 10:34 AM Mark Dilger
> > <mark.dilger@enterprisedb.com> wrote:
> > > The "xid8_" warts are partly motivated by having a type named "xid8", which is a bit of a wart in itself.
> >
> > Just a thought for the future, not sure if it's a good one: would it
> > seem less warty in years to come if we introduced xid4 as an alias for
> > xid, and preferred the name xid4?  Then it wouldn't look so much like
> > xid is the "real" transaction ID type and xid8 is some kind of freaky
> > extended version; instead it would look like xid4 and xid8 are narrow
> > and wide transaction IDs, and xid is just a historical name for xid4.
>
> I'll look into proposing that for PG14.  One reason I like that idea
> is that system view names like backend_xid could potentially retain
> their names while switching to xid8 type, (maybe?) breaking fewer
> queries and avoiding ugly names, on the theory that _xid doesn't
> specify whether it's xid4 or an xid8.

Here's a patch that renames xid to xid4, but I realised that we lack
the technology to create a suitable backwards compat alias.  The
bigint/int8 keyword trick doesn't work here, because it would break
existing queries using xid as, for example, a function argument name.
Perhaps we could invent a new kind of type that is a simple alias for
another type, and is entirely replaced by the base type early in
processing, so that you can do type aliases without bigint-style
keywords.  Perhaps all of this is not worth the churn just for a
neatnick project.