On 10/22/2014 01:55 PM, Teodor Sigaev wrote:
Suggested patch adds GIN support contains operator for ranges over scalar 
column.

It allows more effective GIN scan. Currently, queries like
SELECT * FROM test_int4 WHERE i <= 1 and i >= 1
will be excuted by GIN with two scans: one is from mines infinity to 1 and
another is from -1 to plus infinity. That's because GIN is "generalized" and it
doesn't know a semantics of operation.

With patch it's possible to rewrite query with ranges
SELECT * FROM test_int4 WHERE i <@ '[-1, 1]'::int4range
and GIN index will support this query with single scan from -1 to 1.

Patch provides index support only for existing range types: int4, int8, numeric,
date and timestamp with and without time zone.

I started to look at this, but very quickly got carried away into refactoring away the macros. Defining long functions as macros makes debugging quite difficult, and it's not nice to read or edit the source code either.

I propose the attached refactoring (it doesn't include your range-patch yet, just refactoring the existing code). It turns the bulk of the macros into static functions. GIN_SUPPORT macro still defines the datatype-specific functions, but they are now very thin wrappers that just call the corresponding generic static functions.

It's annoying that we need the concept of a leftmost value, for the < and <= queries. Without that, we could have the support functions look up the required datatype information from the type cache, based on the datatype of the passed argument. Then you could easily use the btree_gin support functions also for user-defined datatypes. But that needs bigger changes, and this is a step in the right direction anyway.

- Heikki

>From fc96b3842e0695f8b4e39fc870ad0c3d628db834 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakan...@iki.fi>
Date: Wed, 17 Dec 2014 19:57:39 +0200
Subject: [PATCH] Turn much of the btree_gin macros into real functions.

This makes the functions much nicer to read and edit, and also makes
debugging easier.

diff --git a/contrib/btree_gin/btree_gin.c b/contrib/btree_gin/btree_gin.c
index 87d23e0..80521fb 100644
--- a/contrib/btree_gin/btree_gin.c
+++ b/contrib/btree_gin/btree_gin.c
@@ -17,34 +17,30 @@
 
 PG_MODULE_MAGIC;
 
-typedef struct TypeInfo
-{
-	bool		is_varlena;
-	Datum		(*leftmostvalue) (void);
-	Datum		(*typecmp) (FunctionCallInfo);
-} TypeInfo;
-
 typedef struct QueryInfo
 {
 	StrategyNumber strategy;
 	Datum		datum;
+	bool		is_varlena;
+	Datum		(*typecmp) (FunctionCallInfo);
 } QueryInfo;
 
-#define  GIN_EXTRACT_VALUE(type)											\
-PG_FUNCTION_INFO_V1(gin_extract_value_##type);								\
-Datum																		\
-gin_extract_value_##type(PG_FUNCTION_ARGS)									\
-{																			\
-	Datum		datum = PG_GETARG_DATUM(0);									\
-	int32	   *nentries = (int32 *) PG_GETARG_POINTER(1);					\
-	Datum	   *entries = (Datum *) palloc(sizeof(Datum));					\
-																			\
-	if ( TypeInfo_##type.is_varlena )										\
-		datum = PointerGetDatum(PG_DETOAST_DATUM(datum));					\
-	entries[0] = datum;														\
-	*nentries = 1;															\
-																			\
-	PG_RETURN_POINTER(entries);												\
+
+/*** GIN support functions shared by all datatypes ***/
+
+static Datum
+gin_btree_extract_value(FunctionCallInfo fcinfo, bool is_varlena)
+{
+	Datum		datum = PG_GETARG_DATUM(0);
+	int32	   *nentries = (int32 *) PG_GETARG_POINTER(1);
+	Datum	   *entries = (Datum *) palloc(sizeof(Datum));
+
+	if (is_varlena)
+		datum = PointerGetDatum(PG_DETOAST_DATUM(datum));
+	entries[0] = datum;
+	*nentries = 1;
+
+	PG_RETURN_POINTER(entries);
 }
 
 /*
@@ -55,49 +51,51 @@ gin_extract_value_##type(PG_FUNCTION_ARGS)									\
  * key, and work forward until the supplied query datum (which must be
  * sent along inside the QueryInfo structure).
  */
+static Datum
+gin_btree_extract_query(FunctionCallInfo fcinfo,
+						bool is_varlena,
+						Datum (*leftmostvalue) (void),
+						Datum (*typecmp) (FunctionCallInfo))
+{
+	Datum		datum = PG_GETARG_DATUM(0);
+	int32	   *nentries = (int32 *) PG_GETARG_POINTER(1);
+	StrategyNumber strategy = PG_GETARG_UINT16(2);
+	bool	  **partialmatch = (bool **) PG_GETARG_POINTER(3);
+	Pointer   **extra_data = (Pointer **) PG_GETARG_POINTER(4);
+	Datum	   *entries = (Datum *) palloc(sizeof(Datum));
+	QueryInfo  *data = (QueryInfo *) palloc(sizeof(QueryInfo));
+	bool	   *ptr_partialmatch;
+
+	*nentries = 1;
+	ptr_partialmatch = *partialmatch = (bool *) palloc(sizeof(bool));
+	*ptr_partialmatch = false;
+	if (is_varlena)
+		datum = PointerGetDatum(PG_DETOAST_DATUM(datum));
+	data->strategy = strategy;
+	data->datum = datum;
+	data->is_varlena = is_varlena;
+	data->typecmp = typecmp;
+	*extra_data = (Pointer *) palloc(sizeof(Pointer));
+	**extra_data = (Pointer) data;
+
+	switch (strategy)
+	{
+		case BTLessStrategyNumber:
+		case BTLessEqualStrategyNumber:
+			entries[0] = leftmostvalue();
+			*ptr_partialmatch = true;
+			break;
+		case BTGreaterEqualStrategyNumber:
+		case BTGreaterStrategyNumber:
+			*ptr_partialmatch = true;
+		case BTEqualStrategyNumber:
+			entries[0] = datum;
+			break;
+		default:
+			elog(ERROR, "unrecognized strategy number: %d", strategy);
+	}
 
-#define GIN_EXTRACT_QUERY(type)												\
-PG_FUNCTION_INFO_V1(gin_extract_query_##type);								\
-Datum																		\
-gin_extract_query_##type(PG_FUNCTION_ARGS)									\
-{																			\
-	Datum		datum = PG_GETARG_DATUM(0);									\
-	int32	   *nentries = (int32 *) PG_GETARG_POINTER(1);					\
-	StrategyNumber strategy = PG_GETARG_UINT16(2);							\
-	bool	  **partialmatch = (bool **) PG_GETARG_POINTER(3);				\
-	Pointer   **extra_data = (Pointer **) PG_GETARG_POINTER(4);				\
-	Datum	   *entries = (Datum *) palloc(sizeof(Datum));					\
-	QueryInfo  *data = (QueryInfo *) palloc(sizeof(QueryInfo));				\
-	bool	   *ptr_partialmatch;											\
-																			\
-	*nentries = 1;															\
-	ptr_partialmatch = *partialmatch = (bool *) palloc(sizeof(bool));		\
-	*ptr_partialmatch = false;												\
-	if ( TypeInfo_##type.is_varlena )										\
-		datum = PointerGetDatum(PG_DETOAST_DATUM(datum));					\
-	data->strategy = strategy;												\
-	data->datum = datum;													\
-	*extra_data = (Pointer *) palloc(sizeof(Pointer));						\
-	**extra_data = (Pointer) data;											\
-																			\
-	switch (strategy)														\
-	{																		\
-		case BTLessStrategyNumber:											\
-		case BTLessEqualStrategyNumber:										\
-			entries[0] = TypeInfo_##type.leftmostvalue();					\
-			*ptr_partialmatch = true;										\
-			break;															\
-		case BTGreaterEqualStrategyNumber:									\
-		case BTGreaterStrategyNumber:										\
-			*ptr_partialmatch = true;										\
-		case BTEqualStrategyNumber:											\
-			entries[0] = datum;												\
-			break;															\
-		default:															\
-			elog(ERROR, "unrecognized strategy number: %d", strategy);		\
-	}																		\
-																			\
-	PG_RETURN_POINTER(entries);												\
+	PG_RETURN_POINTER(entries);
 }
 
 /*
@@ -105,78 +103,70 @@ gin_extract_query_##type(PG_FUNCTION_ARGS)									\
  * strategy it is a left-most value.  So, use original datum from QueryInfo
  * to decide to stop scanning or not.  Datum b is always from index.
  */
-#define GIN_COMPARE_PREFIX(type)											\
-PG_FUNCTION_INFO_V1(gin_compare_prefix_##type);								\
-Datum																		\
-gin_compare_prefix_##type(PG_FUNCTION_ARGS)									\
-{																			\
-	Datum		a = PG_GETARG_DATUM(0);										\
-	Datum		b = PG_GETARG_DATUM(1);										\
-	QueryInfo  *data = (QueryInfo *) PG_GETARG_POINTER(3);					\
-	int32		res,														\
-				cmp;														\
-																			\
-	cmp = DatumGetInt32(DirectFunctionCall2Coll(							\
-				TypeInfo_##type.typecmp,									\
-				PG_GET_COLLATION(),											\
-				(data->strategy == BTLessStrategyNumber ||					\
-				 data->strategy == BTLessEqualStrategyNumber)				\
-				 ? data->datum : a,											\
-				b));														\
-																			\
-	switch (data->strategy)													\
-	{																		\
-		case BTLessStrategyNumber:											\
-			/* If original datum > indexed one then return match */			\
-			if (cmp > 0)													\
-				res = 0;													\
-			else															\
-				res = 1;													\
-			break;															\
-		case BTLessEqualStrategyNumber:										\
-			/* The same except equality */									\
-			if (cmp >= 0)													\
-				res = 0;													\
-			else															\
-				res = 1;													\
-			break;															\
-		case BTEqualStrategyNumber:											\
-			if (cmp != 0)													\
-				res = 1;													\
-			else															\
-				res = 0;													\
-			break;															\
-		case BTGreaterEqualStrategyNumber:									\
-			/* If original datum <= indexed one then return match */		\
-			if (cmp <= 0)													\
-				res = 0;													\
-			else															\
-				res = 1;													\
-			break;															\
-		case BTGreaterStrategyNumber:										\
-			/* If original datum <= indexed one then return match */		\
-			/* If original datum == indexed one then continue scan */		\
-			if (cmp < 0)													\
-				res = 0;													\
-			else if (cmp == 0)												\
-				res = -1;													\
-			else															\
-				res = 1;													\
-			break;															\
-		default:															\
-			elog(ERROR, "unrecognized strategy number: %d",					\
-				 data->strategy);											\
-			res = 0;														\
-	}																		\
-																			\
-	PG_RETURN_INT32(res);													\
-}
-
-#define GIN_SUPPORT(type)			\
-	GIN_EXTRACT_VALUE(type)			\
-	GIN_EXTRACT_QUERY(type)			\
-	GIN_COMPARE_PREFIX(type)
+static Datum
+gin_btree_compare_prefix(FunctionCallInfo fcinfo)
+{
+	Datum		a = PG_GETARG_DATUM(0);
+	Datum		b = PG_GETARG_DATUM(1);
+	QueryInfo  *data = (QueryInfo *) PG_GETARG_POINTER(3);
+	int32		res,
+				cmp;
+
+	cmp = DatumGetInt32(DirectFunctionCall2Coll(
+				data->typecmp,
+				PG_GET_COLLATION(),
+				(data->strategy == BTLessStrategyNumber ||
+				 data->strategy == BTLessEqualStrategyNumber)
+				 ? data->datum : a,
+				b));
+
+	switch (data->strategy)
+	{
+		case BTLessStrategyNumber:
+			/* If original datum > indexed one then return match */
+			if (cmp > 0)
+				res = 0;
+			else
+				res = 1;
+			break;
+		case BTLessEqualStrategyNumber:
+			/* The same except equality */
+			if (cmp >= 0)
+				res = 0;
+			else
+				res = 1;
+			break;
+		case BTEqualStrategyNumber:
+			if (cmp != 0)
+				res = 1;
+			else
+				res = 0;
+			break;
+		case BTGreaterEqualStrategyNumber:
+			/* If original datum <= indexed one then return match */
+			if (cmp <= 0)
+				res = 0;
+			else
+				res = 1;
+			break;
+		case BTGreaterStrategyNumber:
+			/* If original datum <= indexed one then return match */
+			/* If original datum == indexed one then continue scan */
+			if (cmp < 0)
+				res = 0;
+			else if (cmp == 0)
+				res = -1;
+			else
+				res = 1;
+			break;
+		default:
+			elog(ERROR, "unrecognized strategy number: %d",
+				 data->strategy);
+			res = 0;
+	}
 
+	PG_RETURN_INT32(res);
+}
 
 PG_FUNCTION_INFO_V1(gin_btree_consistent);
 Datum
@@ -188,23 +178,45 @@ gin_btree_consistent(PG_FUNCTION_ARGS)
 	PG_RETURN_BOOL(true);
 }
 
+/*** GIN_SUPPORT macro defines the datatype specific functions ***/
+
+#define GIN_SUPPORT(type, is_varlena, leftmostvalue, typecmp)				\
+PG_FUNCTION_INFO_V1(gin_extract_value_##type);								\
+Datum																		\
+gin_extract_value_##type(PG_FUNCTION_ARGS)									\
+{																			\
+	return gin_btree_extract_value(fcinfo, is_varlena);						\
+}																			\
+PG_FUNCTION_INFO_V1(gin_extract_query_##type);								\
+Datum																		\
+gin_extract_query_##type(PG_FUNCTION_ARGS)									\
+{																			\
+	return gin_btree_extract_query(fcinfo,									\
+								   is_varlena, leftmostvalue, typecmp);		\
+}																			\
+PG_FUNCTION_INFO_V1(gin_compare_prefix_##type);								\
+Datum																		\
+gin_compare_prefix_##type(PG_FUNCTION_ARGS)									\
+{																			\
+	return gin_btree_compare_prefix(fcinfo);								\
+}
+
+
+/*** Datatype specifications ***/
+
 static Datum
 leftmostvalue_int2(void)
 {
 	return Int16GetDatum(SHRT_MIN);
 }
-static TypeInfo TypeInfo_int2 = {false, leftmostvalue_int2, btint2cmp};
-
-GIN_SUPPORT(int2)
+GIN_SUPPORT(int2, false, leftmostvalue_int2, btint2cmp)
 
 static Datum
 leftmostvalue_int4(void)
 {
 	return Int32GetDatum(INT_MIN);
 }
-static TypeInfo TypeInfo_int4 = {false, leftmostvalue_int4, btint4cmp};
-
-GIN_SUPPORT(int4)
+GIN_SUPPORT(int4, false, leftmostvalue_int4, btint4cmp)
 
 static Datum
 leftmostvalue_int8(void)
@@ -214,27 +226,21 @@ leftmostvalue_int8(void)
 	 */
 	return Int64GetDatum(SEQ_MINVALUE);
 }
-static TypeInfo TypeInfo_int8 = {false, leftmostvalue_int8, btint8cmp};
-
-GIN_SUPPORT(int8)
+GIN_SUPPORT(int8, false, leftmostvalue_int8, btint8cmp)
 
 static Datum
 leftmostvalue_float4(void)
 {
 	return Float4GetDatum(-get_float4_infinity());
 }
-static TypeInfo TypeInfo_float4 = {false, leftmostvalue_float4, btfloat4cmp};
-
-GIN_SUPPORT(float4)
+GIN_SUPPORT(float4, false, leftmostvalue_float4, btfloat4cmp)
 
 static Datum
 leftmostvalue_float8(void)
 {
 	return Float8GetDatum(-get_float8_infinity());
 }
-static TypeInfo TypeInfo_float8 = {false, leftmostvalue_float8, btfloat8cmp};
-
-GIN_SUPPORT(float8)
+GIN_SUPPORT(float8, false, leftmostvalue_float8, btfloat8cmp)
 
 static Datum
 leftmostvalue_money(void)
@@ -244,40 +250,30 @@ leftmostvalue_money(void)
 	 */
 	return Int64GetDatum(SEQ_MINVALUE);
 }
-static TypeInfo TypeInfo_money = {false, leftmostvalue_money, cash_cmp};
-
-GIN_SUPPORT(money)
+GIN_SUPPORT(money, false, leftmostvalue_money, cash_cmp)
 
 static Datum
 leftmostvalue_oid(void)
 {
 	return ObjectIdGetDatum(0);
 }
-static TypeInfo TypeInfo_oid = {false, leftmostvalue_oid, btoidcmp};
-
-GIN_SUPPORT(oid)
+GIN_SUPPORT(oid, false, leftmostvalue_oid, btoidcmp)
 
 static Datum
 leftmostvalue_timestamp(void)
 {
 	return TimestampGetDatum(DT_NOBEGIN);
 }
-static TypeInfo TypeInfo_timestamp = {false, leftmostvalue_timestamp, timestamp_cmp};
-
-GIN_SUPPORT(timestamp)
-
-static TypeInfo TypeInfo_timestamptz = {false, leftmostvalue_timestamp, timestamp_cmp};
+GIN_SUPPORT(timestamp, false, leftmostvalue_timestamp, timestamp_cmp)
 
-GIN_SUPPORT(timestamptz)
+GIN_SUPPORT(timestamptz, false, leftmostvalue_timestamp, timestamp_cmp)
 
 static Datum
 leftmostvalue_time(void)
 {
 	return TimeADTGetDatum(0);
 }
-static TypeInfo TypeInfo_time = {false, leftmostvalue_time, time_cmp};
-
-GIN_SUPPORT(time)
+GIN_SUPPORT(time, false, leftmostvalue_time, time_cmp)
 
 static Datum
 leftmostvalue_timetz(void)
@@ -289,18 +285,14 @@ leftmostvalue_timetz(void)
 
 	return TimeTzADTPGetDatum(v);
 }
-static TypeInfo TypeInfo_timetz = {false, leftmostvalue_timetz, timetz_cmp};
-
-GIN_SUPPORT(timetz)
+GIN_SUPPORT(timetz, false, leftmostvalue_timetz, timetz_cmp)
 
 static Datum
 leftmostvalue_date(void)
 {
 	return DateADTGetDatum(DATEVAL_NOBEGIN);
 }
-static TypeInfo TypeInfo_date = {false, leftmostvalue_date, date_cmp};
-
-GIN_SUPPORT(date)
+GIN_SUPPORT(date, false, leftmostvalue_date, date_cmp)
 
 static Datum
 leftmostvalue_interval(void)
@@ -312,9 +304,7 @@ leftmostvalue_interval(void)
 	v->month = 0;
 	return IntervalPGetDatum(v);
 }
-static TypeInfo TypeInfo_interval = {false, leftmostvalue_interval, interval_cmp};
-
-GIN_SUPPORT(interval)
+GIN_SUPPORT(interval, false, leftmostvalue_interval, interval_cmp)
 
 static Datum
 leftmostvalue_macaddr(void)
@@ -323,9 +313,7 @@ leftmostvalue_macaddr(void)
 
 	return MacaddrPGetDatum(v);
 }
-static TypeInfo TypeInfo_macaddr = {false, leftmostvalue_macaddr, macaddr_cmp};
-
-GIN_SUPPORT(macaddr)
+GIN_SUPPORT(macaddr, false, leftmostvalue_macaddr, macaddr_cmp)
 
 static Datum
 leftmostvalue_inet(void)
@@ -335,35 +323,25 @@ leftmostvalue_inet(void)
 							   ObjectIdGetDatum(0),
 							   Int32GetDatum(-1));
 }
-static TypeInfo TypeInfo_inet = {true, leftmostvalue_inet, network_cmp};
+GIN_SUPPORT(inet, true, leftmostvalue_inet, network_cmp)
 
-GIN_SUPPORT(inet)
-
-static TypeInfo TypeInfo_cidr = {true, leftmostvalue_inet, network_cmp};
-
-GIN_SUPPORT(cidr)
+GIN_SUPPORT(cidr, true, leftmostvalue_inet, network_cmp)
 
 static Datum
 leftmostvalue_text(void)
 {
 	return PointerGetDatum(cstring_to_text_with_len("", 0));
 }
-static TypeInfo TypeInfo_text = {true, leftmostvalue_text, bttextcmp};
-
-GIN_SUPPORT(text)
+GIN_SUPPORT(text, true, leftmostvalue_text, bttextcmp)
 
 static Datum
 leftmostvalue_char(void)
 {
 	return CharGetDatum(SCHAR_MIN);
 }
-static TypeInfo TypeInfo_char = {false, leftmostvalue_char, btcharcmp};
-
-GIN_SUPPORT(char)
-
-static TypeInfo TypeInfo_bytea = {true, leftmostvalue_text, byteacmp};
+GIN_SUPPORT(char, false, leftmostvalue_char, btcharcmp)
 
-GIN_SUPPORT(bytea)
+GIN_SUPPORT(bytea, true, leftmostvalue_text, byteacmp)
 
 static Datum
 leftmostvalue_bit(void)
@@ -373,9 +351,7 @@ leftmostvalue_bit(void)
 							   ObjectIdGetDatum(0),
 							   Int32GetDatum(-1));
 }
-static TypeInfo TypeInfo_bit = {true, leftmostvalue_bit, bitcmp};
-
-GIN_SUPPORT(bit)
+GIN_SUPPORT(bit, true, leftmostvalue_bit, bitcmp)
 
 static Datum
 leftmostvalue_varbit(void)
@@ -385,9 +361,7 @@ leftmostvalue_varbit(void)
 							   ObjectIdGetDatum(0),
 							   Int32GetDatum(-1));
 }
-static TypeInfo TypeInfo_varbit = {true, leftmostvalue_varbit, bitcmp};
-
-GIN_SUPPORT(varbit)
+GIN_SUPPORT(varbit, true, leftmostvalue_varbit, bitcmp)
 
 /*
  * Numeric type hasn't a real left-most value, so we use PointerGetDatum(NULL)
@@ -431,7 +405,4 @@ leftmostvalue_numeric(void)
 {
 	return PointerGetDatum(NULL);
 }
-
-static TypeInfo TypeInfo_numeric = {true, leftmostvalue_numeric, gin_numeric_cmp};
-
-GIN_SUPPORT(numeric)
+GIN_SUPPORT(numeric, true, leftmostvalue_numeric, gin_numeric_cmp)
-- 
2.1.3

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

Reply via email to