New pg_lsn type doesn't have hash/btree opclasses

Started by Andres Freundover 11 years ago45 messages

andres@2ndquadrant.com

over 11 years ago

Hi,

Craig just mentioned in an internal chat that there's no btree or even
hash opclass for the new pg_lsn type. That restricts what you can do
with it quite severely.
Imo this should be fixed for 9.4 - after all it was possible unto now to
index a table with lsns returned by system functions or have queries
with grouping on them without casting.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane

tgl@sss.pgh.pa.us

over 11 years ago

In reply to: Andres Freund (#1)

Re: New pg_lsn type doesn't have hash/btree opclasses

Andres Freund <andres@2ndquadrant.com> writes:

Craig just mentioned in an internal chat that there's no btree or even
hash opclass for the new pg_lsn type. That restricts what you can do
with it quite severely.
Imo this should be fixed for 9.4 - after all it was possible unto now to
index a table with lsns returned by system functions or have queries
with grouping on them without casting.

Sorry, it is *way* too late for 9.4.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andres Freund

andres@2ndquadrant.com

over 11 years ago

In reply to: Tom Lane (#2)

Re: New pg_lsn type doesn't have hash/btree opclasses

On 2014-05-06 09:37:54 -0400, Tom Lane wrote:

Andres Freund <andres@2ndquadrant.com> writes:

Craig just mentioned in an internal chat that there's no btree or even
hash opclass for the new pg_lsn type. That restricts what you can do
with it quite severely.
Imo this should be fixed for 9.4 - after all it was possible unto now to
index a table with lsns returned by system functions or have queries
with grouping on them without casting.

Sorry, it is *way* too late for 9.4.

It's imo a regression/oversight introduced in the pg_lsn patch. Not a
new feature.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Michael Paquier

michael.paquier@gmail.com

over 11 years ago

In reply to: Andres Freund (#1)

1 attachment(s)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Tue, May 6, 2014 at 4:14 PM, Andres Freund <andres@2ndquadrant.com> wrote:

Hi,

Craig just mentioned in an internal chat that there's no btree or even
hash opclass for the new pg_lsn type. That restricts what you can do
with it quite severely.
Imo this should be fixed for 9.4 - after all it was possible unto now to
index a table with lsns returned by system functions or have queries
with grouping on them without casting.

Makes sense, especially knowing operators needed for btree processing
are already defined. Patch attached solves that. Now to include it in
9.4 where development is over is another story... I wouldn't mind
adding it to the next commit fest either.
Regards,
--
Michael

Attachments:

20140506_pglsn_btree_hash.patchtext/plain; charset=US-ASCII; name=20140506_pglsn_btree_hash.patchDownload

commit 604177ac2af4bfd26a392b8669dd2fc3550f2a3d
Author: Michael Paquier <michael@otacoo.com>
Date:   Tue May 6 22:42:09 2014 +0900

    Support for hash and btree operators for pg_lsn datatype
    
    The following functions are added in pg_lsn bundle for the different
    index operators:
    - pg_lsn_hash for hash index
    - pg_lsn_cmp for btree index
    Related catalogs are updated as well.

diff --git a/src/backend/utils/adt/pg_lsn.c b/src/backend/utils/adt/pg_lsn.c
index e2b528a..f7980f8 100644
--- a/src/backend/utils/adt/pg_lsn.c
+++ b/src/backend/utils/adt/pg_lsn.c
@@ -150,6 +150,24 @@ pg_lsn_ge(PG_FUNCTION_ARGS)
 	PG_RETURN_BOOL(lsn1 >= lsn2);
 }
 
+/* handler for btree index operator */
+Datum
+pg_lsn_cmp(PG_FUNCTION_ARGS)
+{
+	XLogRecPtr lsn1 = PG_GETARG_LSN(0);
+	XLogRecPtr lsn2 = PG_GETARG_LSN(1);
+
+	PG_RETURN_INT32(lsn1 == lsn2);
+}
+
+/* hash index support */
+Datum
+pg_lsn_hash(PG_FUNCTION_ARGS)
+{
+	XLogRecPtr lsn = PG_GETARG_LSN(0);
+
+	return hashint8(lsn);
+}
 
 /*----------------------------------------------------------
  *	Arithmetic operators on PostgreSQL LSNs.
diff --git a/src/include/catalog/pg_amop.h b/src/include/catalog/pg_amop.h
index 8efd3be..5cd170d 100644
--- a/src/include/catalog/pg_amop.h
+++ b/src/include/catalog/pg_amop.h
@@ -513,6 +513,16 @@ DATA(insert (	2968  2950 2950 4 s 2977	403 0 ));
 DATA(insert (	2968  2950 2950 5 s 2975	403 0 ));
 
 /*
+ * btree pglsn_ops
+ */
+
+DATA(insert (	3260  3220 3220 1 s 3224	403 0 ));
+DATA(insert (	3260  3220 3220 2 s 3226	403 0 ));
+DATA(insert (	3260  3220 3220 3 s 3222	403 0 ));
+DATA(insert (	3260  3220 3220 4 s 3227	403 0 ));
+DATA(insert (	3260  3220 3220 5 s 3225	403 0 ));
+
+/*
  *	hash index _ops
  */
 
@@ -581,6 +591,8 @@ DATA(insert (	2231   1042 1042 1 s 1054 405 0 ));
 DATA(insert (	2235   1033 1033 1 s  974 405 0 ));
 /* uuid_ops */
 DATA(insert (	2969   2950 2950 1 s 2972 405 0 ));
+/* pglsn_ops */
+DATA(insert (	3261   3220 3220 1 s 3222 405 0 ));
 /* numeric_ops */
 DATA(insert (	1998   1700 1700 1 s 1752 405 0 ));
 /* array_ops */
diff --git a/src/include/catalog/pg_amproc.h b/src/include/catalog/pg_amproc.h
index 198b126..369adb9 100644
--- a/src/include/catalog/pg_amproc.h
+++ b/src/include/catalog/pg_amproc.h
@@ -134,6 +134,7 @@ DATA(insert (	2789   27 27 1 2794 ));
 DATA(insert (	2968   2950 2950 1 2960 ));
 DATA(insert (	2994   2249 2249 1 2987 ));
 DATA(insert (	3194   2249 2249 1 3187 ));
+DATA(insert (	3260   3220 3220 1 3263 ));
 DATA(insert (	3522   3500 3500 1 3514 ));
 DATA(insert (	3626   3614 3614 1 3622 ));
 DATA(insert (	3683   3615 3615 1 3668 ));
@@ -174,6 +175,7 @@ DATA(insert (	2229   25 25 1 400 ));
 DATA(insert (	2231   1042 1042 1 1080 ));
 DATA(insert (	2235   1033 1033 1 329 ));
 DATA(insert (	2969   2950 2950 1 2963 ));
+DATA(insert (	3261   3220 3220 1 3262 ));
 DATA(insert (	3523   3500 3500 1 3515 ));
 DATA(insert (	3903   3831 3831 1 3902 ));
 DATA(insert (	4034   3802 3802 1 4045 ));
diff --git a/src/include/catalog/pg_opclass.h b/src/include/catalog/pg_opclass.h
index 49b2410..cbcdacd 100644
--- a/src/include/catalog/pg_opclass.h
+++ b/src/include/catalog/pg_opclass.h
@@ -215,6 +215,8 @@ DATA(insert (	2742	_reltime_ops		PGNSP PGUID 2745  1024 t 703 ));
 DATA(insert (	2742	_tinterval_ops		PGNSP PGUID 2745  1025 t 704 ));
 DATA(insert (	403		uuid_ops			PGNSP PGUID 2968  2950 t 0 ));
 DATA(insert (	405		uuid_ops			PGNSP PGUID 2969  2950 t 0 ));
+DATA(insert (	403		pglsn_ops			PGNSP PGUID 3260  3220 t 0 ));
+DATA(insert (	405		pglsn_ops			PGNSP PGUID 3261  3220 t 0 ));
 DATA(insert (	403		enum_ops			PGNSP PGUID 3522  3500 t 0 ));
 DATA(insert (	405		enum_ops			PGNSP PGUID 3523  3500 t 0 ));
 DATA(insert (	403		tsvector_ops		PGNSP PGUID 3626  3614 t 0 ));
diff --git a/src/include/catalog/pg_operator.h b/src/include/catalog/pg_operator.h
index f280af4..87ee4eb 100644
--- a/src/include/catalog/pg_operator.h
+++ b/src/include/catalog/pg_operator.h
@@ -1595,7 +1595,7 @@ DATA(insert OID = 2977 (  ">="	   PGNSP PGUID b f f 2950 2950 16 2976 2974 uuid_
 DESCR("greater than or equal");
 
 /* pg_lsn operators */
-DATA(insert OID = 3222 (  "="	   PGNSP PGUID b f f 3220 3220 16 3222 3223 pg_lsn_eq eqsel eqjoinsel ));
+DATA(insert OID = 3222 (  "="	   PGNSP PGUID b t t 3220 3220 16 3222 3223 pg_lsn_eq eqsel eqjoinsel ));
 DESCR("equal");
 DATA(insert OID = 3223 (  "<>"	   PGNSP PGUID b f f 3220 3220 16 3223 3222 pg_lsn_ne neqsel neqjoinsel ));
 DESCR("not equal");
diff --git a/src/include/catalog/pg_opfamily.h b/src/include/catalog/pg_opfamily.h
index 9e8f4ac..29089d5 100644
--- a/src/include/catalog/pg_opfamily.h
+++ b/src/include/catalog/pg_opfamily.h
@@ -134,6 +134,8 @@ DATA(insert OID = 1029 (	783		point_ops		PGNSP PGUID ));
 DATA(insert OID = 2745 (	2742	array_ops		PGNSP PGUID ));
 DATA(insert OID = 2968 (	403		uuid_ops		PGNSP PGUID ));
 DATA(insert OID = 2969 (	405		uuid_ops		PGNSP PGUID ));
+DATA(insert OID = 3260 (	403		pglsn_ops		PGNSP PGUID ));
+DATA(insert OID = 3261 (	405		pglsn_ops		PGNSP PGUID ));
 DATA(insert OID = 3522 (	403		enum_ops		PGNSP PGUID ));
 DATA(insert OID = 3523 (	405		enum_ops		PGNSP PGUID ));
 DATA(insert OID = 3626 (	403		tsvector_ops	PGNSP PGUID ));
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 98c183b..f82463b 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -4293,6 +4293,10 @@ DATA(insert OID = 3238 (  pg_lsn_recv	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 3
 DESCR("I/O");
 DATA(insert OID = 3239 (  pg_lsn_send	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 17 "3220" _null_ _null_ _null_ _null_ pg_lsn_send _null_ _null_ _null_ ));
 DESCR("I/O");
+DATA(insert OID = 3262 (  pg_lsn_hash	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 23 "3220" _null_ _null_ _null_ _null_ pg_lsn_hash _null_ _null_ _null_ ));
+DESCR("hash");
+DATA(insert OID = 3263 (  pg_lsn_cmp	PGNSP PGUID 12 1 0 0 0 f f f f t f i 2 0 23 "3220 3220" _null_ _null_ _null_ _null_ pg_lsn_cmp _null_ _null_ _null_ ));
+DESCR("less-equal-greater");
 
 /* enum related procs */
 DATA(insert OID = 3504 (  anyenum_in	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 3500 "2275" _null_ _null_ _null_ _null_ anyenum_in _null_ _null_ _null_ ));
diff --git a/src/include/utils/pg_lsn.h b/src/include/utils/pg_lsn.h
index 981fcd6..7dd932d 100644
--- a/src/include/utils/pg_lsn.h
+++ b/src/include/utils/pg_lsn.h
@@ -29,6 +29,8 @@ extern Datum pg_lsn_lt(PG_FUNCTION_ARGS);
 extern Datum pg_lsn_gt(PG_FUNCTION_ARGS);
 extern Datum pg_lsn_le(PG_FUNCTION_ARGS);
 extern Datum pg_lsn_ge(PG_FUNCTION_ARGS);
+extern Datum pg_lsn_cmp(PG_FUNCTION_ARGS);
+extern Datum pg_lsn_hash(PG_FUNCTION_ARGS);
 
 extern Datum pg_lsn_mi(PG_FUNCTION_ARGS);

Tom Lane

tgl@sss.pgh.pa.us

over 11 years ago

In reply to: Andres Freund (#3)

Re: New pg_lsn type doesn't have hash/btree opclasses

Andres Freund <andres@2ndquadrant.com> writes:

On 2014-05-06 09:37:54 -0400, Tom Lane wrote:

Sorry, it is *way* too late for 9.4.

It's imo a regression/oversight introduced in the pg_lsn patch. Not a
new feature.

You can argue that if you like, but it doesn't matter. It's too late for
a change as big as that for such an inessential feature. We are in the
stabilization game at this point, and adding features is not the thing to
be doing.

Especially not features for which no patch even exists. I don't care if
it could be done in a few hours by copy-and-paste, it would still be the
very definition of rushed coding. We're past the window for this kind of
change in 9.4.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Heikki Linnakangas

hlinnakangas@vmware.com

over 11 years ago

In reply to: Tom Lane (#5)

Re: New pg_lsn type doesn't have hash/btree opclasses

On 05/06/2014 05:17 PM, Tom Lane wrote:

Andres Freund <andres@2ndquadrant.com> writes:

On 2014-05-06 09:37:54 -0400, Tom Lane wrote:

Sorry, it is *way* too late for 9.4.

It's imo a regression/oversight introduced in the pg_lsn patch. Not a
new feature.

You can argue that if you like, but it doesn't matter. It's too late for
a change as big as that for such an inessential feature. We are in the
stabilization game at this point, and adding features is not the thing to
be doing.

FWIW, I agree with Andres that this would be a reasonable thing to add.
Exactly the kind of oversight that we should be fixing at this stage in
the release cycle.

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Vik Fearing

vik.fearing@dalibo.com

over 11 years ago

In reply to: Heikki Linnakangas (#6)

Re: New pg_lsn type doesn't have hash/btree opclasses

On 05/06/2014 04:59 PM, Heikki Linnakangas wrote:

On 05/06/2014 05:17 PM, Tom Lane wrote:

Andres Freund <andres@2ndquadrant.com> writes:

On 2014-05-06 09:37:54 -0400, Tom Lane wrote:

Sorry, it is *way* too late for 9.4.

It's imo a regression/oversight introduced in the pg_lsn patch. Not a
new feature.

You can argue that if you like, but it doesn't matter. It's too late
for
a change as big as that for such an inessential feature. We are in the
stabilization game at this point, and adding features is not the
thing to
be doing.

FWIW, I agree with Andres that this would be a reasonable thing to
add. Exactly the kind of oversight that we should be fixing at this
stage in the release cycle.

I agree as well.

--
Vik

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Michael Paquier

michael.paquier@gmail.com

over 11 years ago

In reply to: Michael Paquier (#4)

1 attachment(s)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Tue, May 6, 2014 at 10:49 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

Makes sense, especially knowing operators needed for btree processing
are already defined. Patch attached solves that.

Please find attached an updated patch, I completely forgot that the
compare function needs to return {-1, 0, 1}.
--
Michael

Attachments:

20140507_pglsn_btree_hash_v2.patchtext/plain; charset=US-ASCII; name=20140507_pglsn_btree_hash_v2.patchDownload

diff --git a/src/backend/utils/adt/pg_lsn.c b/src/backend/utils/adt/pg_lsn.c
index e2b528a..baf789a 100644
--- a/src/backend/utils/adt/pg_lsn.c
+++ b/src/backend/utils/adt/pg_lsn.c
@@ -150,6 +150,28 @@ pg_lsn_ge(PG_FUNCTION_ARGS)
 	PG_RETURN_BOOL(lsn1 >= lsn2);
 }
 
+/* handler for btree index operator */
+Datum
+pg_lsn_cmp(PG_FUNCTION_ARGS)
+{
+	XLogRecPtr lsn1 = PG_GETARG_LSN(0);
+	XLogRecPtr lsn2 = PG_GETARG_LSN(1);
+
+	if (lsn1 < lsn2)
+		PG_RETURN_INT32(-1);
+	if (lsn1 > lsn2)
+		PG_RETURN_INT32(1);
+	PG_RETURN_INT32(0);
+}
+
+/* hash index support */
+Datum
+pg_lsn_hash(PG_FUNCTION_ARGS)
+{
+	XLogRecPtr lsn = PG_GETARG_LSN(0);
+
+	return hashint8(lsn);
+}
 
 /*----------------------------------------------------------
  *	Arithmetic operators on PostgreSQL LSNs.
diff --git a/src/include/catalog/pg_amop.h b/src/include/catalog/pg_amop.h
index 8efd3be..5cd170d 100644
--- a/src/include/catalog/pg_amop.h
+++ b/src/include/catalog/pg_amop.h
@@ -513,6 +513,16 @@ DATA(insert (	2968  2950 2950 4 s 2977	403 0 ));
 DATA(insert (	2968  2950 2950 5 s 2975	403 0 ));
 
 /*
+ * btree pglsn_ops
+ */
+
+DATA(insert (	3260  3220 3220 1 s 3224	403 0 ));
+DATA(insert (	3260  3220 3220 2 s 3226	403 0 ));
+DATA(insert (	3260  3220 3220 3 s 3222	403 0 ));
+DATA(insert (	3260  3220 3220 4 s 3227	403 0 ));
+DATA(insert (	3260  3220 3220 5 s 3225	403 0 ));
+
+/*
  *	hash index _ops
  */
 
@@ -581,6 +591,8 @@ DATA(insert (	2231   1042 1042 1 s 1054 405 0 ));
 DATA(insert (	2235   1033 1033 1 s  974 405 0 ));
 /* uuid_ops */
 DATA(insert (	2969   2950 2950 1 s 2972 405 0 ));
+/* pglsn_ops */
+DATA(insert (	3261   3220 3220 1 s 3222 405 0 ));
 /* numeric_ops */
 DATA(insert (	1998   1700 1700 1 s 1752 405 0 ));
 /* array_ops */
diff --git a/src/include/catalog/pg_amproc.h b/src/include/catalog/pg_amproc.h
index 198b126..369adb9 100644
--- a/src/include/catalog/pg_amproc.h
+++ b/src/include/catalog/pg_amproc.h
@@ -134,6 +134,7 @@ DATA(insert (	2789   27 27 1 2794 ));
 DATA(insert (	2968   2950 2950 1 2960 ));
 DATA(insert (	2994   2249 2249 1 2987 ));
 DATA(insert (	3194   2249 2249 1 3187 ));
+DATA(insert (	3260   3220 3220 1 3263 ));
 DATA(insert (	3522   3500 3500 1 3514 ));
 DATA(insert (	3626   3614 3614 1 3622 ));
 DATA(insert (	3683   3615 3615 1 3668 ));
@@ -174,6 +175,7 @@ DATA(insert (	2229   25 25 1 400 ));
 DATA(insert (	2231   1042 1042 1 1080 ));
 DATA(insert (	2235   1033 1033 1 329 ));
 DATA(insert (	2969   2950 2950 1 2963 ));
+DATA(insert (	3261   3220 3220 1 3262 ));
 DATA(insert (	3523   3500 3500 1 3515 ));
 DATA(insert (	3903   3831 3831 1 3902 ));
 DATA(insert (	4034   3802 3802 1 4045 ));
diff --git a/src/include/catalog/pg_opclass.h b/src/include/catalog/pg_opclass.h
index 49b2410..cbcdacd 100644
--- a/src/include/catalog/pg_opclass.h
+++ b/src/include/catalog/pg_opclass.h
@@ -215,6 +215,8 @@ DATA(insert (	2742	_reltime_ops		PGNSP PGUID 2745  1024 t 703 ));
 DATA(insert (	2742	_tinterval_ops		PGNSP PGUID 2745  1025 t 704 ));
 DATA(insert (	403		uuid_ops			PGNSP PGUID 2968  2950 t 0 ));
 DATA(insert (	405		uuid_ops			PGNSP PGUID 2969  2950 t 0 ));
+DATA(insert (	403		pglsn_ops			PGNSP PGUID 3260  3220 t 0 ));
+DATA(insert (	405		pglsn_ops			PGNSP PGUID 3261  3220 t 0 ));
 DATA(insert (	403		enum_ops			PGNSP PGUID 3522  3500 t 0 ));
 DATA(insert (	405		enum_ops			PGNSP PGUID 3523  3500 t 0 ));
 DATA(insert (	403		tsvector_ops		PGNSP PGUID 3626  3614 t 0 ));
diff --git a/src/include/catalog/pg_operator.h b/src/include/catalog/pg_operator.h
index f280af4..87ee4eb 100644
--- a/src/include/catalog/pg_operator.h
+++ b/src/include/catalog/pg_operator.h
@@ -1595,7 +1595,7 @@ DATA(insert OID = 2977 (  ">="	   PGNSP PGUID b f f 2950 2950 16 2976 2974 uuid_
 DESCR("greater than or equal");
 
 /* pg_lsn operators */
-DATA(insert OID = 3222 (  "="	   PGNSP PGUID b f f 3220 3220 16 3222 3223 pg_lsn_eq eqsel eqjoinsel ));
+DATA(insert OID = 3222 (  "="	   PGNSP PGUID b t t 3220 3220 16 3222 3223 pg_lsn_eq eqsel eqjoinsel ));
 DESCR("equal");
 DATA(insert OID = 3223 (  "<>"	   PGNSP PGUID b f f 3220 3220 16 3223 3222 pg_lsn_ne neqsel neqjoinsel ));
 DESCR("not equal");
diff --git a/src/include/catalog/pg_opfamily.h b/src/include/catalog/pg_opfamily.h
index 9e8f4ac..29089d5 100644
--- a/src/include/catalog/pg_opfamily.h
+++ b/src/include/catalog/pg_opfamily.h
@@ -134,6 +134,8 @@ DATA(insert OID = 1029 (	783		point_ops		PGNSP PGUID ));
 DATA(insert OID = 2745 (	2742	array_ops		PGNSP PGUID ));
 DATA(insert OID = 2968 (	403		uuid_ops		PGNSP PGUID ));
 DATA(insert OID = 2969 (	405		uuid_ops		PGNSP PGUID ));
+DATA(insert OID = 3260 (	403		pglsn_ops		PGNSP PGUID ));
+DATA(insert OID = 3261 (	405		pglsn_ops		PGNSP PGUID ));
 DATA(insert OID = 3522 (	403		enum_ops		PGNSP PGUID ));
 DATA(insert OID = 3523 (	405		enum_ops		PGNSP PGUID ));
 DATA(insert OID = 3626 (	403		tsvector_ops	PGNSP PGUID ));
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 98c183b..f82463b 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -4293,6 +4293,10 @@ DATA(insert OID = 3238 (  pg_lsn_recv	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 3
 DESCR("I/O");
 DATA(insert OID = 3239 (  pg_lsn_send	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 17 "3220" _null_ _null_ _null_ _null_ pg_lsn_send _null_ _null_ _null_ ));
 DESCR("I/O");
+DATA(insert OID = 3262 (  pg_lsn_hash	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 23 "3220" _null_ _null_ _null_ _null_ pg_lsn_hash _null_ _null_ _null_ ));
+DESCR("hash");
+DATA(insert OID = 3263 (  pg_lsn_cmp	PGNSP PGUID 12 1 0 0 0 f f f f t f i 2 0 23 "3220 3220" _null_ _null_ _null_ _null_ pg_lsn_cmp _null_ _null_ _null_ ));
+DESCR("less-equal-greater");
 
 /* enum related procs */
 DATA(insert OID = 3504 (  anyenum_in	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 3500 "2275" _null_ _null_ _null_ _null_ anyenum_in _null_ _null_ _null_ ));
diff --git a/src/include/utils/pg_lsn.h b/src/include/utils/pg_lsn.h
index 981fcd6..7dd932d 100644
--- a/src/include/utils/pg_lsn.h
+++ b/src/include/utils/pg_lsn.h
@@ -29,6 +29,8 @@ extern Datum pg_lsn_lt(PG_FUNCTION_ARGS);
 extern Datum pg_lsn_gt(PG_FUNCTION_ARGS);
 extern Datum pg_lsn_le(PG_FUNCTION_ARGS);
 extern Datum pg_lsn_ge(PG_FUNCTION_ARGS);
+extern Datum pg_lsn_cmp(PG_FUNCTION_ARGS);
+extern Datum pg_lsn_hash(PG_FUNCTION_ARGS);
 
 extern Datum pg_lsn_mi(PG_FUNCTION_ARGS);

Andres Freund

andres@2ndquadrant.com

over 11 years ago

In reply to: Michael Paquier (#4)

1 attachment(s)

Re: New pg_lsn type doesn't have hash/btree opclasses

Hi,

On 2014-05-06 22:49:07 +0900, Michael Paquier wrote:

Makes sense, especially knowing operators needed for btree processing
are already defined. Patch attached solves that.

Thanks for doing that quickly.

FWIW, the format you're using makes applying the patch including the
commit message relatively hard. Consider using git format-patch.

+/* handler for btree index operator */
+Datum
+pg_lsn_cmp(PG_FUNCTION_ARGS)
+{
+	XLogRecPtr lsn1 = PG_GETARG_LSN(0);
+	XLogRecPtr lsn2 = PG_GETARG_LSN(1);
+
+	PG_RETURN_INT32(lsn1 == lsn2);
+}

This doesn't look correct. A cmp routine needs to return -1, 0, 1 when a
< b, a = b, a > b respectively. You'll only return 0 and 1 here.

+/* hash index support */
+Datum
+pg_lsn_hash(PG_FUNCTION_ARGS)
+{
+	XLogRecPtr lsn = PG_GETARG_LSN(0);
+
+	return hashint8(lsn);
+}

That can't be right either. There's at least two things wrong here:
a) hashint8 takes PG_FUNCTION_ARGS, not a Datum
b) you're not including the necessary header defining hashint8.

I've used

SELECT DISTINCT (g.i||'/0')::pg_lsn f FROM generate_series(1, 100) g(i), generate_series(1, 5);
SELECT (g.i||'/0')::pg_lsn f FROM generate_series(1, 100) g(i) ORDER BY f;
SELECT (g.i||'/0')::pg_lsn, count(*) FROM generate_series(1, 100) g(i), generate_series(1, 5) GROUP BY 1 ORDER BY 1;
SELECT * FROM
(SELECT (g.i||'/0')::pg_lsn lsn FROM generate_series(1, 10) g(i) ORDER BY g.i) a
JOIN (SELECT (g.i||'/0')::pg_lsn lsn FROM generate_series(1, 10) g(i) ORDER BY g.i DESC) b
ON (a.lsn = b.lsn );

To check that a) hashing works, b) sorting works, c) mergesorts works
after fixing the above issues. New version of the patch attached

I completely agree with Heikki's point that this kind of oversight is
the actual reason for a beta period.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachments:

0001-Support-for-hash-and-btree-operators-for-pg_lsn-data.patchtext/x-patch; charset=us-asciiDownload

>From b25a8f57cb71389bbe823b3c9e2f13440176aad9 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Wed, 7 May 2014 01:05:57 +0200
Subject: [PATCH] Support for hash and btree operators for pg_lsn datatype.

Michael Paquier with fixes from Andres Freund.
---
 src/backend/utils/adt/pg_lsn.c    | 22 ++++++++++++++++++++++
 src/include/catalog/pg_amop.h     | 12 ++++++++++++
 src/include/catalog/pg_amproc.h   |  2 ++
 src/include/catalog/pg_opclass.h  |  2 ++
 src/include/catalog/pg_operator.h |  2 +-
 src/include/catalog/pg_opfamily.h |  2 ++
 src/include/catalog/pg_proc.h     |  4 ++++
 src/include/utils/pg_lsn.h        |  2 ++
 8 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/src/backend/utils/adt/pg_lsn.c b/src/backend/utils/adt/pg_lsn.c
index d1448ae..c021a1e 100644
--- a/src/backend/utils/adt/pg_lsn.c
+++ b/src/backend/utils/adt/pg_lsn.c
@@ -14,6 +14,7 @@
 #include "postgres.h"
 
 #include "funcapi.h"
+#include "access/hash.h"
 #include "libpq/pqformat.h"
 #include "utils/builtins.h"
 #include "utils/pg_lsn.h"
@@ -153,6 +154,27 @@ pg_lsn_ge(PG_FUNCTION_ARGS)
 	PG_RETURN_BOOL(lsn1 >= lsn2);
 }
 
+/* handler for btree index operator */
+Datum
+pg_lsn_cmp(PG_FUNCTION_ARGS)
+{
+	XLogRecPtr a = PG_GETARG_LSN(0);
+	XLogRecPtr b = PG_GETARG_LSN(1);
+
+	if (a > b)
+		PG_RETURN_INT32(1);
+	else if (a == b)
+		PG_RETURN_INT32(0);
+	else
+		PG_RETURN_INT32(-1);
+}
+
+/* hash index support */
+Datum
+pg_lsn_hash(PG_FUNCTION_ARGS)
+{
+	return DirectFunctionCall1(hashint8, PG_GETARG_LSN(0));
+}
 
 /*----------------------------------------------------------
  *	Arithmetic operators on PostgreSQL LSNs.
diff --git a/src/include/catalog/pg_amop.h b/src/include/catalog/pg_amop.h
index 8efd3be..5cd170d 100644
--- a/src/include/catalog/pg_amop.h
+++ b/src/include/catalog/pg_amop.h
@@ -513,6 +513,16 @@ DATA(insert (	2968  2950 2950 4 s 2977	403 0 ));
 DATA(insert (	2968  2950 2950 5 s 2975	403 0 ));
 
 /*
+ * btree pglsn_ops
+ */
+
+DATA(insert (	3260  3220 3220 1 s 3224	403 0 ));
+DATA(insert (	3260  3220 3220 2 s 3226	403 0 ));
+DATA(insert (	3260  3220 3220 3 s 3222	403 0 ));
+DATA(insert (	3260  3220 3220 4 s 3227	403 0 ));
+DATA(insert (	3260  3220 3220 5 s 3225	403 0 ));
+
+/*
  *	hash index _ops
  */
 
@@ -581,6 +591,8 @@ DATA(insert (	2231   1042 1042 1 s 1054 405 0 ));
 DATA(insert (	2235   1033 1033 1 s  974 405 0 ));
 /* uuid_ops */
 DATA(insert (	2969   2950 2950 1 s 2972 405 0 ));
+/* pglsn_ops */
+DATA(insert (	3261   3220 3220 1 s 3222 405 0 ));
 /* numeric_ops */
 DATA(insert (	1998   1700 1700 1 s 1752 405 0 ));
 /* array_ops */
diff --git a/src/include/catalog/pg_amproc.h b/src/include/catalog/pg_amproc.h
index 198b126..369adb9 100644
--- a/src/include/catalog/pg_amproc.h
+++ b/src/include/catalog/pg_amproc.h
@@ -134,6 +134,7 @@ DATA(insert (	2789   27 27 1 2794 ));
 DATA(insert (	2968   2950 2950 1 2960 ));
 DATA(insert (	2994   2249 2249 1 2987 ));
 DATA(insert (	3194   2249 2249 1 3187 ));
+DATA(insert (	3260   3220 3220 1 3263 ));
 DATA(insert (	3522   3500 3500 1 3514 ));
 DATA(insert (	3626   3614 3614 1 3622 ));
 DATA(insert (	3683   3615 3615 1 3668 ));
@@ -174,6 +175,7 @@ DATA(insert (	2229   25 25 1 400 ));
 DATA(insert (	2231   1042 1042 1 1080 ));
 DATA(insert (	2235   1033 1033 1 329 ));
 DATA(insert (	2969   2950 2950 1 2963 ));
+DATA(insert (	3261   3220 3220 1 3262 ));
 DATA(insert (	3523   3500 3500 1 3515 ));
 DATA(insert (	3903   3831 3831 1 3902 ));
 DATA(insert (	4034   3802 3802 1 4045 ));
diff --git a/src/include/catalog/pg_opclass.h b/src/include/catalog/pg_opclass.h
index ecf7063..9a45244 100644
--- a/src/include/catalog/pg_opclass.h
+++ b/src/include/catalog/pg_opclass.h
@@ -215,6 +215,8 @@ DATA(insert (	2742	_reltime_ops		PGNSP PGUID 2745  1024 t 703 ));
 DATA(insert (	2742	_tinterval_ops		PGNSP PGUID 2745  1025 t 704 ));
 DATA(insert (	403		uuid_ops			PGNSP PGUID 2968  2950 t 0 ));
 DATA(insert (	405		uuid_ops			PGNSP PGUID 2969  2950 t 0 ));
+DATA(insert (	403		pglsn_ops			PGNSP PGUID 3260  3220 t 0 ));
+DATA(insert (	405		pglsn_ops			PGNSP PGUID 3261  3220 t 0 ));
 DATA(insert (	403		enum_ops			PGNSP PGUID 3522  3500 t 0 ));
 DATA(insert (	405		enum_ops			PGNSP PGUID 3523  3500 t 0 ));
 DATA(insert (	403		tsvector_ops		PGNSP PGUID 3626  3614 t 0 ));
diff --git a/src/include/catalog/pg_operator.h b/src/include/catalog/pg_operator.h
index f280af4..87ee4eb 100644
--- a/src/include/catalog/pg_operator.h
+++ b/src/include/catalog/pg_operator.h
@@ -1595,7 +1595,7 @@ DATA(insert OID = 2977 (  ">="	   PGNSP PGUID b f f 2950 2950 16 2976 2974 uuid_
 DESCR("greater than or equal");
 
 /* pg_lsn operators */
-DATA(insert OID = 3222 (  "="	   PGNSP PGUID b f f 3220 3220 16 3222 3223 pg_lsn_eq eqsel eqjoinsel ));
+DATA(insert OID = 3222 (  "="	   PGNSP PGUID b t t 3220 3220 16 3222 3223 pg_lsn_eq eqsel eqjoinsel ));
 DESCR("equal");
 DATA(insert OID = 3223 (  "<>"	   PGNSP PGUID b f f 3220 3220 16 3223 3222 pg_lsn_ne neqsel neqjoinsel ));
 DESCR("not equal");
diff --git a/src/include/catalog/pg_opfamily.h b/src/include/catalog/pg_opfamily.h
index 9e8f4ac..29089d5 100644
--- a/src/include/catalog/pg_opfamily.h
+++ b/src/include/catalog/pg_opfamily.h
@@ -134,6 +134,8 @@ DATA(insert OID = 1029 (	783		point_ops		PGNSP PGUID ));
 DATA(insert OID = 2745 (	2742	array_ops		PGNSP PGUID ));
 DATA(insert OID = 2968 (	403		uuid_ops		PGNSP PGUID ));
 DATA(insert OID = 2969 (	405		uuid_ops		PGNSP PGUID ));
+DATA(insert OID = 3260 (	403		pglsn_ops		PGNSP PGUID ));
+DATA(insert OID = 3261 (	405		pglsn_ops		PGNSP PGUID ));
 DATA(insert OID = 3522 (	403		enum_ops		PGNSP PGUID ));
 DATA(insert OID = 3523 (	405		enum_ops		PGNSP PGUID ));
 DATA(insert OID = 3626 (	403		tsvector_ops	PGNSP PGUID ));
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index b59cfee..5f045fe 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -4297,6 +4297,10 @@ DATA(insert OID = 3238 (  pg_lsn_recv	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 3
 DESCR("I/O");
 DATA(insert OID = 3239 (  pg_lsn_send	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 17 "3220" _null_ _null_ _null_ _null_ pg_lsn_send _null_ _null_ _null_ ));
 DESCR("I/O");
+DATA(insert OID = 3262 (  pg_lsn_hash	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 23 "3220" _null_ _null_ _null_ _null_ pg_lsn_hash _null_ _null_ _null_ ));
+DESCR("hash");
+DATA(insert OID = 3263 (  pg_lsn_cmp	PGNSP PGUID 12 1 0 0 0 f f f f t f i 2 0 23 "3220 3220" _null_ _null_ _null_ _null_ pg_lsn_cmp _null_ _null_ _null_ ));
+DESCR("less-equal-greater");
 
 /* enum related procs */
 DATA(insert OID = 3504 (  anyenum_in	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 3500 "2275" _null_ _null_ _null_ _null_ anyenum_in _null_ _null_ _null_ ));
diff --git a/src/include/utils/pg_lsn.h b/src/include/utils/pg_lsn.h
index 981fcd6..7dd932d 100644
--- a/src/include/utils/pg_lsn.h
+++ b/src/include/utils/pg_lsn.h
@@ -29,6 +29,8 @@ extern Datum pg_lsn_lt(PG_FUNCTION_ARGS);
 extern Datum pg_lsn_gt(PG_FUNCTION_ARGS);
 extern Datum pg_lsn_le(PG_FUNCTION_ARGS);
 extern Datum pg_lsn_ge(PG_FUNCTION_ARGS);
+extern Datum pg_lsn_cmp(PG_FUNCTION_ARGS);
+extern Datum pg_lsn_hash(PG_FUNCTION_ARGS);
 
 extern Datum pg_lsn_mi(PG_FUNCTION_ARGS);
 
-- 
1.8.5.rc2.dirty

#10

Michael Paquier

michael.paquier@gmail.com

over 11 years ago

In reply to: Andres Freund (#9)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Wed, May 7, 2014 at 8:07 AM, Andres Freund <andres@2ndquadrant.com> wrote:

On 2014-05-06 22:49:07 +0900, Michael Paquier wrote:
FWIW, the format you're using makes applying the patch including the
commit message relatively hard. Consider using git format-patch.

Could you be clearer? By applying a filterdiff command or by using git
diff? The latter has been used for this patch.

+/* handler for btree index operator */
+Datum
+pg_lsn_cmp(PG_FUNCTION_ARGS)
+{
+     XLogRecPtr lsn1 = PG_GETARG_LSN(0);
+     XLogRecPtr lsn2 = PG_GETARG_LSN(1);
+
+     PG_RETURN_INT32(lsn1 == lsn2);
+}
This doesn't look correct. A cmp routine needs to return -1, 0, 1 when a
< b, a = b, a > b respectively. You'll only return 0 and 1 here.

Thanks, I recalled that this morning as well... And my 2nd patch uses
this flow instead:
+Datum
+pg_lsn_cmp(PG_FUNCTION_ARGS)
+{
+       XLogRecPtr lsn1 = PG_GETARG_LSN(0);
+       XLogRecPtr lsn2 = PG_GETARG_LSN(1);
+
+       if (lsn1 < lsn2)
+               PG_RETURN_INT32(-1);
+       if (lsn1 > lsn2)
+               PG_RETURN_INT32(1);
+       PG_RETURN_INT32(0);
+}

+/* hash index support */
+Datum
+pg_lsn_hash(PG_FUNCTION_ARGS)
+{
+     XLogRecPtr lsn = PG_GETARG_LSN(0);
+
+     return hashint8(lsn);
+}
That can't be right either. There's at least two things wrong here:
a) hashint8 takes PG_FUNCTION_ARGS, not a Datum

In this case you may consider changing timestamp_hash@time.c and
time_hash@date.c as well :)
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#11

Andres Freund

andres@2ndquadrant.com

over 11 years ago

In reply to: Michael Paquier (#10)

Re: New pg_lsn type doesn't have hash/btree opclasses

On 2014-05-07 08:16:38 +0900, Michael Paquier wrote:

On Wed, May 7, 2014 at 8:07 AM, Andres Freund <andres@2ndquadrant.com> wrote:

On 2014-05-06 22:49:07 +0900, Michael Paquier wrote:
FWIW, the format you're using makes applying the patch including the
commit message relatively hard. Consider using git format-patch.

Could you be clearer? By applying a filterdiff command or by using git
diff? The latter has been used for this patch.

As I said, consider using git format-patch. Then the patch can be
applied using git am. Resulting in a local commit including your
commit message.

+/* handler for btree index operator */
+Datum
+pg_lsn_cmp(PG_FUNCTION_ARGS)
+{
+     XLogRecPtr lsn1 = PG_GETARG_LSN(0);
+     XLogRecPtr lsn2 = PG_GETARG_LSN(1);
+
+     PG_RETURN_INT32(lsn1 == lsn2);
+}
This doesn't look correct. A cmp routine needs to return -1, 0, 1 when a
< b, a = b, a > b respectively. You'll only return 0 and 1 here.

Thanks, I recalled that this morning as well... And my 2nd patch uses
this flow instead:
+Datum
+pg_lsn_cmp(PG_FUNCTION_ARGS)
+{
+       XLogRecPtr lsn1 = PG_GETARG_LSN(0);
+       XLogRecPtr lsn2 = PG_GETARG_LSN(1);
+
+       if (lsn1 < lsn2)
+               PG_RETURN_INT32(-1);
+       if (lsn1 > lsn2)
+               PG_RETURN_INT32(1);
+       PG_RETURN_INT32(0);
+}

That's nearly what's in the patch I attached.

+/* hash index support */
+Datum
+pg_lsn_hash(PG_FUNCTION_ARGS)
+{
+     XLogRecPtr lsn = PG_GETARG_LSN(0);
+
+     return hashint8(lsn);
+}
That can't be right either. There's at least two things wrong here:
a) hashint8 takes PG_FUNCTION_ARGS, not a Datum

In this case you may consider changing timestamp_hash@time.c and
time_hash@date.c as well :)

Uh. They're different:

Datum
timestamp_hash(PG_FUNCTION_ARGS)
{
/* We can use either hashint8 or hashfloat8 directly */
#ifdef HAVE_INT64_TIMESTAMP
return hashint8(fcinfo);
#else
return hashfloat8(fcinfo);
#endif
}
note it's passing fcinfo, not the datum as you do. Same with
time_hash.. In fact your version crashes when used because it's
dereferencing a int8 as a pointer inside hashfloat8.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#12

Michael Paquier

michael.paquier@gmail.com

over 11 years ago

In reply to: Andres Freund (#11)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Wed, May 7, 2014 at 8:22 AM, Andres Freund <andres@2ndquadrant.com> wrote:

Uh. They're different:

Datum
timestamp_hash(PG_FUNCTION_ARGS)
{
/* We can use either hashint8 or hashfloat8 directly */
#ifdef HAVE_INT64_TIMESTAMP
return hashint8(fcinfo);
#else
return hashfloat8(fcinfo);
#endif
}
note it's passing fcinfo, not the datum as you do. Same with
time_hash.. In fact your version crashes when used because it's
dereferencing a int8 as a pointer inside hashfloat8.

Thanks, didn't notice that fcinfo was used.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#13

Fabrízio de Royes Mello

fabriziomello@gmail.com

over 11 years ago

In reply to: Michael Paquier (#12)

1 attachment(s)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Tue, May 6, 2014 at 8:25 PM, Michael Paquier <michael.paquier@gmail.com>
wrote:

On Wed, May 7, 2014 at 8:22 AM, Andres Freund <andres@2ndquadrant.com>

wrote:

Uh. They're different:

Datum
timestamp_hash(PG_FUNCTION_ARGS)
{
/* We can use either hashint8 or hashfloat8 directly */
#ifdef HAVE_INT64_TIMESTAMP
return hashint8(fcinfo);
#else
return hashfloat8(fcinfo);
#endif
}
note it's passing fcinfo, not the datum as you do. Same with
time_hash.. In fact your version crashes when used because it's
dereferencing a int8 as a pointer inside hashfloat8.

Thanks, didn't notice that fcinfo was used.

Hi all,

If helps, I added some regression tests to the lastest patch.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL

Show quoted text

Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello

Attachments:

support-hash-btree-for-lsn.patchtext/x-diff; charset=US-ASCII; name=support-hash-btree-for-lsn.patchDownload

diff --git a/src/backend/utils/adt/pg_lsn.c b/src/backend/utils/adt/pg_lsn.c
index d1448ae..c021a1e 100644
--- a/src/backend/utils/adt/pg_lsn.c
+++ b/src/backend/utils/adt/pg_lsn.c
@@ -14,6 +14,7 @@
 #include "postgres.h"
 
 #include "funcapi.h"
+#include "access/hash.h"
 #include "libpq/pqformat.h"
 #include "utils/builtins.h"
 #include "utils/pg_lsn.h"
@@ -153,6 +154,27 @@ pg_lsn_ge(PG_FUNCTION_ARGS)
 	PG_RETURN_BOOL(lsn1 >= lsn2);
 }
 
+/* handler for btree index operator */
+Datum
+pg_lsn_cmp(PG_FUNCTION_ARGS)
+{
+	XLogRecPtr a = PG_GETARG_LSN(0);
+	XLogRecPtr b = PG_GETARG_LSN(1);
+
+	if (a > b)
+		PG_RETURN_INT32(1);
+	else if (a == b)
+		PG_RETURN_INT32(0);
+	else
+		PG_RETURN_INT32(-1);
+}
+
+/* hash index support */
+Datum
+pg_lsn_hash(PG_FUNCTION_ARGS)
+{
+	return DirectFunctionCall1(hashint8, PG_GETARG_LSN(0));
+}
 
 /*----------------------------------------------------------
  *	Arithmetic operators on PostgreSQL LSNs.
diff --git a/src/include/catalog/pg_amop.h b/src/include/catalog/pg_amop.h
index 8efd3be..5cd170d 100644
--- a/src/include/catalog/pg_amop.h
+++ b/src/include/catalog/pg_amop.h
@@ -513,6 +513,16 @@ DATA(insert (	2968  2950 2950 4 s 2977	403 0 ));
 DATA(insert (	2968  2950 2950 5 s 2975	403 0 ));
 
 /*
+ * btree pglsn_ops
+ */
+
+DATA(insert (	3260  3220 3220 1 s 3224	403 0 ));
+DATA(insert (	3260  3220 3220 2 s 3226	403 0 ));
+DATA(insert (	3260  3220 3220 3 s 3222	403 0 ));
+DATA(insert (	3260  3220 3220 4 s 3227	403 0 ));
+DATA(insert (	3260  3220 3220 5 s 3225	403 0 ));
+
+/*
  *	hash index _ops
  */
 
@@ -581,6 +591,8 @@ DATA(insert (	2231   1042 1042 1 s 1054 405 0 ));
 DATA(insert (	2235   1033 1033 1 s  974 405 0 ));
 /* uuid_ops */
 DATA(insert (	2969   2950 2950 1 s 2972 405 0 ));
+/* pglsn_ops */
+DATA(insert (	3261   3220 3220 1 s 3222 405 0 ));
 /* numeric_ops */
 DATA(insert (	1998   1700 1700 1 s 1752 405 0 ));
 /* array_ops */
diff --git a/src/include/catalog/pg_amproc.h b/src/include/catalog/pg_amproc.h
index 198b126..369adb9 100644
--- a/src/include/catalog/pg_amproc.h
+++ b/src/include/catalog/pg_amproc.h
@@ -134,6 +134,7 @@ DATA(insert (	2789   27 27 1 2794 ));
 DATA(insert (	2968   2950 2950 1 2960 ));
 DATA(insert (	2994   2249 2249 1 2987 ));
 DATA(insert (	3194   2249 2249 1 3187 ));
+DATA(insert (	3260   3220 3220 1 3263 ));
 DATA(insert (	3522   3500 3500 1 3514 ));
 DATA(insert (	3626   3614 3614 1 3622 ));
 DATA(insert (	3683   3615 3615 1 3668 ));
@@ -174,6 +175,7 @@ DATA(insert (	2229   25 25 1 400 ));
 DATA(insert (	2231   1042 1042 1 1080 ));
 DATA(insert (	2235   1033 1033 1 329 ));
 DATA(insert (	2969   2950 2950 1 2963 ));
+DATA(insert (	3261   3220 3220 1 3262 ));
 DATA(insert (	3523   3500 3500 1 3515 ));
 DATA(insert (	3903   3831 3831 1 3902 ));
 DATA(insert (	4034   3802 3802 1 4045 ));
diff --git a/src/include/catalog/pg_opclass.h b/src/include/catalog/pg_opclass.h
index ecf7063..9a45244 100644
--- a/src/include/catalog/pg_opclass.h
+++ b/src/include/catalog/pg_opclass.h
@@ -215,6 +215,8 @@ DATA(insert (	2742	_reltime_ops		PGNSP PGUID 2745  1024 t 703 ));
 DATA(insert (	2742	_tinterval_ops		PGNSP PGUID 2745  1025 t 704 ));
 DATA(insert (	403		uuid_ops			PGNSP PGUID 2968  2950 t 0 ));
 DATA(insert (	405		uuid_ops			PGNSP PGUID 2969  2950 t 0 ));
+DATA(insert (	403		pglsn_ops			PGNSP PGUID 3260  3220 t 0 ));
+DATA(insert (	405		pglsn_ops			PGNSP PGUID 3261  3220 t 0 ));
 DATA(insert (	403		enum_ops			PGNSP PGUID 3522  3500 t 0 ));
 DATA(insert (	405		enum_ops			PGNSP PGUID 3523  3500 t 0 ));
 DATA(insert (	403		tsvector_ops		PGNSP PGUID 3626  3614 t 0 ));
diff --git a/src/include/catalog/pg_operator.h b/src/include/catalog/pg_operator.h
index f280af4..87ee4eb 100644
--- a/src/include/catalog/pg_operator.h
+++ b/src/include/catalog/pg_operator.h
@@ -1595,7 +1595,7 @@ DATA(insert OID = 2977 (  ">="	   PGNSP PGUID b f f 2950 2950 16 2976 2974 uuid_
 DESCR("greater than or equal");
 
 /* pg_lsn operators */
-DATA(insert OID = 3222 (  "="	   PGNSP PGUID b f f 3220 3220 16 3222 3223 pg_lsn_eq eqsel eqjoinsel ));
+DATA(insert OID = 3222 (  "="	   PGNSP PGUID b t t 3220 3220 16 3222 3223 pg_lsn_eq eqsel eqjoinsel ));
 DESCR("equal");
 DATA(insert OID = 3223 (  "<>"	   PGNSP PGUID b f f 3220 3220 16 3223 3222 pg_lsn_ne neqsel neqjoinsel ));
 DESCR("not equal");
diff --git a/src/include/catalog/pg_opfamily.h b/src/include/catalog/pg_opfamily.h
index 9e8f4ac..29089d5 100644
--- a/src/include/catalog/pg_opfamily.h
+++ b/src/include/catalog/pg_opfamily.h
@@ -134,6 +134,8 @@ DATA(insert OID = 1029 (	783		point_ops		PGNSP PGUID ));
 DATA(insert OID = 2745 (	2742	array_ops		PGNSP PGUID ));
 DATA(insert OID = 2968 (	403		uuid_ops		PGNSP PGUID ));
 DATA(insert OID = 2969 (	405		uuid_ops		PGNSP PGUID ));
+DATA(insert OID = 3260 (	403		pglsn_ops		PGNSP PGUID ));
+DATA(insert OID = 3261 (	405		pglsn_ops		PGNSP PGUID ));
 DATA(insert OID = 3522 (	403		enum_ops		PGNSP PGUID ));
 DATA(insert OID = 3523 (	405		enum_ops		PGNSP PGUID ));
 DATA(insert OID = 3626 (	403		tsvector_ops	PGNSP PGUID ));
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index e601ccd..2fbc8f1 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -4293,6 +4293,10 @@ DATA(insert OID = 3238 (  pg_lsn_recv	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 3
 DESCR("I/O");
 DATA(insert OID = 3239 (  pg_lsn_send	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 17 "3220" _null_ _null_ _null_ _null_ pg_lsn_send _null_ _null_ _null_ ));
 DESCR("I/O");
+DATA(insert OID = 3262 (  pg_lsn_hash	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 23 "3220" _null_ _null_ _null_ _null_ pg_lsn_hash _null_ _null_ _null_ ));
+DESCR("hash");
+DATA(insert OID = 3263 (  pg_lsn_cmp	PGNSP PGUID 12 1 0 0 0 f f f f t f i 2 0 23 "3220 3220" _null_ _null_ _null_ _null_ pg_lsn_cmp _null_ _null_ _null_ ));
+DESCR("less-equal-greater");
 
 /* enum related procs */
 DATA(insert OID = 3504 (  anyenum_in	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 3500 "2275" _null_ _null_ _null_ _null_ anyenum_in _null_ _null_ _null_ ));
diff --git a/src/include/utils/pg_lsn.h b/src/include/utils/pg_lsn.h
index 981fcd6..7dd932d 100644
--- a/src/include/utils/pg_lsn.h
+++ b/src/include/utils/pg_lsn.h
@@ -29,6 +29,8 @@ extern Datum pg_lsn_lt(PG_FUNCTION_ARGS);
 extern Datum pg_lsn_gt(PG_FUNCTION_ARGS);
 extern Datum pg_lsn_le(PG_FUNCTION_ARGS);
 extern Datum pg_lsn_ge(PG_FUNCTION_ARGS);
+extern Datum pg_lsn_cmp(PG_FUNCTION_ARGS);
+extern Datum pg_lsn_hash(PG_FUNCTION_ARGS);
 
 extern Datum pg_lsn_mi(PG_FUNCTION_ARGS);
 
diff --git a/src/test/regress/expected/pg_lsn.out b/src/test/regress/expected/pg_lsn.out
index 504768c..cdd8103 100644
--- a/src/test/regress/expected/pg_lsn.out
+++ b/src/test/regress/expected/pg_lsn.out
@@ -1,32 +1,31 @@
 --
 -- PG_LSN
 --
-CREATE TABLE PG_LSN_TBL (f1 pg_lsn);
+CREATE TABLE pg_lsn_tbl (f1 pg_lsn);
 -- Largest and smallest input
-INSERT INTO PG_LSN_TBL VALUES ('0/0');
-INSERT INTO PG_LSN_TBL VALUES ('FFFFFFFF/FFFFFFFF');
+INSERT INTO pg_lsn_tbl VALUES ('0/0');
+INSERT INTO pg_lsn_tbl VALUES ('FFFFFFFF/FFFFFFFF');
 -- Incorrect input
-INSERT INTO PG_LSN_TBL VALUES ('G/0');
+INSERT INTO pg_lsn_tbl VALUES ('G/0');
 ERROR:  invalid input syntax for type pg_lsn: "G/0"
-LINE 1: INSERT INTO PG_LSN_TBL VALUES ('G/0');
+LINE 1: INSERT INTO pg_lsn_tbl VALUES ('G/0');
                                        ^
-INSERT INTO PG_LSN_TBL VALUES ('-1/0');
+INSERT INTO pg_lsn_tbl VALUES ('-1/0');
 ERROR:  invalid input syntax for type pg_lsn: "-1/0"
-LINE 1: INSERT INTO PG_LSN_TBL VALUES ('-1/0');
+LINE 1: INSERT INTO pg_lsn_tbl VALUES ('-1/0');
                                        ^
-INSERT INTO PG_LSN_TBL VALUES (' 0/12345678');
+INSERT INTO pg_lsn_tbl VALUES (' 0/12345678');
 ERROR:  invalid input syntax for type pg_lsn: " 0/12345678"
-LINE 1: INSERT INTO PG_LSN_TBL VALUES (' 0/12345678');
+LINE 1: INSERT INTO pg_lsn_tbl VALUES (' 0/12345678');
                                        ^
-INSERT INTO PG_LSN_TBL VALUES ('ABCD/');
+INSERT INTO pg_lsn_tbl VALUES ('ABCD/');
 ERROR:  invalid input syntax for type pg_lsn: "ABCD/"
-LINE 1: INSERT INTO PG_LSN_TBL VALUES ('ABCD/');
+LINE 1: INSERT INTO pg_lsn_tbl VALUES ('ABCD/');
                                        ^
-INSERT INTO PG_LSN_TBL VALUES ('/ABCD');
+INSERT INTO pg_lsn_tbl VALUES ('/ABCD');
 ERROR:  invalid input syntax for type pg_lsn: "/ABCD"
-LINE 1: INSERT INTO PG_LSN_TBL VALUES ('/ABCD');
+LINE 1: INSERT INTO pg_lsn_tbl VALUES ('/ABCD');
                                        ^
-DROP TABLE PG_LSN_TBL;
 -- Operators
 SELECT '0/16AE7F8' = '0/16AE7F8'::pg_lsn;
  ?column? 
@@ -64,3 +63,26 @@ SELECT '0/16AE7F8'::pg_lsn - '0/16AE7F7'::pg_lsn;
         1
 (1 row)
 
+-- Operator Class
+TRUNCATE pg_lsn_tbl;
+INSERT INTO pg_lsn_tbl SELECT DISTINCT (g.i||'/0')::pg_lsn f FROM generate_series(1, 5) g(i), generate_series(1, 5);
+SELECT * FROM pg_lsn_tbl ORDER BY 1;
+ f1  
+-----
+ 1/0
+ 2/0
+ 3/0
+ 4/0
+ 5/0
+(5 rows)
+
+CREATE INDEX pg_lsn_tbl_1 ON pg_lsn_tbl USING btree (f1);
+CREATE INDEX pg_lsn_tbl_2 ON pg_lsn_tbl USING hash (f1);
+SELECT indexdef FROM pg_indexes WHERE tablename = 'pg_lsn_tbl' ORDER BY 1;
+                         indexdef                         
+----------------------------------------------------------
+ CREATE INDEX pg_lsn_tbl_1 ON pg_lsn_tbl USING btree (f1)
+ CREATE INDEX pg_lsn_tbl_2 ON pg_lsn_tbl USING hash (f1)
+(2 rows)
+
+DROP TABLE pg_lsn_tbl;
diff --git a/src/test/regress/sql/pg_lsn.sql b/src/test/regress/sql/pg_lsn.sql
index 1634d37..da505de 100644
--- a/src/test/regress/sql/pg_lsn.sql
+++ b/src/test/regress/sql/pg_lsn.sql
@@ -2,19 +2,18 @@
 -- PG_LSN
 --
 
-CREATE TABLE PG_LSN_TBL (f1 pg_lsn);
+CREATE TABLE pg_lsn_tbl (f1 pg_lsn);
 
 -- Largest and smallest input
-INSERT INTO PG_LSN_TBL VALUES ('0/0');
-INSERT INTO PG_LSN_TBL VALUES ('FFFFFFFF/FFFFFFFF');
+INSERT INTO pg_lsn_tbl VALUES ('0/0');
+INSERT INTO pg_lsn_tbl VALUES ('FFFFFFFF/FFFFFFFF');
 
 -- Incorrect input
-INSERT INTO PG_LSN_TBL VALUES ('G/0');
-INSERT INTO PG_LSN_TBL VALUES ('-1/0');
-INSERT INTO PG_LSN_TBL VALUES (' 0/12345678');
-INSERT INTO PG_LSN_TBL VALUES ('ABCD/');
-INSERT INTO PG_LSN_TBL VALUES ('/ABCD');
-DROP TABLE PG_LSN_TBL;
+INSERT INTO pg_lsn_tbl VALUES ('G/0');
+INSERT INTO pg_lsn_tbl VALUES ('-1/0');
+INSERT INTO pg_lsn_tbl VALUES (' 0/12345678');
+INSERT INTO pg_lsn_tbl VALUES ('ABCD/');
+INSERT INTO pg_lsn_tbl VALUES ('/ABCD');
 
 -- Operators
 SELECT '0/16AE7F8' = '0/16AE7F8'::pg_lsn;
@@ -23,3 +22,12 @@ SELECT '0/16AE7F7' < '0/16AE7F8'::pg_lsn;
 SELECT '0/16AE7F8' > pg_lsn '0/16AE7F7';
 SELECT '0/16AE7F7'::pg_lsn - '0/16AE7F8'::pg_lsn;
 SELECT '0/16AE7F8'::pg_lsn - '0/16AE7F7'::pg_lsn;
+
+-- Operator Class
+TRUNCATE pg_lsn_tbl;
+INSERT INTO pg_lsn_tbl SELECT DISTINCT (g.i||'/0')::pg_lsn f FROM generate_series(1, 5) g(i), generate_series(1, 5);
+SELECT * FROM pg_lsn_tbl ORDER BY 1;
+CREATE INDEX pg_lsn_tbl_1 ON pg_lsn_tbl USING btree (f1);
+CREATE INDEX pg_lsn_tbl_2 ON pg_lsn_tbl USING hash (f1);
+SELECT indexdef FROM pg_indexes WHERE tablename = 'pg_lsn_tbl' ORDER BY 1;
+DROP TABLE pg_lsn_tbl;

#14

Craig Ringer

craig@2ndquadrant.com

over 11 years ago

In reply to: Michael Paquier (#10)

Re: New pg_lsn type doesn't have hash/btree opclasses

On 05/07/2014 07:16 AM, Michael Paquier wrote:

On Wed, May 7, 2014 at 8:07 AM, Andres Freund <andres@2ndquadrant.com> wrote:

On 2014-05-06 22:49:07 +0900, Michael Paquier wrote:
FWIW, the format you're using makes applying the patch including the
commit message relatively hard. Consider using git format-patch.

Could you be clearer? By applying a filterdiff command or by using git
diff? The latter has been used for this patch.

git format-patch -1

is usually what you want to emit a patch for the top commit. To produce
a patchset between the current branch and master:

git format-patch master

It doesn't use context diff, but I think people here have mostly stopped
caring about that now (?).

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#15

Michael Paquier

michael.paquier@gmail.com

over 11 years ago

In reply to: Craig Ringer (#14)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Wed, May 7, 2014 at 1:21 PM, Craig Ringer <craig@2ndquadrant.com> wrote:

On 05/07/2014 07:16 AM, Michael Paquier wrote:

On Wed, May 7, 2014 at 8:07 AM, Andres Freund <andres@2ndquadrant.com> wrote:

On 2014-05-06 22:49:07 +0900, Michael Paquier wrote:
FWIW, the format you're using makes applying the patch including the
commit message relatively hard. Consider using git format-patch.

Could you be clearer? By applying a filterdiff command or by using git
diff? The latter has been used for this patch.

git format-patch -1

is usually what you want to emit a patch for the top commit. To produce
a patchset between the current branch and master:

git format-patch master

It doesn't use context diff, but I think people here have mostly stopped
caring about that now (?).

Thanks for the details, I didn't know this subcommand of git.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#16

Andres Freund

andres@2ndquadrant.com

over 11 years ago

In reply to: Fabrízio de Royes Mello (#13)

Re: New pg_lsn type doesn't have hash/btree opclasses

Hi,

On 2014-05-06 23:55:04 -0300, Fabrízio de Royes Mello wrote:

If helps, I added some regression tests to the lastest patch.

I think we should apply this patch now. It's much more sensible with the
opclasses present and we don't win anything by waiting for 9.5.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#17

Fujii Masao

masao.fujii@gmail.com

over 11 years ago

In reply to: Fabrízio de Royes Mello (#13)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Wed, May 7, 2014 at 11:55 AM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:

On Tue, May 6, 2014 at 8:25 PM, Michael Paquier <michael.paquier@gmail.com>
wrote:

On Wed, May 7, 2014 at 8:22 AM, Andres Freund <andres@2ndquadrant.com>
wrote:

Uh. They're different:

Datum
timestamp_hash(PG_FUNCTION_ARGS)
{
/* We can use either hashint8 or hashfloat8 directly */
#ifdef HAVE_INT64_TIMESTAMP
return hashint8(fcinfo);
#else
return hashfloat8(fcinfo);
#endif
}
note it's passing fcinfo, not the datum as you do. Same with
time_hash.. In fact your version crashes when used because it's
dereferencing a int8 as a pointer inside hashfloat8.

Thanks, didn't notice that fcinfo was used.

Hi all,

If helps, I added some regression tests to the lastest patch.

+DATA(insert OID = 3260 (    403        pglsn_ops        PGNSP PGUID ));
+DATA(insert OID = 3261 (    405        pglsn_ops        PGNSP PGUID ));

The patch looks good to me except the name of index operator class.
I think that "pg_lsn_ops" is better than "pglsn_ops" because it's for "pg_lsn"
data type.

Regards,

--
Fujii Masao

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#18

Andres Freund

andres@2ndquadrant.com

over 11 years ago

In reply to: Fujii Masao (#17)

Re: New pg_lsn type doesn't have hash/btree opclasses

On 2014-05-09 22:01:07 +0900, Fujii Masao wrote:

On Wed, May 7, 2014 at 11:55 AM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:

On Tue, May 6, 2014 at 8:25 PM, Michael Paquier <michael.paquier@gmail.com>
wrote:

On Wed, May 7, 2014 at 8:22 AM, Andres Freund <andres@2ndquadrant.com>
wrote:

Uh. They're different:

Datum
timestamp_hash(PG_FUNCTION_ARGS)
{
/* We can use either hashint8 or hashfloat8 directly */
#ifdef HAVE_INT64_TIMESTAMP
return hashint8(fcinfo);
#else
return hashfloat8(fcinfo);
#endif
}
note it's passing fcinfo, not the datum as you do. Same with
time_hash.. In fact your version crashes when used because it's
dereferencing a int8 as a pointer inside hashfloat8.

Thanks, didn't notice that fcinfo was used.

Hi all,

If helps, I added some regression tests to the lastest patch.
+DATA(insert OID = 3260 (    403        pglsn_ops        PGNSP PGUID ));
+DATA(insert OID = 3261 (    405        pglsn_ops        PGNSP PGUID ));
The patch looks good to me except the name of index operator class.

FWIW, I've tested and looked through the patch as well.

I think that "pg_lsn_ops" is better than "pglsn_ops" because it's for "pg_lsn"
data type.

You're right, that's marginally prettier.

You plan to commit it?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#19

Michael Paquier

michael.paquier@gmail.com

over 11 years ago

In reply to: Fujii Masao (#17)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Fri, May 9, 2014 at 10:01 PM, Fujii Masao <masao.fujii@gmail.com> wrote:

+DATA(insert OID = 3260 (    403        pglsn_ops        PGNSP PGUID ));
+DATA(insert OID = 3261 (    405        pglsn_ops        PGNSP PGUID ));
The patch looks good to me except the name of index operator class.
I think that "pg_lsn_ops" is better than "pglsn_ops" because it's for "pg_lsn"
data type.

Well, yes... Btw, before doing anything with this patch, I think that
the version of Fabrizio could be used as a base, but the regression
tests should be reshuffled a bit...
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#20

Andres Freund

andres@2ndquadrant.com

over 11 years ago

In reply to: Michael Paquier (#19)

Re: New pg_lsn type doesn't have hash/btree opclasses

On 2014-05-09 22:14:25 +0900, Michael Paquier wrote:

On Fri, May 9, 2014 at 10:01 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
+DATA(insert OID = 3260 (    403        pglsn_ops        PGNSP PGUID ));
+DATA(insert OID = 3261 (    405        pglsn_ops        PGNSP PGUID ));
The patch looks good to me except the name of index operator class.
I think that "pg_lsn_ops" is better than "pglsn_ops" because it's for "pg_lsn"
data type.
Well, yes... Btw, before doing anything with this patch, I think that
the version of Fabrizio could be used as a base, but the regression
tests should be reshuffled a bit...

I honestly don't really see much point in the added tests. If at all I'd
rather see my tests from
http://archives.postgresql.org/message-id/20140506230722.GE24808%40awork2.anarazel.de
addded. With some EXPLAIN (COSTS OFF) they'd test more.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#21

Fujii Masao

masao.fujii@gmail.com

over 11 years ago

In reply to: Andres Freund (#18)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Fri, May 9, 2014 at 10:03 PM, Andres Freund <andres@2ndquadrant.com> wrote:

On 2014-05-09 22:01:07 +0900, Fujii Masao wrote:
On Wed, May 7, 2014 at 11:55 AM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:

On Tue, May 6, 2014 at 8:25 PM, Michael Paquier <michael.paquier@gmail.com>
wrote:

On Wed, May 7, 2014 at 8:22 AM, Andres Freund <andres@2ndquadrant.com>
wrote:

Uh. They're different:

Datum
timestamp_hash(PG_FUNCTION_ARGS)
{
/* We can use either hashint8 or hashfloat8 directly */
#ifdef HAVE_INT64_TIMESTAMP
return hashint8(fcinfo);
#else
return hashfloat8(fcinfo);
#endif
}
note it's passing fcinfo, not the datum as you do. Same with
time_hash.. In fact your version crashes when used because it's
dereferencing a int8 as a pointer inside hashfloat8.

Thanks, didn't notice that fcinfo was used.

Hi all,

If helps, I added some regression tests to the lastest patch.
+DATA(insert OID = 3260 (    403        pglsn_ops        PGNSP PGUID ));
+DATA(insert OID = 3261 (    405        pglsn_ops        PGNSP PGUID ));
The patch looks good to me except the name of index operator class.
FWIW, I've tested and looked through the patch as well.

I think that "pg_lsn_ops" is better than "pglsn_ops" because it's for "pg_lsn"
data type.

You're right, that's marginally prettier.

You plan to commit it?

Yes unless many people object the commit.

Michael,
You're now modifying the patch?

Regards,

--
Fujii Masao

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#22

Michael Paquier

michael.paquier@gmail.com

over 11 years ago

In reply to: Fujii Masao (#21)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Fri, May 9, 2014 at 10:49 PM, Fujii Masao <masao.fujii@gmail.com> wrote:

Yes unless many people object the commit.

Michael,
You're now modifying the patch?

Not within a couple of days.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#23

Tom Lane

tgl@sss.pgh.pa.us

over 11 years ago

In reply to: Michael Paquier (#22)

Re: New pg_lsn type doesn't have hash/btree opclasses

Michael Paquier <michael.paquier@gmail.com> writes:

On Fri, May 9, 2014 at 10:49 PM, Fujii Masao <masao.fujii@gmail.com> wrote:

Yes unless many people object the commit.

Michael,
You're now modifying the patch?

Not within a couple of days.

I think it's really too late for this for 9.4. At this point it's
less than 48 hours until beta1 wraps, and we do not have the bandwidth
to do anything but worry about stabilizing the features we've already
got.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#24

Fabrízio de Royes Mello

fabriziomello@gmail.com

over 11 years ago

In reply to: Andres Freund (#20)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Fri, May 9, 2014 at 10:18 AM, Andres Freund <andres@2ndquadrant.com>
wrote:

On 2014-05-09 22:14:25 +0900, Michael Paquier wrote:

On Fri, May 9, 2014 at 10:01 PM, Fujii Masao <masao.fujii@gmail.com>

wrote:

+DATA(insert OID = 3260 ( 403 pglsn_ops PGNSP PGUID

));

+DATA(insert OID = 3261 ( 405 pglsn_ops PGNSP PGUID

));

The patch looks good to me except the name of index operator class.
I think that "pg_lsn_ops" is better than "pglsn_ops" because it's for

"pg_lsn"

data type.

Well, yes... Btw, before doing anything with this patch, I think that
the version of Fabrizio could be used as a base, but the regression
tests should be reshuffled a bit...

I honestly don't really see much point in the added tests. If at all I'd
rather see my tests from

http://archives.postgresql.org/message-id/20140506230722.GE24808%40awork2.anarazel.de

addded. With some EXPLAIN (COSTS OFF) they'd test more.

Ok. In a few hours I'll add your suggestion and send it again.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL

Show quoted text

Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello

#25

Fabrízio de Royes Mello

fabriziomello@gmail.com

over 11 years ago

In reply to: Tom Lane (#23)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Fri, May 9, 2014 at 8:42 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Michael Paquier <michael.paquier@gmail.com> writes:

On Fri, May 9, 2014 at 10:49 PM, Fujii Masao <masao.fujii@gmail.com>

wrote:

Yes unless many people object the commit.

Michael,
You're now modifying the patch?

Not within a couple of days.

I think it's really too late for this for 9.4. At this point it's
less than 48 hours until beta1 wraps, and we do not have the bandwidth
to do anything but worry about stabilizing the features we've already
got.

But it's a very small change with many benefits, and Michael acted very
proactive to make this happens.

In a few hours I'll fix the last two suggestions (Fujii and Andres) and
send it again.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL

Show quoted text

Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello

#26

Tom Lane

tgl@sss.pgh.pa.us

over 11 years ago

In reply to: Fabrízio de Royes Mello (#25)

Re: New pg_lsn type doesn't have hash/btree opclasses

=?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?= <fabriziomello@gmail.com> writes:

On Fri, May 9, 2014 at 8:42 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I think it's really too late for this for 9.4. At this point it's
less than 48 hours until beta1 wraps, and we do not have the bandwidth
to do anything but worry about stabilizing the features we've already
got.

But it's a very small change with many benefits, and Michael acted very
proactive to make this happens.

[ shrug... ] "proactive" would have been doing this a month ago.
If we're going to ship a release, we have to stop taking new features
at some point, and we are really past that point for 9.4.

And, to be blunt, this is not important enough to hold up the release
for, nor to take any stability risks for. It should go into the next
commitfest cycle where it can get a non-rushed review.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#27

Fabrízio de Royes Mello

fabriziomello@gmail.com

over 11 years ago

In reply to: Tom Lane (#26)

1 attachment(s)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Fri, May 9, 2014 at 9:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

=?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?= <fabriziomello@gmail.com> writes:

On Fri, May 9, 2014 at 8:42 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I think it's really too late for this for 9.4. At this point it's
less than 48 hours until beta1 wraps, and we do not have the bandwidth
to do anything but worry about stabilizing the features we've already
got.

But it's a very small change with many benefits, and Michael acted very
proactive to make this happens.

[ shrug... ] "proactive" would have been doing this a month ago.
If we're going to ship a release, we have to stop taking new features
at some point, and we are really past that point for 9.4.

And, to be blunt, this is not important enough to hold up the release
for, nor to take any stability risks for. It should go into the next
commitfest cycle where it can get a non-rushed review.

I agree with you that is too late to add *new features*.

But I agree with Andres when he said this is a regression introcuced in the
pg_lsn patch.

So we'll release a version that break a simple query like that:

fabrizio=# SELECT DISTINCT (g.i||'/0')::pg_lsn f FROM generate_series(1,
100) g(i), generate_series(1, 5);
ERROR: could not identify an equality operator for type pg_lsn
LINE 1: SELECT DISTINCT (g.i||'/0')::pg_lsn f FROM generate_series(1...
^

I attached the last version of this fix with the Andres and Fujii
suggestions.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL

Show quoted text

Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello

Attachments:

support-hash-btree-for-lsn_v2.patchtext/x-patch; charset=US-ASCII; name=support-hash-btree-for-lsn_v2.patchDownload

diff --git a/src/backend/utils/adt/pg_lsn.c b/src/backend/utils/adt/pg_lsn.c
index d1448ae..c021a1e 100644
--- a/src/backend/utils/adt/pg_lsn.c
+++ b/src/backend/utils/adt/pg_lsn.c
@@ -14,6 +14,7 @@
 #include "postgres.h"
 
 #include "funcapi.h"
+#include "access/hash.h"
 #include "libpq/pqformat.h"
 #include "utils/builtins.h"
 #include "utils/pg_lsn.h"
@@ -153,6 +154,27 @@ pg_lsn_ge(PG_FUNCTION_ARGS)
 	PG_RETURN_BOOL(lsn1 >= lsn2);
 }
 
+/* handler for btree index operator */
+Datum
+pg_lsn_cmp(PG_FUNCTION_ARGS)
+{
+	XLogRecPtr a = PG_GETARG_LSN(0);
+	XLogRecPtr b = PG_GETARG_LSN(1);
+
+	if (a > b)
+		PG_RETURN_INT32(1);
+	else if (a == b)
+		PG_RETURN_INT32(0);
+	else
+		PG_RETURN_INT32(-1);
+}
+
+/* hash index support */
+Datum
+pg_lsn_hash(PG_FUNCTION_ARGS)
+{
+	return DirectFunctionCall1(hashint8, PG_GETARG_LSN(0));
+}
 
 /*----------------------------------------------------------
  *	Arithmetic operators on PostgreSQL LSNs.
diff --git a/src/include/catalog/pg_amop.h b/src/include/catalog/pg_amop.h
index 8efd3be..35fd17e 100644
--- a/src/include/catalog/pg_amop.h
+++ b/src/include/catalog/pg_amop.h
@@ -513,6 +513,16 @@ DATA(insert (	2968  2950 2950 4 s 2977	403 0 ));
 DATA(insert (	2968  2950 2950 5 s 2975	403 0 ));
 
 /*
+ * btree pg_lsn_ops
+ */
+
+DATA(insert (	3260  3220 3220 1 s 3224	403 0 ));
+DATA(insert (	3260  3220 3220 2 s 3226	403 0 ));
+DATA(insert (	3260  3220 3220 3 s 3222	403 0 ));
+DATA(insert (	3260  3220 3220 4 s 3227	403 0 ));
+DATA(insert (	3260  3220 3220 5 s 3225	403 0 ));
+
+/*
  *	hash index _ops
  */
 
@@ -581,6 +591,8 @@ DATA(insert (	2231   1042 1042 1 s 1054 405 0 ));
 DATA(insert (	2235   1033 1033 1 s  974 405 0 ));
 /* uuid_ops */
 DATA(insert (	2969   2950 2950 1 s 2972 405 0 ));
+/* pg_lsn_ops */
+DATA(insert (	3261   3220 3220 1 s 3222 405 0 ));
 /* numeric_ops */
 DATA(insert (	1998   1700 1700 1 s 1752 405 0 ));
 /* array_ops */
diff --git a/src/include/catalog/pg_amproc.h b/src/include/catalog/pg_amproc.h
index 198b126..369adb9 100644
--- a/src/include/catalog/pg_amproc.h
+++ b/src/include/catalog/pg_amproc.h
@@ -134,6 +134,7 @@ DATA(insert (	2789   27 27 1 2794 ));
 DATA(insert (	2968   2950 2950 1 2960 ));
 DATA(insert (	2994   2249 2249 1 2987 ));
 DATA(insert (	3194   2249 2249 1 3187 ));
+DATA(insert (	3260   3220 3220 1 3263 ));
 DATA(insert (	3522   3500 3500 1 3514 ));
 DATA(insert (	3626   3614 3614 1 3622 ));
 DATA(insert (	3683   3615 3615 1 3668 ));
@@ -174,6 +175,7 @@ DATA(insert (	2229   25 25 1 400 ));
 DATA(insert (	2231   1042 1042 1 1080 ));
 DATA(insert (	2235   1033 1033 1 329 ));
 DATA(insert (	2969   2950 2950 1 2963 ));
+DATA(insert (	3261   3220 3220 1 3262 ));
 DATA(insert (	3523   3500 3500 1 3515 ));
 DATA(insert (	3903   3831 3831 1 3902 ));
 DATA(insert (	4034   3802 3802 1 4045 ));
diff --git a/src/include/catalog/pg_opclass.h b/src/include/catalog/pg_opclass.h
index ecf7063..2298a83 100644
--- a/src/include/catalog/pg_opclass.h
+++ b/src/include/catalog/pg_opclass.h
@@ -215,6 +215,8 @@ DATA(insert (	2742	_reltime_ops		PGNSP PGUID 2745  1024 t 703 ));
 DATA(insert (	2742	_tinterval_ops		PGNSP PGUID 2745  1025 t 704 ));
 DATA(insert (	403		uuid_ops			PGNSP PGUID 2968  2950 t 0 ));
 DATA(insert (	405		uuid_ops			PGNSP PGUID 2969  2950 t 0 ));
+DATA(insert (	403		pg_lsn_ops			PGNSP PGUID 3260  3220 t 0 ));
+DATA(insert (	405		pg_lsn_ops			PGNSP PGUID 3261  3220 t 0 ));
 DATA(insert (	403		enum_ops			PGNSP PGUID 3522  3500 t 0 ));
 DATA(insert (	405		enum_ops			PGNSP PGUID 3523  3500 t 0 ));
 DATA(insert (	403		tsvector_ops		PGNSP PGUID 3626  3614 t 0 ));
diff --git a/src/include/catalog/pg_operator.h b/src/include/catalog/pg_operator.h
index f280af4..87ee4eb 100644
--- a/src/include/catalog/pg_operator.h
+++ b/src/include/catalog/pg_operator.h
@@ -1595,7 +1595,7 @@ DATA(insert OID = 2977 (  ">="	   PGNSP PGUID b f f 2950 2950 16 2976 2974 uuid_
 DESCR("greater than or equal");
 
 /* pg_lsn operators */
-DATA(insert OID = 3222 (  "="	   PGNSP PGUID b f f 3220 3220 16 3222 3223 pg_lsn_eq eqsel eqjoinsel ));
+DATA(insert OID = 3222 (  "="	   PGNSP PGUID b t t 3220 3220 16 3222 3223 pg_lsn_eq eqsel eqjoinsel ));
 DESCR("equal");
 DATA(insert OID = 3223 (  "<>"	   PGNSP PGUID b f f 3220 3220 16 3223 3222 pg_lsn_ne neqsel neqjoinsel ));
 DESCR("not equal");
diff --git a/src/include/catalog/pg_opfamily.h b/src/include/catalog/pg_opfamily.h
index 9e8f4ac..84f66ac 100644
--- a/src/include/catalog/pg_opfamily.h
+++ b/src/include/catalog/pg_opfamily.h
@@ -134,6 +134,8 @@ DATA(insert OID = 1029 (	783		point_ops		PGNSP PGUID ));
 DATA(insert OID = 2745 (	2742	array_ops		PGNSP PGUID ));
 DATA(insert OID = 2968 (	403		uuid_ops		PGNSP PGUID ));
 DATA(insert OID = 2969 (	405		uuid_ops		PGNSP PGUID ));
+DATA(insert OID = 3260 (	403		pg_lsn_ops		PGNSP PGUID ));
+DATA(insert OID = 3261 (	405		pg_lsn_ops		PGNSP PGUID ));
 DATA(insert OID = 3522 (	403		enum_ops		PGNSP PGUID ));
 DATA(insert OID = 3523 (	405		enum_ops		PGNSP PGUID ));
 DATA(insert OID = 3626 (	403		tsvector_ops	PGNSP PGUID ));
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index e601ccd..2fbc8f1 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -4293,6 +4293,10 @@ DATA(insert OID = 3238 (  pg_lsn_recv	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 3
 DESCR("I/O");
 DATA(insert OID = 3239 (  pg_lsn_send	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 17 "3220" _null_ _null_ _null_ _null_ pg_lsn_send _null_ _null_ _null_ ));
 DESCR("I/O");
+DATA(insert OID = 3262 (  pg_lsn_hash	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 23 "3220" _null_ _null_ _null_ _null_ pg_lsn_hash _null_ _null_ _null_ ));
+DESCR("hash");
+DATA(insert OID = 3263 (  pg_lsn_cmp	PGNSP PGUID 12 1 0 0 0 f f f f t f i 2 0 23 "3220 3220" _null_ _null_ _null_ _null_ pg_lsn_cmp _null_ _null_ _null_ ));
+DESCR("less-equal-greater");
 
 /* enum related procs */
 DATA(insert OID = 3504 (  anyenum_in	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 3500 "2275" _null_ _null_ _null_ _null_ anyenum_in _null_ _null_ _null_ ));
diff --git a/src/include/utils/pg_lsn.h b/src/include/utils/pg_lsn.h
index 981fcd6..7dd932d 100644
--- a/src/include/utils/pg_lsn.h
+++ b/src/include/utils/pg_lsn.h
@@ -29,6 +29,8 @@ extern Datum pg_lsn_lt(PG_FUNCTION_ARGS);
 extern Datum pg_lsn_gt(PG_FUNCTION_ARGS);
 extern Datum pg_lsn_le(PG_FUNCTION_ARGS);
 extern Datum pg_lsn_ge(PG_FUNCTION_ARGS);
+extern Datum pg_lsn_cmp(PG_FUNCTION_ARGS);
+extern Datum pg_lsn_hash(PG_FUNCTION_ARGS);
 
 extern Datum pg_lsn_mi(PG_FUNCTION_ARGS);
 
diff --git a/src/test/regress/expected/pg_lsn.out b/src/test/regress/expected/pg_lsn.out
index 504768c..8421e15 100644
--- a/src/test/regress/expected/pg_lsn.out
+++ b/src/test/regress/expected/pg_lsn.out
@@ -1,32 +1,31 @@
 --
 -- PG_LSN
 --
-CREATE TABLE PG_LSN_TBL (f1 pg_lsn);
+CREATE TABLE pg_lsn_tbl (f1 pg_lsn);
 -- Largest and smallest input
-INSERT INTO PG_LSN_TBL VALUES ('0/0');
-INSERT INTO PG_LSN_TBL VALUES ('FFFFFFFF/FFFFFFFF');
+INSERT INTO pg_lsn_tbl VALUES ('0/0');
+INSERT INTO pg_lsn_tbl VALUES ('FFFFFFFF/FFFFFFFF');
 -- Incorrect input
-INSERT INTO PG_LSN_TBL VALUES ('G/0');
+INSERT INTO pg_lsn_tbl VALUES ('G/0');
 ERROR:  invalid input syntax for type pg_lsn: "G/0"
-LINE 1: INSERT INTO PG_LSN_TBL VALUES ('G/0');
+LINE 1: INSERT INTO pg_lsn_tbl VALUES ('G/0');
                                        ^
-INSERT INTO PG_LSN_TBL VALUES ('-1/0');
+INSERT INTO pg_lsn_tbl VALUES ('-1/0');
 ERROR:  invalid input syntax for type pg_lsn: "-1/0"
-LINE 1: INSERT INTO PG_LSN_TBL VALUES ('-1/0');
+LINE 1: INSERT INTO pg_lsn_tbl VALUES ('-1/0');
                                        ^
-INSERT INTO PG_LSN_TBL VALUES (' 0/12345678');
+INSERT INTO pg_lsn_tbl VALUES (' 0/12345678');
 ERROR:  invalid input syntax for type pg_lsn: " 0/12345678"
-LINE 1: INSERT INTO PG_LSN_TBL VALUES (' 0/12345678');
+LINE 1: INSERT INTO pg_lsn_tbl VALUES (' 0/12345678');
                                        ^
-INSERT INTO PG_LSN_TBL VALUES ('ABCD/');
+INSERT INTO pg_lsn_tbl VALUES ('ABCD/');
 ERROR:  invalid input syntax for type pg_lsn: "ABCD/"
-LINE 1: INSERT INTO PG_LSN_TBL VALUES ('ABCD/');
+LINE 1: INSERT INTO pg_lsn_tbl VALUES ('ABCD/');
                                        ^
-INSERT INTO PG_LSN_TBL VALUES ('/ABCD');
+INSERT INTO pg_lsn_tbl VALUES ('/ABCD');
 ERROR:  invalid input syntax for type pg_lsn: "/ABCD"
-LINE 1: INSERT INTO PG_LSN_TBL VALUES ('/ABCD');
+LINE 1: INSERT INTO pg_lsn_tbl VALUES ('/ABCD');
                                        ^
-DROP TABLE PG_LSN_TBL;
 -- Operators
 SELECT '0/16AE7F8' = '0/16AE7F8'::pg_lsn;
  ?column? 
@@ -64,3 +63,75 @@ SELECT '0/16AE7F8'::pg_lsn - '0/16AE7F7'::pg_lsn;
         1
 (1 row)
 
+-- Operator Class
+TRUNCATE pg_lsn_tbl;
+INSERT INTO pg_lsn_tbl SELECT DISTINCT (g.i||'/0')::pg_lsn f FROM generate_series(1, 5) g(i), generate_series(1, 5);
+SELECT * FROM pg_lsn_tbl ORDER BY 1;
+ f1  
+-----
+ 1/0
+ 2/0
+ 3/0
+ 4/0
+ 5/0
+(5 rows)
+
+CREATE INDEX pg_lsn_tbl_1 ON pg_lsn_tbl USING btree (f1);
+CREATE INDEX pg_lsn_tbl_2 ON pg_lsn_tbl USING hash (f1);
+SELECT indexdef FROM pg_indexes WHERE tablename = 'pg_lsn_tbl' ORDER BY 1;
+                         indexdef                         
+----------------------------------------------------------
+ CREATE INDEX pg_lsn_tbl_1 ON pg_lsn_tbl USING btree (f1)
+ CREATE INDEX pg_lsn_tbl_2 ON pg_lsn_tbl USING hash (f1)
+(2 rows)
+
+DROP TABLE pg_lsn_tbl;
+EXPLAIN (COSTS OFF) SELECT DISTINCT (g.i||'/0')::pg_lsn f FROM generate_series(1, 100) g(i), generate_series(1, 5);
+                     QUERY PLAN                     
+----------------------------------------------------
+ HashAggregate
+   Group Key: (((g.i)::text || '/0'::text))::pg_lsn
+   ->  Nested Loop
+         ->  Function Scan on generate_series g
+         ->  Function Scan on generate_series
+(5 rows)
+
+EXPLAIN (COSTS OFF) SELECT (g.i||'/0')::pg_lsn f FROM generate_series(1, 100) g(i) ORDER BY f;
+                    QUERY PLAN                     
+---------------------------------------------------
+ Sort
+   Sort Key: ((((i)::text || '/0'::text))::pg_lsn)
+   ->  Function Scan on generate_series g
+(3 rows)
+
+EXPLAIN (COSTS OFF) SELECT (g.i||'/0')::pg_lsn, count(*) FROM generate_series(1, 100) g(i), generate_series(1, 5) GROUP BY 1 ORDER BY 1;
+                        QUERY PLAN                        
+----------------------------------------------------------
+ Sort
+   Sort Key: ((((g.i)::text || '/0'::text))::pg_lsn)
+   ->  HashAggregate
+         Group Key: (((g.i)::text || '/0'::text))::pg_lsn
+         ->  Nested Loop
+               ->  Function Scan on generate_series g
+               ->  Function Scan on generate_series
+(7 rows)
+
+EXPLAIN (COSTS OFF) SELECT * FROM (SELECT (g.i||'/0')::pg_lsn lsn FROM generate_series(1, 10) g(i) ORDER BY g.i) a JOIN (SELECT (g.i||'/0')::pg_lsn lsn FROM generate_series(1, 10) g(i) ORDER BY g.i DESC) b ON (a.lsn = b.lsn );
+                          QUERY PLAN                          
+--------------------------------------------------------------
+ Merge Join
+   Merge Cond: (a.lsn = b.lsn)
+   ->  Sort
+         Sort Key: a.lsn
+         ->  Subquery Scan on a
+               ->  Sort
+                     Sort Key: g.i
+                     ->  Function Scan on generate_series g
+   ->  Sort
+         Sort Key: b.lsn
+         ->  Subquery Scan on b
+               ->  Sort
+                     Sort Key: g_1.i
+                     ->  Function Scan on generate_series g_1
+(14 rows)
+
diff --git a/src/test/regress/sql/pg_lsn.sql b/src/test/regress/sql/pg_lsn.sql
index 1634d37..8ff302e 100644
--- a/src/test/regress/sql/pg_lsn.sql
+++ b/src/test/regress/sql/pg_lsn.sql
@@ -2,19 +2,18 @@
 -- PG_LSN
 --
 
-CREATE TABLE PG_LSN_TBL (f1 pg_lsn);
+CREATE TABLE pg_lsn_tbl (f1 pg_lsn);
 
 -- Largest and smallest input
-INSERT INTO PG_LSN_TBL VALUES ('0/0');
-INSERT INTO PG_LSN_TBL VALUES ('FFFFFFFF/FFFFFFFF');
+INSERT INTO pg_lsn_tbl VALUES ('0/0');
+INSERT INTO pg_lsn_tbl VALUES ('FFFFFFFF/FFFFFFFF');
 
 -- Incorrect input
-INSERT INTO PG_LSN_TBL VALUES ('G/0');
-INSERT INTO PG_LSN_TBL VALUES ('-1/0');
-INSERT INTO PG_LSN_TBL VALUES (' 0/12345678');
-INSERT INTO PG_LSN_TBL VALUES ('ABCD/');
-INSERT INTO PG_LSN_TBL VALUES ('/ABCD');
-DROP TABLE PG_LSN_TBL;
+INSERT INTO pg_lsn_tbl VALUES ('G/0');
+INSERT INTO pg_lsn_tbl VALUES ('-1/0');
+INSERT INTO pg_lsn_tbl VALUES (' 0/12345678');
+INSERT INTO pg_lsn_tbl VALUES ('ABCD/');
+INSERT INTO pg_lsn_tbl VALUES ('/ABCD');
 
 -- Operators
 SELECT '0/16AE7F8' = '0/16AE7F8'::pg_lsn;
@@ -23,3 +22,16 @@ SELECT '0/16AE7F7' < '0/16AE7F8'::pg_lsn;
 SELECT '0/16AE7F8' > pg_lsn '0/16AE7F7';
 SELECT '0/16AE7F7'::pg_lsn - '0/16AE7F8'::pg_lsn;
 SELECT '0/16AE7F8'::pg_lsn - '0/16AE7F7'::pg_lsn;
+
+-- Operator Class
+TRUNCATE pg_lsn_tbl;
+INSERT INTO pg_lsn_tbl SELECT DISTINCT (g.i||'/0')::pg_lsn f FROM generate_series(1, 5) g(i), generate_series(1, 5);
+SELECT * FROM pg_lsn_tbl ORDER BY 1;
+CREATE INDEX pg_lsn_tbl_1 ON pg_lsn_tbl USING btree (f1);
+CREATE INDEX pg_lsn_tbl_2 ON pg_lsn_tbl USING hash (f1);
+SELECT indexdef FROM pg_indexes WHERE tablename = 'pg_lsn_tbl' ORDER BY 1;
+DROP TABLE pg_lsn_tbl;
+EXPLAIN (COSTS OFF) SELECT DISTINCT (g.i||'/0')::pg_lsn f FROM generate_series(1, 100) g(i), generate_series(1, 5);
+EXPLAIN (COSTS OFF) SELECT (g.i||'/0')::pg_lsn f FROM generate_series(1, 100) g(i) ORDER BY f;
+EXPLAIN (COSTS OFF) SELECT (g.i||'/0')::pg_lsn, count(*) FROM generate_series(1, 100) g(i), generate_series(1, 5) GROUP BY 1 ORDER BY 1;
+EXPLAIN (COSTS OFF) SELECT * FROM (SELECT (g.i||'/0')::pg_lsn lsn FROM generate_series(1, 10) g(i) ORDER BY g.i) a JOIN (SELECT (g.i||'/0')::pg_lsn lsn FROM generate_series(1, 10) g(i) ORDER BY g.i DESC) b ON (a.lsn = b.lsn );

#28

Michael Paquier

michael.paquier@gmail.com

over 11 years ago

In reply to: Fujii Masao (#21)

1 attachment(s)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Fri, May 9, 2014 at 10:49 PM, Fujii Masao <masao.fujii@gmail.com> wrote:

On Fri, May 9, 2014 at 10:03 PM, Andres Freund <andres@2ndquadrant.com> wrote:

You plan to commit it?

Yes unless many people object the commit.

Michael, you're now modifying the patch?

OK, I have been able to put my head into that earlier than I thought
as it would be better to get that done before the beta, finishing with
the attached. When hacking that, I noticed that some tests were
missing for some of the operators. I am including them in this patch
as well, with more tests for unique index creation, ordering, and
hash/btree index creation. The test suite looks cleaner with all that.
--
Michael

Attachments:

0001-Add-hash-btree-operators-for-pg_lsn.patchtext/plain; charset=US-ASCII; name=0001-Add-hash-btree-operators-for-pg_lsn.patchDownload

From 13513f3392085edb0b3a75af12ef9d08334bdb2e Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@otacoo.com>
Date: Sat, 10 May 2014 14:34:04 +0900
Subject: [PATCH] Add hash/btree operators for pg_lsn

At the same time, regression tests are completed with some tests for
operators that were missing and of course new tests for the new btree
and hash operators.
---
 src/backend/utils/adt/pg_lsn.c       | 21 ++++++++
 src/include/catalog/pg_amop.h        | 12 +++++
 src/include/catalog/pg_amproc.h      |  2 +
 src/include/catalog/pg_opclass.h     |  2 +
 src/include/catalog/pg_operator.h    |  2 +-
 src/include/catalog/pg_opfamily.h    |  2 +
 src/include/catalog/pg_proc.h        |  4 ++
 src/include/utils/pg_lsn.h           |  2 +
 src/test/regress/expected/pg_lsn.out | 96 ++++++++++++++++++++++++++++++------
 src/test/regress/sql/pg_lsn.sql      | 46 +++++++++++++----
 10 files changed, 165 insertions(+), 24 deletions(-)

diff --git a/src/backend/utils/adt/pg_lsn.c b/src/backend/utils/adt/pg_lsn.c
index d1448ae..b779907 100644
--- a/src/backend/utils/adt/pg_lsn.c
+++ b/src/backend/utils/adt/pg_lsn.c
@@ -14,6 +14,7 @@
 #include "postgres.h"
 
 #include "funcapi.h"
+#include "access/hash.h"
 #include "libpq/pqformat.h"
 #include "utils/builtins.h"
 #include "utils/pg_lsn.h"
@@ -153,6 +154,26 @@ pg_lsn_ge(PG_FUNCTION_ARGS)
 	PG_RETURN_BOOL(lsn1 >= lsn2);
 }
 
+/* handler for btree index operator */
+Datum
+pg_lsn_cmp(PG_FUNCTION_ARGS)
+{
+	XLogRecPtr a = PG_GETARG_LSN(0);
+	XLogRecPtr b = PG_GETARG_LSN(1);
+
+	if (a < b)
+		PG_RETURN_INT32(-1);
+	else if (a > b)
+		PG_RETURN_INT32(1);
+	PG_RETURN_INT32(0);
+}
+
+/* hash index support */
+Datum
+pg_lsn_hash(PG_FUNCTION_ARGS)
+{
+	return DirectFunctionCall1(hashint8, PG_GETARG_LSN(0));
+}
 
 /*----------------------------------------------------------
  *	Arithmetic operators on PostgreSQL LSNs.
diff --git a/src/include/catalog/pg_amop.h b/src/include/catalog/pg_amop.h
index 8efd3be..35fd17e 100644
--- a/src/include/catalog/pg_amop.h
+++ b/src/include/catalog/pg_amop.h
@@ -513,6 +513,16 @@ DATA(insert (	2968  2950 2950 4 s 2977	403 0 ));
 DATA(insert (	2968  2950 2950 5 s 2975	403 0 ));
 
 /*
+ * btree pg_lsn_ops
+ */
+
+DATA(insert (	3260  3220 3220 1 s 3224	403 0 ));
+DATA(insert (	3260  3220 3220 2 s 3226	403 0 ));
+DATA(insert (	3260  3220 3220 3 s 3222	403 0 ));
+DATA(insert (	3260  3220 3220 4 s 3227	403 0 ));
+DATA(insert (	3260  3220 3220 5 s 3225	403 0 ));
+
+/*
  *	hash index _ops
  */
 
@@ -581,6 +591,8 @@ DATA(insert (	2231   1042 1042 1 s 1054 405 0 ));
 DATA(insert (	2235   1033 1033 1 s  974 405 0 ));
 /* uuid_ops */
 DATA(insert (	2969   2950 2950 1 s 2972 405 0 ));
+/* pg_lsn_ops */
+DATA(insert (	3261   3220 3220 1 s 3222 405 0 ));
 /* numeric_ops */
 DATA(insert (	1998   1700 1700 1 s 1752 405 0 ));
 /* array_ops */
diff --git a/src/include/catalog/pg_amproc.h b/src/include/catalog/pg_amproc.h
index 198b126..369adb9 100644
--- a/src/include/catalog/pg_amproc.h
+++ b/src/include/catalog/pg_amproc.h
@@ -134,6 +134,7 @@ DATA(insert (	2789   27 27 1 2794 ));
 DATA(insert (	2968   2950 2950 1 2960 ));
 DATA(insert (	2994   2249 2249 1 2987 ));
 DATA(insert (	3194   2249 2249 1 3187 ));
+DATA(insert (	3260   3220 3220 1 3263 ));
 DATA(insert (	3522   3500 3500 1 3514 ));
 DATA(insert (	3626   3614 3614 1 3622 ));
 DATA(insert (	3683   3615 3615 1 3668 ));
@@ -174,6 +175,7 @@ DATA(insert (	2229   25 25 1 400 ));
 DATA(insert (	2231   1042 1042 1 1080 ));
 DATA(insert (	2235   1033 1033 1 329 ));
 DATA(insert (	2969   2950 2950 1 2963 ));
+DATA(insert (	3261   3220 3220 1 3262 ));
 DATA(insert (	3523   3500 3500 1 3515 ));
 DATA(insert (	3903   3831 3831 1 3902 ));
 DATA(insert (	4034   3802 3802 1 4045 ));
diff --git a/src/include/catalog/pg_opclass.h b/src/include/catalog/pg_opclass.h
index ecf7063..2298a83 100644
--- a/src/include/catalog/pg_opclass.h
+++ b/src/include/catalog/pg_opclass.h
@@ -215,6 +215,8 @@ DATA(insert (	2742	_reltime_ops		PGNSP PGUID 2745  1024 t 703 ));
 DATA(insert (	2742	_tinterval_ops		PGNSP PGUID 2745  1025 t 704 ));
 DATA(insert (	403		uuid_ops			PGNSP PGUID 2968  2950 t 0 ));
 DATA(insert (	405		uuid_ops			PGNSP PGUID 2969  2950 t 0 ));
+DATA(insert (	403		pg_lsn_ops			PGNSP PGUID 3260  3220 t 0 ));
+DATA(insert (	405		pg_lsn_ops			PGNSP PGUID 3261  3220 t 0 ));
 DATA(insert (	403		enum_ops			PGNSP PGUID 3522  3500 t 0 ));
 DATA(insert (	405		enum_ops			PGNSP PGUID 3523  3500 t 0 ));
 DATA(insert (	403		tsvector_ops		PGNSP PGUID 3626  3614 t 0 ));
diff --git a/src/include/catalog/pg_operator.h b/src/include/catalog/pg_operator.h
index f280af4..87ee4eb 100644
--- a/src/include/catalog/pg_operator.h
+++ b/src/include/catalog/pg_operator.h
@@ -1595,7 +1595,7 @@ DATA(insert OID = 2977 (  ">="	   PGNSP PGUID b f f 2950 2950 16 2976 2974 uuid_
 DESCR("greater than or equal");
 
 /* pg_lsn operators */
-DATA(insert OID = 3222 (  "="	   PGNSP PGUID b f f 3220 3220 16 3222 3223 pg_lsn_eq eqsel eqjoinsel ));
+DATA(insert OID = 3222 (  "="	   PGNSP PGUID b t t 3220 3220 16 3222 3223 pg_lsn_eq eqsel eqjoinsel ));
 DESCR("equal");
 DATA(insert OID = 3223 (  "<>"	   PGNSP PGUID b f f 3220 3220 16 3223 3222 pg_lsn_ne neqsel neqjoinsel ));
 DESCR("not equal");
diff --git a/src/include/catalog/pg_opfamily.h b/src/include/catalog/pg_opfamily.h
index 9e8f4ac..84f66ac 100644
--- a/src/include/catalog/pg_opfamily.h
+++ b/src/include/catalog/pg_opfamily.h
@@ -134,6 +134,8 @@ DATA(insert OID = 1029 (	783		point_ops		PGNSP PGUID ));
 DATA(insert OID = 2745 (	2742	array_ops		PGNSP PGUID ));
 DATA(insert OID = 2968 (	403		uuid_ops		PGNSP PGUID ));
 DATA(insert OID = 2969 (	405		uuid_ops		PGNSP PGUID ));
+DATA(insert OID = 3260 (	403		pg_lsn_ops		PGNSP PGUID ));
+DATA(insert OID = 3261 (	405		pg_lsn_ops		PGNSP PGUID ));
 DATA(insert OID = 3522 (	403		enum_ops		PGNSP PGUID ));
 DATA(insert OID = 3523 (	405		enum_ops		PGNSP PGUID ));
 DATA(insert OID = 3626 (	403		tsvector_ops	PGNSP PGUID ));
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index e601ccd..2fbc8f1 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -4293,6 +4293,10 @@ DATA(insert OID = 3238 (  pg_lsn_recv	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 3
 DESCR("I/O");
 DATA(insert OID = 3239 (  pg_lsn_send	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 17 "3220" _null_ _null_ _null_ _null_ pg_lsn_send _null_ _null_ _null_ ));
 DESCR("I/O");
+DATA(insert OID = 3262 (  pg_lsn_hash	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 23 "3220" _null_ _null_ _null_ _null_ pg_lsn_hash _null_ _null_ _null_ ));
+DESCR("hash");
+DATA(insert OID = 3263 (  pg_lsn_cmp	PGNSP PGUID 12 1 0 0 0 f f f f t f i 2 0 23 "3220 3220" _null_ _null_ _null_ _null_ pg_lsn_cmp _null_ _null_ _null_ ));
+DESCR("less-equal-greater");
 
 /* enum related procs */
 DATA(insert OID = 3504 (  anyenum_in	PGNSP PGUID 12 1 0 0 0 f f f f t f i 1 0 3500 "2275" _null_ _null_ _null_ _null_ anyenum_in _null_ _null_ _null_ ));
diff --git a/src/include/utils/pg_lsn.h b/src/include/utils/pg_lsn.h
index 981fcd6..7dd932d 100644
--- a/src/include/utils/pg_lsn.h
+++ b/src/include/utils/pg_lsn.h
@@ -29,6 +29,8 @@ extern Datum pg_lsn_lt(PG_FUNCTION_ARGS);
 extern Datum pg_lsn_gt(PG_FUNCTION_ARGS);
 extern Datum pg_lsn_le(PG_FUNCTION_ARGS);
 extern Datum pg_lsn_ge(PG_FUNCTION_ARGS);
+extern Datum pg_lsn_cmp(PG_FUNCTION_ARGS);
+extern Datum pg_lsn_hash(PG_FUNCTION_ARGS);
 
 extern Datum pg_lsn_mi(PG_FUNCTION_ARGS);
 
diff --git a/src/test/regress/expected/pg_lsn.out b/src/test/regress/expected/pg_lsn.out
index 504768c..1a77ba8 100644
--- a/src/test/regress/expected/pg_lsn.out
+++ b/src/test/regress/expected/pg_lsn.out
@@ -1,32 +1,81 @@
 --
 -- PG_LSN
 --
-CREATE TABLE PG_LSN_TBL (f1 pg_lsn);
+CREATE TABLE pg_lsn_tbl (f1 pg_lsn);
+-- Some data
+INSERT INTO pg_lsn_tbl VALUES ('0/16AE7F8');
+INSERT INTO pg_lsn_tbl VALUES ('0/16AE7F7');
+INSERT INTO pg_lsn_tbl VALUES ('16/16AE7F8');
+INSERT INTO pg_lsn_tbl VALUES ('16/16AE7F7');
+INSERT INTO pg_lsn_tbl VALUES ('FF/16AE7F8');
+INSERT INTO pg_lsn_tbl VALUES ('FF/16AE7F7');
 -- Largest and smallest input
-INSERT INTO PG_LSN_TBL VALUES ('0/0');
-INSERT INTO PG_LSN_TBL VALUES ('FFFFFFFF/FFFFFFFF');
+INSERT INTO pg_lsn_tbl VALUES ('0/0');
+INSERT INTO pg_lsn_tbl VALUES ('FFFFFFFF/FFFFFFFF');
 -- Incorrect input
-INSERT INTO PG_LSN_TBL VALUES ('G/0');
+INSERT INTO pg_lsn_tbl VALUES ('G/0');
 ERROR:  invalid input syntax for type pg_lsn: "G/0"
-LINE 1: INSERT INTO PG_LSN_TBL VALUES ('G/0');
+LINE 1: INSERT INTO pg_lsn_tbl VALUES ('G/0');
                                        ^
-INSERT INTO PG_LSN_TBL VALUES ('-1/0');
+INSERT INTO pg_lsn_tbl VALUES ('-1/0');
 ERROR:  invalid input syntax for type pg_lsn: "-1/0"
-LINE 1: INSERT INTO PG_LSN_TBL VALUES ('-1/0');
+LINE 1: INSERT INTO pg_lsn_tbl VALUES ('-1/0');
                                        ^
-INSERT INTO PG_LSN_TBL VALUES (' 0/12345678');
+INSERT INTO pg_lsn_tbl VALUES (' 0/12345678');
 ERROR:  invalid input syntax for type pg_lsn: " 0/12345678"
-LINE 1: INSERT INTO PG_LSN_TBL VALUES (' 0/12345678');
+LINE 1: INSERT INTO pg_lsn_tbl VALUES (' 0/12345678');
                                        ^
-INSERT INTO PG_LSN_TBL VALUES ('ABCD/');
+INSERT INTO pg_lsn_tbl VALUES ('ABCD/');
 ERROR:  invalid input syntax for type pg_lsn: "ABCD/"
-LINE 1: INSERT INTO PG_LSN_TBL VALUES ('ABCD/');
+LINE 1: INSERT INTO pg_lsn_tbl VALUES ('ABCD/');
                                        ^
-INSERT INTO PG_LSN_TBL VALUES ('/ABCD');
+INSERT INTO pg_lsn_tbl VALUES ('/ABCD');
 ERROR:  invalid input syntax for type pg_lsn: "/ABCD"
-LINE 1: INSERT INTO PG_LSN_TBL VALUES ('/ABCD');
+LINE 1: INSERT INTO pg_lsn_tbl VALUES ('/ABCD');
                                        ^
-DROP TABLE PG_LSN_TBL;
+-- ordering test
+SELECT f1 FROM pg_lsn_tbl ORDER BY f1 ASC;
+        f1         
+-------------------
+ 0/0
+ 0/16AE7F7
+ 0/16AE7F8
+ 16/16AE7F7
+ 16/16AE7F8
+ FF/16AE7F7
+ FF/16AE7F8
+ FFFFFFFF/FFFFFFFF
+(8 rows)
+
+SELECT f1 FROM pg_lsn_tbl ORDER BY f1 DESC;
+        f1         
+-------------------
+ FFFFFFFF/FFFFFFFF
+ FF/16AE7F8
+ FF/16AE7F7
+ 16/16AE7F8
+ 16/16AE7F7
+ 0/16AE7F8
+ 0/16AE7F7
+ 0/0
+(8 rows)
+
+-- btree and hash index creation test
+CREATE INDEX pg_lsn_btree ON pg_lsn_tbl USING BTREE (f1);
+CREATE INDEX pg_lsn_hash  ON pg_lsn_tbl USING HASH  (f1);
+-- unique index test
+CREATE UNIQUE INDEX pg_lsn_unique_btree ON pg_lsn_tbl USING BTREE (f1);
+-- this will fail
+INSERT INTO pg_lsn_tbl VALUES ('16/16AE7F7');
+ERROR:  duplicate key value violates unique constraint "pg_lsn_unique_btree"
+DETAIL:  Key (f1)=(16/16AE7F7) already exists.
+-- check to see whether the new indexes are actually there
+SELECT count(*) FROM pg_class WHERE relkind='i' AND relname LIKE 'pg_lsn%';
+ count 
+-------
+     3
+(1 row)
+
 -- Operators
 SELECT '0/16AE7F8' = '0/16AE7F8'::pg_lsn;
  ?column? 
@@ -34,6 +83,12 @@ SELECT '0/16AE7F8' = '0/16AE7F8'::pg_lsn;
  t
 (1 row)
 
+SELECT '0/16AE7F8' <> '0/16AE7F8'::pg_lsn;
+ ?column? 
+----------
+ f
+(1 row)
+
 SELECT '0/16AE7F8'::pg_lsn != '0/16AE7F7';
  ?column? 
 ----------
@@ -46,12 +101,24 @@ SELECT '0/16AE7F7' < '0/16AE7F8'::pg_lsn;
  t
 (1 row)
 
+SELECT '0/16AE7F7' <= '0/16AE7F8'::pg_lsn;
+ ?column? 
+----------
+ t
+(1 row)
+
 SELECT '0/16AE7F8' > pg_lsn '0/16AE7F7';
  ?column? 
 ----------
  t
 (1 row)
 
+SELECT '0/16AE7F8' >= pg_lsn '0/16AE7F7';
+ ?column? 
+----------
+ t
+(1 row)
+
 SELECT '0/16AE7F7'::pg_lsn - '0/16AE7F8'::pg_lsn;
  ?column? 
 ----------
@@ -64,3 +131,4 @@ SELECT '0/16AE7F8'::pg_lsn - '0/16AE7F7'::pg_lsn;
         1
 (1 row)
 
+DROP TABLE pg_lsn_tbl;
diff --git a/src/test/regress/sql/pg_lsn.sql b/src/test/regress/sql/pg_lsn.sql
index 1634d37..c42e03c 100644
--- a/src/test/regress/sql/pg_lsn.sql
+++ b/src/test/regress/sql/pg_lsn.sql
@@ -2,24 +2,52 @@
 -- PG_LSN
 --
 
-CREATE TABLE PG_LSN_TBL (f1 pg_lsn);
+CREATE TABLE pg_lsn_tbl (f1 pg_lsn);
+
+-- Some data
+INSERT INTO pg_lsn_tbl VALUES ('0/16AE7F8');
+INSERT INTO pg_lsn_tbl VALUES ('0/16AE7F7');
+INSERT INTO pg_lsn_tbl VALUES ('16/16AE7F8');
+INSERT INTO pg_lsn_tbl VALUES ('16/16AE7F7');
+INSERT INTO pg_lsn_tbl VALUES ('FF/16AE7F8');
+INSERT INTO pg_lsn_tbl VALUES ('FF/16AE7F7');
 
 -- Largest and smallest input
-INSERT INTO PG_LSN_TBL VALUES ('0/0');
-INSERT INTO PG_LSN_TBL VALUES ('FFFFFFFF/FFFFFFFF');
+INSERT INTO pg_lsn_tbl VALUES ('0/0');
+INSERT INTO pg_lsn_tbl VALUES ('FFFFFFFF/FFFFFFFF');
 
 -- Incorrect input
-INSERT INTO PG_LSN_TBL VALUES ('G/0');
-INSERT INTO PG_LSN_TBL VALUES ('-1/0');
-INSERT INTO PG_LSN_TBL VALUES (' 0/12345678');
-INSERT INTO PG_LSN_TBL VALUES ('ABCD/');
-INSERT INTO PG_LSN_TBL VALUES ('/ABCD');
-DROP TABLE PG_LSN_TBL;
+INSERT INTO pg_lsn_tbl VALUES ('G/0');
+INSERT INTO pg_lsn_tbl VALUES ('-1/0');
+INSERT INTO pg_lsn_tbl VALUES (' 0/12345678');
+INSERT INTO pg_lsn_tbl VALUES ('ABCD/');
+INSERT INTO pg_lsn_tbl VALUES ('/ABCD');
+
+-- ordering test
+SELECT f1 FROM pg_lsn_tbl ORDER BY f1 ASC;
+SELECT f1 FROM pg_lsn_tbl ORDER BY f1 DESC;
+
+-- btree and hash index creation test
+CREATE INDEX pg_lsn_btree ON pg_lsn_tbl USING BTREE (f1);
+CREATE INDEX pg_lsn_hash  ON pg_lsn_tbl USING HASH  (f1);
+
+-- unique index test
+CREATE UNIQUE INDEX pg_lsn_unique_btree ON pg_lsn_tbl USING BTREE (f1);
+-- this will fail
+INSERT INTO pg_lsn_tbl VALUES ('16/16AE7F7');
+
+-- check to see whether the new indexes are actually there
+SELECT count(*) FROM pg_class WHERE relkind='i' AND relname LIKE 'pg_lsn%';
 
 -- Operators
 SELECT '0/16AE7F8' = '0/16AE7F8'::pg_lsn;
+SELECT '0/16AE7F8' <> '0/16AE7F8'::pg_lsn;
 SELECT '0/16AE7F8'::pg_lsn != '0/16AE7F7';
 SELECT '0/16AE7F7' < '0/16AE7F8'::pg_lsn;
+SELECT '0/16AE7F7' <= '0/16AE7F8'::pg_lsn;
 SELECT '0/16AE7F8' > pg_lsn '0/16AE7F7';
+SELECT '0/16AE7F8' >= pg_lsn '0/16AE7F7';
 SELECT '0/16AE7F7'::pg_lsn - '0/16AE7F8'::pg_lsn;
 SELECT '0/16AE7F8'::pg_lsn - '0/16AE7F7'::pg_lsn;
+
+DROP TABLE pg_lsn_tbl;
-- 
1.9.2

#29

Fabrízio de Royes Mello

fabriziomello@gmail.com

over 11 years ago

In reply to: Michael Paquier (#28)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Sat, May 10, 2014 at 2:43 AM, Michael Paquier
<michael.paquier@gmail.com>wrote:

On Fri, May 9, 2014 at 10:49 PM, Fujii Masao <masao.fujii@gmail.com>
wrote:

On Fri, May 9, 2014 at 10:03 PM, Andres Freund <andres@2ndquadrant.com>

wrote:

You plan to commit it?

Yes unless many people object the commit.

Michael, you're now modifying the patch?

OK, I have been able to put my head into that earlier than I thought
as it would be better to get that done before the beta, finishing with
the attached. When hacking that, I noticed that some tests were
missing for some of the operators. I am including them in this patch
as well, with more tests for unique index creation, ordering, and
hash/btree index creation. The test suite looks cleaner with all that.

Last night I sent a patch [1]/messages/by-id/CAFcNs+pMViL+X1kF3pumc2Cp+e1jUoGBDMyZ+SE0PLRR9tmAWw@mail.gmail.com to add more tests and change the operator
name. Maybe we can merge the test cases... ;-)

Regards,

[1]: /messages/by-id/CAFcNs+pMViL+X1kF3pumc2Cp+e1jUoGBDMyZ+SE0PLRR9tmAWw@mail.gmail.com
/messages/by-id/CAFcNs+pMViL+X1kF3pumc2Cp+e1jUoGBDMyZ+SE0PLRR9tmAWw@mail.gmail.com

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL

Show quoted text

Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello

#30

Fujii Masao

masao.fujii@gmail.com

over 11 years ago

In reply to: Fabrízio de Royes Mello (#27)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Sat, May 10, 2014 at 9:51 AM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:

On Fri, May 9, 2014 at 9:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

=?UTF-8?Q?Fabr=C3=ADzio_de_Royes_Mello?= <fabriziomello@gmail.com> writes:

On Fri, May 9, 2014 at 8:42 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I think it's really too late for this for 9.4. At this point it's
less than 48 hours until beta1 wraps, and we do not have the bandwidth
to do anything but worry about stabilizing the features we've already
got.

But it's a very small change with many benefits, and Michael acted very
proactive to make this happens.

[ shrug... ] "proactive" would have been doing this a month ago.
If we're going to ship a release, we have to stop taking new features
at some point, and we are really past that point for 9.4.

And, to be blunt, this is not important enough to hold up the release
for, nor to take any stability risks for. It should go into the next
commitfest cycle where it can get a non-rushed review.

I agree with you that is too late to add *new features*.

But I agree with Andres when he said this is a regression introcuced in the
pg_lsn patch.

So we'll release a version that break a simple query like that:

fabrizio=# SELECT DISTINCT (g.i||'/0')::pg_lsn f FROM generate_series(1,
100) g(i), generate_series(1, 5);
ERROR: could not identify an equality operator for type pg_lsn
LINE 1: SELECT DISTINCT (g.i||'/0')::pg_lsn f FROM generate_series(1...
^

I agree that this is not new feature but just the fix of oversight of
the pg_lsn patch.
Without such opclass, we cannot execute even such simple query.

Regards,

--
Fujii Masao

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#31

Andres Freund

andres@2ndquadrant.com

over 11 years ago

In reply to: Fujii Masao (#30)

Re: New pg_lsn type doesn't have hash/btree opclasses

On 2014-05-11 06:02:23 +0900, Fujii Masao wrote:

On Sat, May 10, 2014 at 9:51 AM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:

On Fri, May 9, 2014 at 9:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

[ shrug... ] "proactive" would have been doing this a month ago.
If we're going to ship a release, we have to stop taking new features
at some point, and we are really past that point for 9.4.

We *couldn't* do it a month ago since we didn't know about it a month
ago. My delorean is getting a bit old.

I agree with you that is too late to add *new features*.

But I agree with Andres when he said this is a regression introcuced in the
pg_lsn patch.

So we'll release a version that break a simple query like that:

fabrizio=# SELECT DISTINCT (g.i||'/0')::pg_lsn f FROM generate_series(1,
100) g(i), generate_series(1, 5);
ERROR: could not identify an equality operator for type pg_lsn
LINE 1: SELECT DISTINCT (g.i||'/0')::pg_lsn f FROM generate_series(1...
^

I agree that this is not new feature but just the fix of oversight of
the pg_lsn patch.
Without such opclass, we cannot execute even such simple query.

I don't even understand why it's questionable whether this should be
fixed. It's an almost trivial patch for an oversight in a newly
introduced feature. We gain absolutely nothing by patching this in the
next cycle, except that things need to be tested twice.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#32

Tom Lane

tgl@sss.pgh.pa.us

over 11 years ago

In reply to: Andres Freund (#31)

Re: New pg_lsn type doesn't have hash/btree opclasses

Andres Freund <andres@2ndquadrant.com> writes:

I don't even understand why it's questionable whether this should be
fixed.

Sigh. We have some debate isomorphic to this one every year, it seems
like. The argument why it shouldn't be fixed now is: ITS. TOO. LATE.
Which part of that isn't clear to you?

Or, if you think that this feature is so important that we should slip
the beta schedule to get it in, we can take a vote on that. But at
this point any slip means no beta till after PGCon.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#33

Fabrízio de Royes Mello

fabriziomello@gmail.com

over 11 years ago

In reply to: Tom Lane (#32)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Sat, May 10, 2014 at 6:52 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andres Freund <andres@2ndquadrant.com> writes:

I don't even understand why it's questionable whether this should be
fixed.

Sigh. We have some debate isomorphic to this one every year, it seems
like. The argument why it shouldn't be fixed now is: ITS. TOO. LATE.
Which part of that isn't clear to you?

Sorry but I don't understand why it's too late. The 9.4 branch not been
created yet.

And in the last hours various patches (mostly fixes) have been committed
and IMO this is not different.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL

Show quoted text

Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello

#34

Andres Freund

andres@2ndquadrant.com

over 11 years ago

In reply to: Fabrízio de Royes Mello (#33)

Re: New pg_lsn type doesn't have hash/btree opclasses

On 2014-05-10 19:19:22 -0300, Fabrízio de Royes Mello wrote:

On Sat, May 10, 2014 at 6:52 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andres Freund <andres@2ndquadrant.com> writes:

I don't even understand why it's questionable whether this should be
fixed.

Sigh. We have some debate isomorphic to this one every year, it seems
like. The argument why it shouldn't be fixed now is: ITS. TOO. LATE.
Which part of that isn't clear to you?

Sorry but I don't understand why it's too late. The 9.4 branch not been
created yet.

The problem is that once the beta is in progress (starting tomorrow),
the projects tries to avoid changes which require a dump and restore (or
pg_upgrade). Since the patch changes the catalog it'd require that.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#35

Magnus Hagander

magnus@hagander.net

over 11 years ago

In reply to: Andres Freund (#34)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Sun, May 11, 2014 at 12:27 AM, Andres Freund <andres@2ndquadrant.com>wrote:

On 2014-05-10 19:19:22 -0300, Fabrízio de Royes Mello wrote:

On Sat, May 10, 2014 at 6:52 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andres Freund <andres@2ndquadrant.com> writes:

I don't even understand why it's questionable whether this should be
fixed.

Sigh. We have some debate isomorphic to this one every year, it seems
like. The argument why it shouldn't be fixed now is: ITS. TOO. LATE.
Which part of that isn't clear to you?

Sorry but I don't understand why it's too late. The 9.4 branch not been
created yet.

The problem is that once the beta is in progress (starting tomorrow),
the projects tries to avoid changes which require a dump and restore (or
pg_upgrade). Since the patch changes the catalog it'd require that.

It would be pg_upgrade'able though, wouldn't it? Don't we have precedents
for requiring pg_upgrade during beta? At least that's a smaller problem
than requiring a complete dump/reload.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

#36

Tom Lane

tgl@sss.pgh.pa.us

over 11 years ago

In reply to: Andres Freund (#34)

Re: New pg_lsn type doesn't have hash/btree opclasses

Andres Freund <andres@2ndquadrant.com> writes:

On 2014-05-10 19:19:22 -0300, Fabr�zio de Royes Mello wrote:

Sorry but I don't understand why it's too late. The 9.4 branch not been
created yet.

The problem is that once the beta is in progress (starting tomorrow),
the projects tries to avoid changes which require a dump and restore (or
pg_upgrade). Since the patch changes the catalog it'd require that.

Exactly. If this isn't in before beta1, it's not going in, unless perhaps
we encounter some bug bad enough to force an initdb later in beta.
Which is certainly possible, and if it did happen there might be room
to reconsider. But the same policy means there is zero margin for error
with a catalog-changing patch applied right before beta starts, which
is what we're debating here.

As a comparison point, in a nearby thread we're debating renaming
jsonb_hash_ops, which is a sufficiently mechanical change that I'd
not be too afraid to do it the day before beta. But there are enough
ways to screw up a new operator class that I'm very afraid of putting
one in this weekend.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#37

Fabrízio de Royes Mello

fabriziomello@gmail.com

over 11 years ago

In reply to: Andres Freund (#34)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Sat, May 10, 2014 at 7:27 PM, Andres Freund <andres@2ndquadrant.com>
wrote:

On 2014-05-10 19:19:22 -0300, Fabrízio de Royes Mello wrote:

On Sat, May 10, 2014 at 6:52 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andres Freund <andres@2ndquadrant.com> writes:

I don't even understand why it's questionable whether this should be
fixed.

Sigh. We have some debate isomorphic to this one every year, it seems
like. The argument why it shouldn't be fixed now is: ITS. TOO. LATE.
Which part of that isn't clear to you?

Sorry but I don't understand why it's too late. The 9.4 branch not been
created yet.

The problem is that once the beta is in progress (starting tomorrow),
the projects tries to avoid changes which require a dump and restore (or
pg_upgrade). Since the patch changes the catalog it'd require that.

Hmmmm... so the other option maybe is create an extension to add this
operators.

Thoughts?

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL

Show quoted text

Timbira: http://www.timbira.com.br
Blog sobre TI: http://fabriziomello.blogspot.com
Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello

#38

Andres Freund

andres@2ndquadrant.com

over 11 years ago

In reply to: Magnus Hagander (#35)

Re: New pg_lsn type doesn't have hash/btree opclasses

On 2014-05-11 00:31:09 +0200, Magnus Hagander wrote:

On Sun, May 11, 2014 at 12:27 AM, Andres Freund <andres@2ndquadrant.com>wrote:

On 2014-05-10 19:19:22 -0300, Fabrízio de Royes Mello wrote:

On Sat, May 10, 2014 at 6:52 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andres Freund <andres@2ndquadrant.com> writes:

I don't even understand why it's questionable whether this should be
fixed.

Sigh. We have some debate isomorphic to this one every year, it seems
like. The argument why it shouldn't be fixed now is: ITS. TOO. LATE.
Which part of that isn't clear to you?

Sorry but I don't understand why it's too late. The 9.4 branch not been
created yet.

The problem is that once the beta is in progress (starting tomorrow),
the projects tries to avoid changes which require a dump and restore (or
pg_upgrade). Since the patch changes the catalog it'd require that.

It would be pg_upgrade'able though, wouldn't it?

Yes.

Don't we have precedents
for requiring pg_upgrade during beta? At least that's a smaller problem
than requiring a complete dump/reload.

I think in reality there's been catversion updates after most of the
recent beta1 releases.

9.3 (one):
git log 817a89423f429a6a8b36847ee499f5b6be39c3be.. -- src/include/catalog/catversion.h
9.2 (one, but reverted):
git log f70fa835e08eee4cb2dc0f72d66cf633089c37e8..REL9_2_0 -- src/include/catalog/catversion.h
9.1 (two):
git log 993c5e59047dd568d4831f7ec5c6199acd21f17f..REL9_1_0 -- src/include/catalog/catversion.h
9.0 (one)
git log -p f9d9b2b34a094b94fda39231c16ab5f2e6bfbbe4..REL9_0_0 -- src/include/catalog/catversion.h

(makes me really wish betas were properly tagged with git as well...)

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#39

Michael Paquier

michael.paquier@gmail.com

over 11 years ago

In reply to: Andres Freund (#38)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Sun, May 11, 2014 at 7:39 AM, Andres Freund <andres@2ndquadrant.com> wrote:

(makes me really wish betas were properly tagged with git as well...)

They are tags for betas, here is for example the update of CATVERSION for 9.3:
$ git log -p REL9_3_BETA1..REL9_3_0 src/include/catalog/catversion.h |
grep commit
commit dc3eb5638349e74a6628130a5101ce866455f4a3
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#40

Tom Lane

tgl@sss.pgh.pa.us

over 11 years ago

In reply to: Magnus Hagander (#35)

Re: New pg_lsn type doesn't have hash/btree opclasses

Magnus Hagander <magnus@hagander.net> writes:

On Sun, May 11, 2014 at 12:27 AM, Andres Freund <andres@2ndquadrant.com>wrote:

The problem is that once the beta is in progress (starting tomorrow),
the projects tries to avoid changes which require a dump and restore (or
pg_upgrade). Since the patch changes the catalog it'd require that.

It would be pg_upgrade'able though, wouldn't it? Don't we have precedents
for requiring pg_upgrade during beta? At least that's a smaller problem
than requiring a complete dump/reload.

pg_upgrade makes the penalty for screwups smaller, but a post-beta1 initdb
is still the result of a screwup. None of the historical examples you
mention were planned in advance of beta.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#41

Andres Freund

andres@2ndquadrant.com

over 11 years ago

In reply to: Tom Lane (#40)

Re: New pg_lsn type doesn't have hash/btree opclasses

On 2014-05-10 19:08:48 -0400, Tom Lane wrote:

Magnus Hagander <magnus@hagander.net> writes:

On Sun, May 11, 2014 at 12:27 AM, Andres Freund <andres@2ndquadrant.com>wrote:

The problem is that once the beta is in progress (starting tomorrow),
the projects tries to avoid changes which require a dump and restore (or
pg_upgrade). Since the patch changes the catalog it'd require that.

It would be pg_upgrade'able though, wouldn't it? Don't we have precedents
for requiring pg_upgrade during beta? At least that's a smaller problem
than requiring a complete dump/reload.

pg_upgrade makes the penalty for screwups smaller, but a post-beta1 initdb
is still the result of a screwup. None of the historical examples you
mention were planned in advance of beta.

Yea, I posted that just to answer Magnus' question.

I've argued that this omission should be fixed since tuesday. There's
been a tested and reviewed patch since
20140506230722.GE24808@awork2.anarazel.de. Given how many changes went
in since it certainly wouldn't have been a very destabilizing commit.

Anyway. I accept it's too late for beta1 now. Let's commit it if there's
another catversion bump.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#42

Michael Paquier

michael.paquier@gmail.com

over 11 years ago

In reply to: Fabrízio de Royes Mello (#29)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Sat, May 10, 2014 at 9:45 PM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:

Last night I sent a patch [1] to add more tests and change the operator
name. Maybe we can merge the test cases... ;-)

Sure, I noticed that. But I think that they are more complicated than
necessary. I am as well doubtful about adding test cases with EXPLAIN
for a data type test suite.

The patch introduces two new things: pg_lsn_cmp and pg_lsn_hash. To
test the former a simple ORDER BY query is enough as cmp is used as an
ordering operator. And for the latter creating a hash index looks
enough as it tests using the hash function when inserting index
tuples. Not to mention as well that the tests are more readable.
Regards,
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#43

Michael Paquier

michael.paquier@gmail.com

over 11 years ago

In reply to: Andres Freund (#41)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Sun, May 11, 2014 at 8:17 AM, Andres Freund <andres@2ndquadrant.com> wrote:

Anyway. I accept it's too late for beta1 now. Let's commit it if there's
another catversion bump.

+1. Let's rely on the experience of senior committers here.
-- 
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#44

Tom Lane

tgl@sss.pgh.pa.us

over 11 years ago

In reply to: Andres Freund (#20)

Re: New pg_lsn type doesn't have hash/btree opclasses

Andres Freund <andres@2ndquadrant.com> writes:

On 2014-05-09 22:14:25 +0900, Michael Paquier wrote:

[ patch ]

I've committed a revised version of Andres' patch. Mostly cosmetic
fixes, but the hash implementation was still wrong:

return DirectFunctionCall1(hashint8, PG_GETARG_LSN(0));

DirectFunctionCallN takes Datums, not de-Datumified values. This coding
would've crashed on 32-bit machines, where hashint8 would be expecting
to receive a Datum containing a pointer to int64.

We could have done it correctly like this:

return DirectFunctionCall1(hashint8, PG_GETARG_DATUM(0));

but I thought it was better, and microscopically more efficient, to
follow the existing precedent in timestamp_hash:

return hashint8(fcinfo);

since that avoids an extra FunctionCallInfoData setup.

On Fri, May 9, 2014 at 10:01 PM, Fujii Masao <masao.fujii@gmail.com> wrote:

The patch looks good to me except the name of index operator class.
I think that "pg_lsn_ops" is better than "pglsn_ops" because it's for "pg_lsn"
data type.

I concur, and changed this.

I honestly don't really see much point in the added tests. If at all I'd
rather see my tests from
http://archives.postgresql.org/message-id/20140506230722.GE24808%40awork2.anarazel.de
addded. With some EXPLAIN (COSTS OFF) they'd test more.

I thought even that was kind of overkill; but a bigger problem is the
output was sensitive to hash values which are not portable across
different architectures. With a bit of experimentation I found that
a SELECT DISTINCT ... ORDER BY query would exercise both hashing and
sorting, so that's what got committed. (I'm not entirely sure though
whether the plan will be stable across architectures; we might have
to tweak the test case based on buildfarm feedback.)

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#45

Michael Paquier

michael.paquier@gmail.com

over 11 years ago

In reply to: Tom Lane (#44)

Re: New pg_lsn type doesn't have hash/btree opclasses

On Thu, Jun 5, 2014 at 9:54 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andres Freund <andres@2ndquadrant.com> writes:

On 2014-05-09 22:14:25 +0900, Michael Paquier wrote:

[ patch ]

I've committed a revised version of Andres' patch.

Thanks!

I thought even that was kind of overkill; but a bigger problem is the
output was sensitive to hash values which are not portable across
different architectures. With a bit of experimentation I found that
a SELECT DISTINCT ... ORDER BY query would exercise both hashing and
sorting, so that's what got committed. (I'm not entirely sure though
whether the plan will be stable across architectures; we might have
to tweak the test case based on buildfarm feedback.)

Yeah this was a bit too much, and came up with some more light-weight
regression tests instead in the patch here:
/messages/by-id/CAB7nPqSgZVrHRgsgUg63SCFY+AwH-=3JuDy7moq-_fo7Wi4++Q@mail.gmail.com
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers