Subject: Re: [HACKERS] user-defined numeric data types triggering ERROR:
 unsupported type
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
References: <b68441b6-d18f-13ab-b43b-9a72188a4e02@2ndquadrant.com>
 <9389.1506003842@sss.pgh.pa.us>
 <4b444362-4240-49e0-3523-724e5932b060@2ndquadrant.com>
 <24767.1506180446@sss.pgh.pa.us>
 <d526eb53-dbd6-d414-8e5d-21e644ce4549@2ndquadrant.com>
 <14597.1520038563@sss.pgh.pa.us>
 <42f4e37e-f923-6ea9-4172-cdec5a8c2d25@2ndquadrant.com>
Message-ID: <b5cede7b-354b-83e6-ab1f-7a1bf8a232fa@2ndquadrant.com>
Date: Sat, 3 Mar 2018 19:03:07 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.4.0
MIME-Version: 1.0
In-Reply-To: <42f4e37e-f923-6ea9-4172-cdec5a8c2d25@2ndquadrant.com>
Content-Type: multipart/mixed;
 boundary="------------23DF37F91CFE0EE510DA46FD"
Content-Language: en-US
Precedence: bulk

This is a multi-part message in MIME format.
--------------23DF37F91CFE0EE510DA46FD
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit

Hi,

Here is v2 of the fix. It does handle all the convert_to_scalar calls
for various data types, just like the numeric one did in v1 with the
exception of bytea.

The bytea case is fixed by checking that the boundary values are
varlenas. This seems better than checking for BYTEAOID explicitly, which
would fail for custom varlena-based types. At first I've been thinking
there might be issues when the data types has mismatching ordering, but
I don't think the patch makes it any worse.

I've also added a bunch of regression tests, checking each case. The
bytea test it should cause segfault on master, of course.

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--------------23DF37F91CFE0EE510DA46FD
Content-Type: text/x-patch;
 name="unsupported-type-fix-v2.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="unsupported-type-fix-v2.patch"

diff --git a/src/backend/utils/adt/network.c b/src/backend/utils/adt/network.c
index aac7621..34cf336 100644
--- a/src/backend/utils/adt/network.c
+++ b/src/backend/utils/adt/network.c
@@ -904,7 +904,7 @@ inet_merge(PG_FUNCTION_ARGS)
  * involving network types.
  */
 double
-convert_network_to_scalar(Datum value, Oid typid)
+convert_network_to_scalar(Datum value, Oid typid, bool *unknown_type)
 {
 	switch (typid)
 	{
@@ -960,7 +960,7 @@ convert_network_to_scalar(Datum value, Oid typid)
 	 * Can't get here unless someone tries to use scalarineqsel() on an
 	 * operator with one network and one non-network operand.
 	 */
-	elog(ERROR, "unsupported type: %u", typid);
+	*unknown_type = true;
 	return 0;
 }
 
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index fcc8323..b210541 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -176,7 +176,7 @@ static bool estimate_multivariate_ndistinct(PlannerInfo *root,
 static bool convert_to_scalar(Datum value, Oid valuetypid, double *scaledvalue,
 				  Datum lobound, Datum hibound, Oid boundstypid,
 				  double *scaledlobound, double *scaledhibound);
-static double convert_numeric_to_scalar(Datum value, Oid typid);
+static double convert_numeric_to_scalar(Datum value, Oid typid, bool *unknown_type);
 static void convert_string_to_scalar(char *value,
 						 double *scaledvalue,
 						 char *lobound,
@@ -193,8 +193,9 @@ static double convert_one_string_to_scalar(char *value,
 							 int rangelo, int rangehi);
 static double convert_one_bytea_to_scalar(unsigned char *value, int valuelen,
 							int rangelo, int rangehi);
-static char *convert_string_datum(Datum value, Oid typid);
-static double convert_timevalue_to_scalar(Datum value, Oid typid);
+static char *convert_string_datum(Datum value, Oid typid, bool *unknown_type);
+static double convert_timevalue_to_scalar(Datum value, Oid typid,
+							bool *unknown_type);
 static void examine_simple_variable(PlannerInfo *root, Var *var,
 						VariableStatData *vardata);
 static bool get_variable_range(PlannerInfo *root, VariableStatData *vardata,
@@ -4071,10 +4072,18 @@ convert_to_scalar(Datum value, Oid valuetypid, double *scaledvalue,
 		case REGDICTIONARYOID:
 		case REGROLEOID:
 		case REGNAMESPACEOID:
-			*scaledvalue = convert_numeric_to_scalar(value, valuetypid);
-			*scaledlobound = convert_numeric_to_scalar(lobound, boundstypid);
-			*scaledhibound = convert_numeric_to_scalar(hibound, boundstypid);
-			return true;
+			{
+				bool	unknown_type = false;
+
+				*scaledvalue = convert_numeric_to_scalar(value, valuetypid,
+														 &unknown_type);
+				*scaledlobound = convert_numeric_to_scalar(lobound, boundstypid,
+														   &unknown_type);
+				*scaledhibound = convert_numeric_to_scalar(hibound, boundstypid,
+														   &unknown_type);
+
+				return (!unknown_type);
+			}
 
 			/*
 			 * Built-in string types
@@ -4085,9 +4094,18 @@ convert_to_scalar(Datum value, Oid valuetypid, double *scaledvalue,
 		case TEXTOID:
 		case NAMEOID:
 			{
-				char	   *valstr = convert_string_datum(value, valuetypid);
-				char	   *lostr = convert_string_datum(lobound, boundstypid);
-				char	   *histr = convert_string_datum(hibound, boundstypid);
+				bool	unknown_type = false;
+
+				char   *valstr = convert_string_datum(value, valuetypid,
+													  &unknown_type);
+				char   *lostr = convert_string_datum(lobound, boundstypid,
+													 &unknown_type);
+				char   *histr = convert_string_datum(hibound, boundstypid,
+													 &unknown_type);
+
+				/* bail out if any of the values is not a string */
+				if (unknown_type)
+					return false;
 
 				convert_string_to_scalar(valstr, scaledvalue,
 										 lostr, scaledlobound,
@@ -4103,6 +4121,18 @@ convert_to_scalar(Datum value, Oid valuetypid, double *scaledvalue,
 			 */
 		case BYTEAOID:
 			{
+				TypeCacheEntry *typcache;
+
+				/*
+				 * Ensure parameters passed to convert_bytea_to_scalar are
+				 * all varlena. We're dealing with bytea values, but this
+				 * seems like a reasonable way to handle custom types.
+				 */
+				typcache = lookup_type_cache(boundstypid, 0);
+
+				if (typcache->typlen != -1)
+					return false;
+
 				convert_bytea_to_scalar(value, scaledvalue,
 										lobound, scaledlobound,
 										hibound, scaledhibound);
@@ -4121,10 +4151,18 @@ convert_to_scalar(Datum value, Oid valuetypid, double *scaledvalue,
 		case TINTERVALOID:
 		case TIMEOID:
 		case TIMETZOID:
-			*scaledvalue = convert_timevalue_to_scalar(value, valuetypid);
-			*scaledlobound = convert_timevalue_to_scalar(lobound, boundstypid);
-			*scaledhibound = convert_timevalue_to_scalar(hibound, boundstypid);
-			return true;
+			{
+				bool	unknown_type = false;
+
+				*scaledvalue = convert_timevalue_to_scalar(value, valuetypid,
+														   &unknown_type);
+				*scaledlobound = convert_timevalue_to_scalar(lobound, boundstypid,
+															 &unknown_type);
+				*scaledhibound = convert_timevalue_to_scalar(hibound, boundstypid,
+															 &unknown_type);
+
+				return (!unknown_type);
+			}
 
 			/*
 			 * Built-in network types
@@ -4133,10 +4171,18 @@ convert_to_scalar(Datum value, Oid valuetypid, double *scaledvalue,
 		case CIDROID:
 		case MACADDROID:
 		case MACADDR8OID:
-			*scaledvalue = convert_network_to_scalar(value, valuetypid);
-			*scaledlobound = convert_network_to_scalar(lobound, boundstypid);
-			*scaledhibound = convert_network_to_scalar(hibound, boundstypid);
-			return true;
+			{
+				bool	unknown_type = false;
+
+				*scaledvalue = convert_network_to_scalar(value, valuetypid,
+														 &unknown_type);
+				*scaledlobound = convert_network_to_scalar(lobound, boundstypid,
+														 &unknown_type);
+				*scaledhibound = convert_network_to_scalar(hibound, boundstypid,
+														 &unknown_type);
+
+				return (!unknown_type);
+			}
 	}
 	/* Don't know how to convert */
 	*scaledvalue = *scaledlobound = *scaledhibound = 0;
@@ -4147,7 +4193,7 @@ convert_to_scalar(Datum value, Oid valuetypid, double *scaledvalue,
  * Do convert_to_scalar()'s work for any numeric data type.
  */
 static double
-convert_numeric_to_scalar(Datum value, Oid typid)
+convert_numeric_to_scalar(Datum value, Oid typid, bool *unknown_type)
 {
 	switch (typid)
 	{
@@ -4185,9 +4231,11 @@ convert_numeric_to_scalar(Datum value, Oid typid)
 
 	/*
 	 * Can't get here unless someone tries to use scalarineqsel() on an
-	 * operator with one numeric and one non-numeric operand.
+	 * operator with one numeric and one non-numeric operand. Or more
+	 * precisely, operand that we don't know how to transform to scalar.
 	 */
-	elog(ERROR, "unsupported type: %u", typid);
+	*unknown_type = true;
+
 	return 0;
 }
 
@@ -4340,7 +4388,7 @@ convert_one_string_to_scalar(char *value, int rangelo, int rangehi)
  * before continuing, so as to generate correct locale-specific results.
  */
 static char *
-convert_string_datum(Datum value, Oid typid)
+convert_string_datum(Datum value, Oid typid, bool *unknown_type)
 {
 	char	   *val;
 
@@ -4369,7 +4417,7 @@ convert_string_datum(Datum value, Oid typid)
 			 * Can't get here unless someone tries to use scalarineqsel() on
 			 * an operator with one string and one non-string operand.
 			 */
-			elog(ERROR, "unsupported type: %u", typid);
+			*unknown_type = true;
 			return NULL;
 	}
 
@@ -4524,7 +4572,7 @@ convert_one_bytea_to_scalar(unsigned char *value, int valuelen,
  * Do convert_to_scalar()'s work for any timevalue data type.
  */
 static double
-convert_timevalue_to_scalar(Datum value, Oid typid)
+convert_timevalue_to_scalar(Datum value, Oid typid, bool *unknown_type)
 {
 	switch (typid)
 	{
@@ -4574,7 +4622,7 @@ convert_timevalue_to_scalar(Datum value, Oid typid)
 	 * Can't get here unless someone tries to use scalarineqsel() on an
 	 * operator with one timevalue and one non-timevalue operand.
 	 */
-	elog(ERROR, "unsupported type: %u", typid);
+	*unknown_type = true;
 	return 0;
 }
 
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index 3e462f1..630eeba 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -103,7 +103,8 @@ extern int inet_net_pton(int af, const char *src,
 			  void *dst, size_t size);
 
 /* network.c */
-extern double convert_network_to_scalar(Datum value, Oid typid);
+extern double convert_network_to_scalar(Datum value, Oid typid,
+			  bool *unknown_type);
 extern Datum network_scan_first(Datum in);
 extern Datum network_scan_last(Datum in);
 extern void clean_ipv6_addr(int addr_family, char *addr);
diff --git a/src/test/regress/expected/convert_to_scalar.out b/src/test/regress/expected/convert_to_scalar.out
new file mode 100644
index 0000000..7b4fe2b
--- /dev/null
+++ b/src/test/regress/expected/convert_to_scalar.out
@@ -0,0 +1,105 @@
+-- tables used in these tests
+create table scalar_text (a text);
+insert into scalar_text select i::text from generate_series(1,1000) s(i);
+analyze scalar_text;
+create table scalar_int (a int);
+insert into scalar_int select i from generate_series(1,1000) s(i);
+analyze scalar_int;
+-- convert_bytea_to_scalar
+CREATE OR REPLACE FUNCTION int_bytea_proc(int, bytea) RETURNS bool AS $$
+BEGIN
+    RETURN ($1 > 500);
+END;
+$$ LANGUAGE plpgsql;
+CREATE OPERATOR > (
+    LEFTARG = int,
+    RIGHTARG = bytea,
+    PROCEDURE = int_bytea_proc,
+    RESTRICT = scalargtsel
+);
+EXPLAIN (COSTS OFF) SELECT * FROM scalar_int
+WHERE a > E'\x37666565353'::bytea;
+                   QUERY PLAN                    
+-------------------------------------------------
+ Seq Scan on scalar_int
+   Filter: (a > '\x37363636353635333533'::bytea)
+(2 rows)
+
+-- convert_numeric_to_scalar
+CREATE OR REPLACE FUNCTION text_int_proc(text, int) RETURNS bool AS $$
+BEGIN
+    RETURN ($1::int > 500);
+END;
+$$ LANGUAGE plpgsql;
+CREATE OPERATOR > (
+    LEFTARG = text,
+    RIGHTARG = int,
+    PROCEDURE = text_int_proc,
+    RESTRICT = scalargtsel
+);
+EXPLAIN (COSTS OFF) SELECT * FROM scalar_text WHERE a > 30;
+       QUERY PLAN        
+-------------------------
+ Seq Scan on scalar_text
+   Filter: (a > 30)
+(2 rows)
+
+-- convert_string_datum
+CREATE OR REPLACE FUNCTION int_text_proc(int, text) RETURNS bool AS $$
+BEGIN
+    RETURN ($1 > 500);
+END;
+$$ LANGUAGE plpgsql;
+CREATE OPERATOR > (
+    LEFTARG = int,
+    RIGHTARG = text,
+    PROCEDURE = int_text_proc,
+    RESTRICT = scalargtsel
+);
+EXPLAIN (COSTS OFF) SELECT * FROM scalar_int WHERE a > '30'::text;
+         QUERY PLAN         
+----------------------------
+ Seq Scan on scalar_int
+   Filter: (a > '30'::text)
+(2 rows)
+
+-- convert_timevalue_to_scalar
+CREATE OR REPLACE FUNCTION int_timestamp_proc(int, timestamptz) RETURNS bool AS $$
+BEGIN
+    RETURN ($1 > 500);
+END;
+$$ LANGUAGE plpgsql;
+CREATE OPERATOR > (
+    LEFTARG = int,
+    RIGHTARG = timestamptz,
+    PROCEDURE = int_timestamp_proc,
+    RESTRICT = scalargtsel
+);
+EXPLAIN (COSTS OFF) SELECT * FROM scalar_int WHERE a > now();
+       QUERY PLAN       
+------------------------
+ Seq Scan on scalar_int
+   Filter: (a > now())
+(2 rows)
+
+-- convert_network_to_scalar
+CREATE OR REPLACE FUNCTION int_inet_proc(int, inet) RETURNS bool AS $$
+BEGIN
+    RETURN ($1 > 500);
+END;
+$$ LANGUAGE plpgsql;
+CREATE OPERATOR > (
+    LEFTARG = int,
+    RIGHTARG = inet,
+    PROCEDURE = int_inet_proc,
+    RESTRICT = scalargtsel
+);
+EXPLAIN (COSTS OFF) SELECT * FROM scalar_int WHERE a > '127.0.0.1'::inet;
+            QUERY PLAN             
+-----------------------------------
+ Seq Scan on scalar_int
+   Filter: (a > '127.0.0.1'::inet)
+(2 rows)
+
+DROP TABLE scalar_int;
+DROP TABLE scalar_text;
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index ad9434f..019dcf5 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -60,7 +60,7 @@ test: create_index create_view
 # ----------
 # Another group of parallel tests
 # ----------
-test: create_aggregate create_function_3 create_cast constraints triggers inherit create_table_like typed_table vacuum drop_if_exists updatable_views rolenames roleattributes create_am hash_func
+test: create_aggregate create_function_3 create_cast constraints triggers inherit create_table_like typed_table vacuum drop_if_exists updatable_views rolenames roleattributes create_am hash_func convert_to_scalar
 
 # ----------
 # sanity_check does a vacuum, affecting the sort order of SELECT *
diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule
index 27cd498..050421e 100644
--- a/src/test/regress/serial_schedule
+++ b/src/test/regress/serial_schedule
@@ -69,6 +69,7 @@ test: create_view
 test: create_aggregate
 test: create_function_3
 test: create_cast
+test: convert_to_scalar
 test: constraints
 test: triggers
 test: inherit
diff --git a/src/test/regress/sql/convert_to_scalar.sql b/src/test/regress/sql/convert_to_scalar.sql
new file mode 100644
index 0000000..f984f48
--- /dev/null
+++ b/src/test/regress/sql/convert_to_scalar.sql
@@ -0,0 +1,105 @@
+-- tables used in these tests
+
+create table scalar_text (a text);
+
+insert into scalar_text select i::text from generate_series(1,1000) s(i);
+
+analyze scalar_text;
+
+
+create table scalar_int (a int);
+
+insert into scalar_int select i from generate_series(1,1000) s(i);
+
+analyze scalar_int;
+
+
+-- convert_bytea_to_scalar
+
+CREATE OR REPLACE FUNCTION int_bytea_proc(int, bytea) RETURNS bool AS $$
+BEGIN
+    RETURN ($1 > 500);
+END;
+$$ LANGUAGE plpgsql;
+
+CREATE OPERATOR > (
+    LEFTARG = int,
+    RIGHTARG = bytea,
+    PROCEDURE = int_bytea_proc,
+    RESTRICT = scalargtsel
+);
+
+EXPLAIN (COSTS OFF) SELECT * FROM scalar_int
+WHERE a > E'\x37666565353'::bytea;
+
+
+-- convert_numeric_to_scalar
+
+CREATE OR REPLACE FUNCTION text_int_proc(text, int) RETURNS bool AS $$
+BEGIN
+    RETURN ($1::int > 500);
+END;
+$$ LANGUAGE plpgsql;
+
+CREATE OPERATOR > (
+    LEFTARG = text,
+    RIGHTARG = int,
+    PROCEDURE = text_int_proc,
+    RESTRICT = scalargtsel
+);
+
+EXPLAIN (COSTS OFF) SELECT * FROM scalar_text WHERE a > 30;
+
+-- convert_string_datum
+
+CREATE OR REPLACE FUNCTION int_text_proc(int, text) RETURNS bool AS $$
+BEGIN
+    RETURN ($1 > 500);
+END;
+$$ LANGUAGE plpgsql;
+
+CREATE OPERATOR > (
+    LEFTARG = int,
+    RIGHTARG = text,
+    PROCEDURE = int_text_proc,
+    RESTRICT = scalargtsel
+);
+
+EXPLAIN (COSTS OFF) SELECT * FROM scalar_int WHERE a > '30'::text;
+
+-- convert_timevalue_to_scalar
+
+CREATE OR REPLACE FUNCTION int_timestamp_proc(int, timestamptz) RETURNS bool AS $$
+BEGIN
+    RETURN ($1 > 500);
+END;
+$$ LANGUAGE plpgsql;
+
+CREATE OPERATOR > (
+    LEFTARG = int,
+    RIGHTARG = timestamptz,
+    PROCEDURE = int_timestamp_proc,
+    RESTRICT = scalargtsel
+);
+
+EXPLAIN (COSTS OFF) SELECT * FROM scalar_int WHERE a > now();
+
+-- convert_network_to_scalar
+
+CREATE OR REPLACE FUNCTION int_inet_proc(int, inet) RETURNS bool AS $$
+BEGIN
+    RETURN ($1 > 500);
+END;
+$$ LANGUAGE plpgsql;
+
+CREATE OPERATOR > (
+    LEFTARG = int,
+    RIGHTARG = inet,
+    PROCEDURE = int_inet_proc,
+    RESTRICT = scalargtsel
+);
+
+EXPLAIN (COSTS OFF) SELECT * FROM scalar_int WHERE a > '127.0.0.1'::inet;
+
+DROP TABLE scalar_int;
+DROP TABLE scalar_text;

--------------23DF37F91CFE0EE510DA46FD--