Don't convert unsupported numerics to double by default (#795)

This stops converting unsupported NUMERIC sizes to doubles. This
behaviour is still supported, but it is now opt-in. This means pg_duckdb
won't silently reduce precision. It also starts requiring users to
specify precision when using NUMERIC types in a table definition,
because the default precision is different between PostgreSQL and
DuckDB.

Closes #471

---------

Co-authored-by: dentiny <[email protected]>

Author: JelteF

include/pgduckdb/pg/types.hpp

@@ -6,7 +6,7 @@ namespace pgduckdb::pg {
 bool IsArrayType(Oid type_oid);
 bool IsDomainType(Oid type_oid);
 bool IsArrayDomainType(Oid type_oid);
-Oid GetBaseDuckColumnType(Oid attribute_type_oid);
+Oid GetBaseTypeAndTypmod(Oid attribute_type_oid, int32_t *type_modifier);
 Datum StringToNumeric(const char *str);
 Datum StringToVarbit(const char *str);
 const char *VarbitToString(Datum pg_varbit);

include/pgduckdb/pgduckdb_guc.hpp

@@ -5,6 +5,7 @@ void InitGUC();
 
 extern bool duckdb_force_execution;
 extern bool duckdb_unsafe_allow_mixed_transactions;
+extern bool duckdb_convert_unsupported_numeric_to_double;
 extern bool duckdb_log_pg_explain;
 extern int duckdb_maximum_threads;
 extern char *duckdb_maximum_memory;

include/pgduckdb/pgduckdb_types.hpp

@@ -31,7 +31,7 @@ constexpr int64_t PGDUCKDB_MAX_TIMESTAMP_VALUE = 9223371244800000000;
 constexpr int64_t PGDUCKDB_MIN_TIMESTAMP_VALUE = -210866803200000000;
 
 duckdb::LogicalType ConvertPostgresToDuckColumnType(Form_pg_attribute &attribute);
-Oid GetPostgresDuckDBType(const duckdb::LogicalType &type);
+Oid GetPostgresDuckDBType(const duckdb::LogicalType &type, bool throw_error = false);
 int32_t GetPostgresDuckDBTypemod(const duckdb::LogicalType &type);
 duckdb::Value ConvertPostgresParameterToDuckValue(Datum value, Oid postgres_type);
 void ConvertPostgresToDuckValue(Oid attr_type, Datum value, duckdb::Vector &result, uint64_t offset);

src/pg/types.cpp

@@ -35,11 +35,11 @@ IsArrayDomainType(Oid type_oid) {
 }
 
 static Oid
-GetBaseDuckColumnType_C(Oid attribute_type_oid) {
+GetBaseTypeAndTypmod_C(Oid attribute_type_oid, int32 *type_modifier) {
 	Oid typoid = attribute_type_oid;
 	if (get_typtype(attribute_type_oid) == TYPTYPE_DOMAIN) {
 		/* It is a domain type that needs to be reduced to its base type */
-		typoid = getBaseType(attribute_type_oid);
+		typoid = getBaseTypeAndTypmod(attribute_type_oid, type_modifier);
 	} else if (type_is_array(attribute_type_oid)) {
 		Oid eltoid = get_base_element_type(attribute_type_oid);
 		if (OidIsValid(eltoid) && get_typtype(eltoid) == TYPTYPE_DOMAIN) {
@@ -50,9 +50,15 @@ GetBaseDuckColumnType_C(Oid attribute_type_oid) {
 	return typoid;
 }
 
+/*
+ * If the given type is a domain, return its base type and type_modifier;
+ * If the type is an array of a domain type, return the type of array-base
+ * type. This leaves the type_modifier unchanged.
+ * Otherwise, return the type's own OID, and leave *type_modifier unchanged.
+ */
 Oid
-GetBaseDuckColumnType(Oid attribute_type_oid) {
-	return PostgresFunctionGuard(GetBaseDuckColumnType_C, attribute_type_oid);
+GetBaseTypeAndTypmod(Oid attribute_type_oid, int32 *type_modifier) {
+	return PostgresFunctionGuard(GetBaseTypeAndTypmod_C, attribute_type_oid, type_modifier);
 }
 
 static Datum

src/pgduckdb_guc.cpp

@@ -111,6 +111,7 @@ DefineCustomDuckDBVariable(const char *name, const char *short_desc, T *var, T m
 
 bool duckdb_force_execution = false;
 bool duckdb_unsafe_allow_mixed_transactions = false;
+bool duckdb_convert_unsupported_numeric_to_double = false;
 bool duckdb_log_pg_explain = false;
 int duckdb_max_workers_per_postgres_scan = 2;
 char *duckdb_motherduck_session_hint = strdup("");
@@ -137,6 +138,10 @@ InitGUC() {
 	                     "Allow mixed transactions between DuckDB and Postgres",
 	                     &duckdb_unsafe_allow_mixed_transactions);
 
+	DefineCustomVariable("duckdb.convert_unsupported_numeric_to_double",
+	                     "Convert NUMERIC types of unsupported precision to DOUBLE",
+	                     &duckdb_convert_unsupported_numeric_to_double);
+
 	DefineCustomVariable("duckdb.log_pg_explain", "Logs the EXPLAIN plan of a Postgres scan at the NOTICE log level",
 	                     &duckdb_log_pg_explain);
 

src/pgduckdb_node.cpp

@@ -131,10 +131,8 @@ Duckdb_BeginCustomScan_Cpp(CustomScanState *cscanstate, EState *estate, int /*ef
 		}
 
 		for (size_t i = 0; i < prepared_result_types.size(); i++) {
-			Oid postgres_column_oid = pgduckdb::GetPostgresDuckDBType(prepared_result_types[i]);
-			if (!OidIsValid(postgres_column_oid)) {
-				elog(ERROR, "(PGDuckDB/CreatePlan) Cache lookup failed for type %u", postgres_column_oid);
-			}
+			Oid postgres_column_oid = pgduckdb::GetPostgresDuckDBType(prepared_result_types[i], true);
+
 			TargetEntry *target_entry =
 			    list_nth_node(TargetEntry, duckdb_scan_state->custom_scan->custom_scan_tlist, i);
 			Var *var = castNode(Var, target_entry->expr);

src/pgduckdb_planner.cpp

@@ -64,10 +64,9 @@ CreatePlan(Query *query, bool throw_error) {
 	auto &prepared_result_types = prepared_query->GetTypes();
 
 	for (size_t i = 0; i < prepared_result_types.size(); i++) {
-		Oid postgresColumnOid = pgduckdb::GetPostgresDuckDBType(prepared_result_types[i]);
+		Oid postgresColumnOid = pgduckdb::GetPostgresDuckDBType(prepared_result_types[i], throw_error);
 
 		if (!OidIsValid(postgresColumnOid)) {
-			elog(elevel, "(PGDuckDB/CreatePlan) Cache lookup failed for type %u", postgresColumnOid);
 			return nullptr;
 		}
 

src/pgduckdb_ruleutils.cpp

@@ -1,5 +1,6 @@
 #include "duckdb.hpp"
 #include "pgduckdb/pg/string_utils.hpp"
+#include "pgduckdb/pgduckdb_types.hpp"
 
 extern "C" {
 #include "postgres.h"
@@ -635,6 +636,15 @@ pgduckdb_get_tabledef(Oid relation_oid) {
 
 		const char *column_name = NameStr(column->attname);
 
+		/*
+		 * Check that this type is known by DuckDB, and throw the appropriate
+		 * error otherwise. This is particularly important for NUMERIC without
+		 * precision specified. Because that means something very different in
+		 * Postgres
+		 */
+		auto duck_type = pgduckdb::ConvertPostgresToDuckColumnType(column);
+		pgduckdb::GetPostgresDuckDBType(duck_type, true);
+
 		const char *column_type_name = format_type_with_typemod(column->atttypid, column->atttypmod);
 
 		if (first_column_printed) {

src/pgduckdb_types.cpp

@@ -5,6 +5,7 @@
 #include "duckdb/common/types/blob.hpp"
 #include "duckdb/common/types/uuid.hpp"
 
+#include "pgduckdb/pgduckdb_guc.hpp"
 #include "pgduckdb/pgduckdb_types.hpp"
 #include "pgduckdb/pgduckdb_metadata_cache.hpp"
 #include "pgduckdb/pgduckdb_utils.hpp"
@@ -1211,7 +1212,8 @@ numeric_typmod_scale(int32 typmod) {
 
 duckdb::LogicalType
 ConvertPostgresToBaseDuckColumnType(Form_pg_attribute &attribute) {
-	Oid typoid = pg::GetBaseDuckColumnType(attribute->atttypid);
+	int32 type_modifier = attribute->atttypmod;
+	Oid typoid = pg::GetBaseTypeAndTypmod(attribute->atttypid, &type_modifier);
 	switch (typoid) {
 	case BOOLOID:
 	case BOOLARRAYOID:
@@ -1265,12 +1267,40 @@ ConvertPostgresToBaseDuckColumnType(Form_pg_attribute &attribute) {
 		return duckdb::LogicalTypeId::DOUBLE;
 	case NUMERICOID:
 	case NUMERICARRAYOID: {
-		auto &typmod = attribute->atttypmod;
-		auto precision = numeric_typmod_precision(typmod);
-		auto scale = numeric_typmod_scale(typmod);
-		if (typmod == -1 || precision < 0 || scale < 0 || precision > 38) {
-			auto extra_type_info = duckdb::make_shared_ptr<NumericAsDouble>();
-			return duckdb::LogicalType(duckdb::LogicalTypeId::DOUBLE, std::move(extra_type_info));
+		auto precision = numeric_typmod_precision(type_modifier);
+		auto scale = numeric_typmod_scale(type_modifier);
+
+		/*
+		 * DuckDB decimals only support up to 38 digits. So we cannot convert
+		 * NUMERICs of higher precision losslessly. We do allow conversion to
+		 * doubles.
+		 * https://duckdb.org/docs/stable/sql/data_types/numeric.html#fixed-point-decimals
+		 */
+		if (type_modifier == -1 || precision < 1 || precision > 38 || scale < 0 || scale > 38 || scale > precision) {
+			if (duckdb_convert_unsupported_numeric_to_double) {
+				auto extra_type_info = duckdb::make_shared_ptr<NumericAsDouble>();
+				return duckdb::LogicalType(duckdb::LogicalTypeId::DOUBLE, std::move(extra_type_info));
+			}
+
+			/* We don't allow conversion then! */
+			if (type_modifier == -1) {
+				return duckdb::LogicalType::USER(
+				    "DuckDB requires the precision of a NUMERIC to be set. You can choose to convert these NUMERICs to "
+				    "a DOUBLE by using 'SET duckdb.duckdb.convert_unsupported_numeric_to_double = true'");
+			} else if (precision < 1 || precision > 38) {
+				return duckdb::LogicalType::USER(
+				    "DuckDB only supports NUMERIC with a precision of 1-38. You can choose to convert these NUMERICs "
+				    "to a DOUBLE by using 'SET duckdb.duckdb.convert_unsupported_numeric_to_double = true'");
+			} else if (scale < 0 || scale > 38) {
+				return duckdb::LogicalType::USER(
+				    "DuckDB only supports NUMERIC with a scale of 0-38. You can choose to convert these NUMERICs to a "
+				    "DOUBLE by using 'SET duckdb.duckdb.convert_unsupported_numeric_to_double = true'");
+			} else {
+				return duckdb::LogicalType::USER(
+				    "DuckDB does not support NUMERIC with a scale that is larger than the precision. You can choose to "
+				    "convert these NUMERICs to a DOUBLE by using 'SET "
+				    "duckdb.duckdb.convert_unsupported_numeric_to_double = true'");
+			}
 		}
 
 		return duckdb::LogicalType::DECIMAL(precision, scale);
@@ -1295,7 +1325,8 @@ ConvertPostgresToBaseDuckColumnType(Form_pg_attribute &attribute) {
 		} else if (typoid == pgduckdb::DuckdbStructOid()) {
 			return duckdb::LogicalTypeId::STRUCT;
 		}
-		return duckdb::LogicalType::USER("UnsupportedPostgresType (Oid=" + std::to_string(attribute->atttypid) + ")");
+		return duckdb::LogicalType::USER(
+		    "No conversion to DuckDB available for type with oid=" + std::to_string(attribute->atttypid) + ")");
 	}
 }
 
@@ -1339,7 +1370,7 @@ ConvertPostgresToDuckColumnType(Form_pg_attribute &attribute) {
 }
 
 Oid
-GetPostgresArrayDuckDBType(const duckdb::LogicalType &type) {
+GetPostgresArrayDuckDBType(const duckdb::LogicalType &type, bool throw_error) {
 	switch (type.id()) {
 	case duckdb::LogicalTypeId::BOOLEAN:
 		return BOOLARRAYOID;
@@ -1385,16 +1416,30 @@ GetPostgresArrayDuckDBType(const duckdb::LogicalType &type) {
 		return UUIDARRAYOID;
 	case duckdb::LogicalTypeId::BLOB:
 		return BYTEAARRAYOID;
+	case duckdb::LogicalTypeId::VARINT:
+		return NUMERICARRAYOID;
+	case duckdb::LogicalTypeId::USER: {
+		std::string type_name = duckdb::UserType::GetTypeName(type);
+		if (throw_error) {
+			throw duckdb::NotImplementedException("Unsupported Postgres type: " + type_name);
+		} else {
+			pd_log(WARNING, "Unsupported Postgres type: %s", type_name.c_str());
+			return InvalidOid;
+		}
+	}
 	default: {
-		elog(WARNING, "(PGDuckDB/GetPostgresDuckDBType) Unsupported `LIST` subtype %d to Postgres type",
-		     static_cast<uint8_t>(type.id()));
-		return InvalidOid;
+		if (throw_error) {
+			throw duckdb::NotImplementedException("Unsupported DuckDB `LIST` subtype: " + type.ToString());
+		} else {
+			pd_log(WARNING, "Unsupported DuckDB `LIST` subtype: %s", type.ToString().c_str());
+			return InvalidOid;
+		}
 	}
 	}
 }
 
 Oid
-GetPostgresDuckDBType(const duckdb::LogicalType &type) {
+GetPostgresDuckDBType(const duckdb::LogicalType &type, bool throw_error) {
 	switch (type.id()) {
 	case duckdb::LogicalTypeId::BOOLEAN:
 		return BOOLOID;
@@ -1454,7 +1499,7 @@ GetPostgresDuckDBType(const duckdb::LogicalType &type) {
 			auto &child_type = pgduckdb::GetChildType(*duck_type);
 			duck_type = &child_type;
 		}
-		return GetPostgresArrayDuckDBType(*duck_type);
+		return GetPostgresArrayDuckDBType(*duck_type, throw_error);
 	}
 	case duckdb::LogicalTypeId::BLOB:
 		return BYTEAOID;
@@ -1464,10 +1509,23 @@ GetPostgresDuckDBType(const duckdb::LogicalType &type) {
 		return pgduckdb::DuckdbMapOid();
 	case duckdb::LogicalTypeId::ENUM:
 		return VARCHAROID;
+	case duckdb::LogicalTypeId::USER: {
+		std::string type_name = duckdb::UserType::GetTypeName(type);
+		if (throw_error) {
+			throw duckdb::NotImplementedException("Unsupported Postgres type: " + type_name);
+		} else {
+			pd_log(WARNING, "Unsupported Postgres type: %s", type_name.c_str());
+			return InvalidOid;
+		}
+	}
 	default: {
-		elog(WARNING, "(PGDuckDB/GetPostgresDuckDBType) Could not convert DuckDB type: %s to Postgres type",
-		     type.ToString().c_str());
-		return InvalidOid;
+		if (throw_error) {
+			throw duckdb::NotImplementedException("Could not convert DuckDB type: " + type.ToString() +
+			                                      " to Postgres type");
+		} else {
+			pd_log(WARNING, "Could not convert DuckDB type: %s to Postgres type", type.ToString().c_str());
+			return InvalidOid;
+		}
 	}
 	}
 }

test/regression/expected/array_type_support.out

@@ -280,6 +280,7 @@ INSERT INTO numeric_array_1d SELECT CAST(a as NUMERIC[]) FROM (VALUES
     ('{}')
 ) t(a);
 SELECT * FROM numeric_array_1d;
+WARNING:  Unsupported Postgres type: DuckDB requires the precision of a NUMERIC to be set. You can choose to convert these NUMERICs to a DOUBLE by using 'SET duckdb.duckdb.convert_unsupported_numeric_to_double = true'
          a          
 --------------------
  {1.1,2.2,3.3}
@@ -288,6 +289,17 @@ SELECT * FROM numeric_array_1d;
  {}
 (4 rows)
 
+SET duckdb.convert_unsupported_numeric_to_double = true;
+SELECT * FROM numeric_array_1d;
+         a          
+--------------------
+ {1.1,2.2,3.3}
+ 
+ {4.4,5.5,NULL,7.7}
+ {}
+(4 rows)
+
+RESET duckdb.convert_unsupported_numeric_to_double;
 -- UUID (single dimension)
 CREATE TABLE uuid_array_1d(a UUID[]);
 INSERT INTO uuid_array_1d SELECT CAST(a as UUID[]) FROM (VALUES
@@ -555,6 +567,18 @@ INSERT INTO numeric_array_2d VALUES
     (NULL),
     ('{}'),
     ('{{11.1,12.2},{NULL,14.4}}');
+SELECT * FROM numeric_array_2d;
+WARNING:  Unsupported Postgres type: DuckDB requires the precision of a NUMERIC to be set. You can choose to convert these NUMERICs to a DOUBLE by using 'SET duckdb.duckdb.convert_unsupported_numeric_to_double = true'
+               a                
+--------------------------------
+ {{1.1,2.2},{3.3,4.4}}
+ {{5.5,6.6,7.7},{8.8,9.9,10.1}}
+ 
+ {}
+ {{11.1,12.2},{NULL,14.4}}
+(5 rows)
+
+SET duckdb.convert_unsupported_numeric_to_double = true;
 SELECT * FROM numeric_array_2d;
                a                
 --------------------------------
@@ -565,6 +589,7 @@ SELECT * FROM numeric_array_2d;
  {{11.1,12.2},{NULL,14.4}}
 (5 rows)
 
+RESET duckdb.convert_unsupported_numeric_to_double;
 -- UUID (two dimensions)
 CREATE TABLE uuid_array_2d(a UUID[][]);
 INSERT INTO uuid_array_2d VALUES