On 2022/02/08 13:23, Ken Kato wrote:
Thank you for the comments!
if (FullTransactionIdFollows(fxid1, fxid2))
PG_RETURN_FULLTRANSACTIONID(fxid1);
else
PG_RETURN_FULLTRANSACTIONID(fxid2);
Isn't it better to use '0xffffffffffffffff'::xid8 instead of
'18446744073709551615'::xid8, to more easily understand that this test
uses maximum number allowed as xid8?
I updated these two parts as you suggested.
In addition to those two xid8 values, IMO it's better to insert also
the xid8 value neither minimum nor maximum xid8 ones, for example,
'42'::xid8.
I added '010'::xid8, '42'::xid8, and '-1'::xid8
in addition to '0'::xid8 and '0xffffffffffffffff'::xid8
just to have more varieties.
Thanks for updating the patch! It basically looks good to me. I applied the
following small changes to the patch. Updated version of the patch attached.
Could you review this version?
+ if (FullTransactionIdFollowsOrEquals(fxid1, fxid2))
+ PG_RETURN_FULLTRANSACTIONID(fxid1);
I used FullTransactionIdFollows() and FullTransactionIdPrecedes() in xid8_larger() and xid8_smaller()
because other xxx_larger() and xxx_smaller() functions also use ">" operator instead of
">=".
+create table xid8_tab (x xid8);
+insert into xid8_tab values ('0'::xid8), ('010'::xid8),
+('42'::xid8), ('0xffffffffffffffff'::xid8), ('-1'::xid8);
Since "::xid8" is not necessary here, I got rid of it from the above query.
I also merged this xid8_tab and the existing xid8_t1 table, to reduce the
number of table creation.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATIONdiff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 8754f2f89b..1b064b4feb 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -19973,7 +19973,7 @@ SELECT NULLIF(value, '(none)') ...
values. Available for any numeric, string, date/time, or enum type,
as well as <type>inet</type>, <type>interval</type>,
<type>money</type>, <type>oid</type>, <type>pg_lsn</type>,
- <type>tid</type>,
+ <type>tid</type>, <type>xid8</type>,
and arrays of any of these types.
</para></entry>
<entry>Yes</entry>
@@ -19992,7 +19992,7 @@ SELECT NULLIF(value, '(none)') ...
values. Available for any numeric, string, date/time, or enum type,
as well as <type>inet</type>, <type>interval</type>,
<type>money</type>, <type>oid</type>, <type>pg_lsn</type>,
- <type>tid</type>,
+ <type>tid</type>, <type>xid8</type>,
and arrays of any of these types.
</para></entry>
<entry>Yes</entry>
diff --git a/src/backend/utils/adt/xid.c b/src/backend/utils/adt/xid.c
index 9b4ceaea47..e4b4952a28 100644
--- a/src/backend/utils/adt/xid.c
+++ b/src/backend/utils/adt/xid.c
@@ -286,6 +286,30 @@ xid8cmp(PG_FUNCTION_ARGS)
PG_RETURN_INT32(-1);
}
+Datum
+xid8_larger(PG_FUNCTION_ARGS)
+{
+ FullTransactionId fxid1 = PG_GETARG_FULLTRANSACTIONID(0);
+ FullTransactionId fxid2 = PG_GETARG_FULLTRANSACTIONID(1);
+
+ if (FullTransactionIdFollows(fxid1, fxid2))
+ PG_RETURN_FULLTRANSACTIONID(fxid1);
+ else
+ PG_RETURN_FULLTRANSACTIONID(fxid2);
+}
+
+Datum
+xid8_smaller(PG_FUNCTION_ARGS)
+{
+ FullTransactionId fxid1 = PG_GETARG_FULLTRANSACTIONID(0);
+ FullTransactionId fxid2 = PG_GETARG_FULLTRANSACTIONID(1);
+
+ if (FullTransactionIdPrecedes(fxid1, fxid2))
+ PG_RETURN_FULLTRANSACTIONID(fxid1);
+ else
+ PG_RETURN_FULLTRANSACTIONID(fxid2);
+}
+
/*****************************************************************************
* COMMAND IDENTIFIER ROUTINES
*
*****************************************************************************/
diff --git a/src/include/catalog/pg_aggregate.dat
b/src/include/catalog/pg_aggregate.dat
index 137f6eef69..2843f4b415 100644
--- a/src/include/catalog/pg_aggregate.dat
+++ b/src/include/catalog/pg_aggregate.dat
@@ -149,6 +149,9 @@
{ aggfnoid => 'max(pg_lsn)', aggtransfn => 'pg_lsn_larger',
aggcombinefn => 'pg_lsn_larger', aggsortop => '>(pg_lsn,pg_lsn)',
aggtranstype => 'pg_lsn' },
+{ aggfnoid => 'max(xid8)', aggtransfn => 'xid8_larger',
+ aggcombinefn => 'xid8_larger', aggsortop => '>(xid8,xid8)',
+ aggtranstype => 'xid8' },
# min
{ aggfnoid => 'min(int8)', aggtransfn => 'int8smaller',
@@ -214,6 +217,9 @@
{ aggfnoid => 'min(pg_lsn)', aggtransfn => 'pg_lsn_smaller',
aggcombinefn => 'pg_lsn_smaller', aggsortop => '<(pg_lsn,pg_lsn)',
aggtranstype => 'pg_lsn' },
+{ aggfnoid => 'min(xid8)', aggtransfn => 'xid8_smaller',
+ aggcombinefn => 'xid8_smaller', aggsortop => '<(xid8,xid8)',
+ aggtranstype => 'xid8' },
# count
{ aggfnoid => 'count(any)', aggtransfn => 'int8inc_any',
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 7024dbe10a..62f36daa98 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -203,6 +203,12 @@
{ oid => '5071', descr => 'convert xid8 to xid',
proname => 'xid', prorettype => 'xid', proargtypes => 'xid8',
prosrc => 'xid8toxid' },
+{ oid => '5097', descr => 'larger of two',
+ proname => 'xid8_larger', prorettype => 'xid8', proargtypes => 'xid8 xid8',
+ prosrc => 'xid8_larger' },
+{ oid => '5098', descr => 'smaller of two',
+ proname => 'xid8_smaller', prorettype => 'xid8', proargtypes => 'xid8 xid8',
+ prosrc => 'xid8_smaller' },
{ oid => '69',
proname => 'cideq', proleakproof => 't', prorettype => 'bool',
proargtypes => 'cid cid', prosrc => 'cideq' },
@@ -6564,6 +6570,9 @@
{ oid => '4189', descr => 'maximum value of all pg_lsn input values',
proname => 'max', prokind => 'a', proisstrict => 'f', prorettype => 'pg_lsn',
proargtypes => 'pg_lsn', prosrc => 'aggregate_dummy' },
+{ oid => '5099', descr => 'maximum value of all xid8 input values',
+ proname => 'max', prokind => 'a', proisstrict => 'f', prorettype => 'xid8',
+ proargtypes => 'xid8', prosrc => 'aggregate_dummy' },
{ oid => '2131', descr => 'minimum value of all bigint input values',
proname => 'min', prokind => 'a', proisstrict => 'f', prorettype => 'int8',
@@ -6631,6 +6640,9 @@
{ oid => '4190', descr => 'minimum value of all pg_lsn input values',
proname => 'min', prokind => 'a', proisstrict => 'f', prorettype => 'pg_lsn',
proargtypes => 'pg_lsn', prosrc => 'aggregate_dummy' },
+{ oid => '5100', descr => 'minimum value of all xid8 input values',
+ proname => 'min', prokind => 'a', proisstrict => 'f', prorettype => 'xid8',
+ proargtypes => 'xid8', prosrc => 'aggregate_dummy' },
# count has two forms: count(any) and count(*)
{ oid => '2147',
diff --git a/src/test/regress/expected/xid.out
b/src/test/regress/expected/xid.out
index b7a1ed0f9e..d8e76f3321 100644
--- a/src/test/regress/expected/xid.out
+++ b/src/test/regress/expected/xid.out
@@ -129,8 +129,16 @@ select xid8cmp('1', '2'), xid8cmp('2', '2'), xid8cmp('2',
'1');
-1 | 0 | 1
(1 row)
--- xid8 has btree and hash opclasses
+-- min() and max() for xid8
create table xid8_t1 (x xid8);
+insert into xid8_t1 values ('0'), ('010'), ('42'), ('0xffffffffffffffff'),
('-1');
+select min(x), max(x) from xid8_t1;
+ min | max
+-----+----------------------
+ 0 | 18446744073709551615
+(1 row)
+
+-- xid8 has btree and hash opclasses
create index on xid8_t1 using btree(x);
create index on xid8_t1 using hash(x);
drop table xid8_t1;
diff --git a/src/test/regress/sql/xid.sql b/src/test/regress/sql/xid.sql
index 565714d462..bee17e6364 100644
--- a/src/test/regress/sql/xid.sql
+++ b/src/test/regress/sql/xid.sql
@@ -41,8 +41,12 @@ select '1'::xid8 >= '2'::xid8, '2'::xid8 >= '2'::xid8,
'2'::xid8 >= '1'::xid8;
-- we also have a 3way compare for btrees
select xid8cmp('1', '2'), xid8cmp('2', '2'), xid8cmp('2', '1');
--- xid8 has btree and hash opclasses
+-- min() and max() for xid8
create table xid8_t1 (x xid8);
+insert into xid8_t1 values ('0'), ('010'), ('42'), ('0xffffffffffffffff'),
('-1');
+select min(x), max(x) from xid8_t1;
+
+-- xid8 has btree and hash opclasses
create index on xid8_t1 using btree(x);
create index on xid8_t1 using hash(x);
drop table xid8_t1;
