From 4bb0f727d74eb23b637576cdea77f630a8863100 Mon Sep 17 00:00:00 2001 From: dentiny Date: Thu, 5 Dec 2024 12:03:14 +0000 Subject: [PATCH 1/3] disallow table creation with out-of-range scale/width --- src/pgduckdb_ddl.cpp | 43 +++++++++++++++++++ .../expected/array_type_support.out | 4 +- test/regression/expected/type_support.out | 16 ------- test/regression/sql/array_type_support.sql | 4 +- test/regression/sql/type_support.sql | 10 ----- 5 files changed, 47 insertions(+), 30 deletions(-) diff --git a/src/pgduckdb_ddl.cpp b/src/pgduckdb_ddl.cpp index ca0458cc..a64a4f14 100644 --- a/src/pgduckdb_ddl.cpp +++ b/src/pgduckdb_ddl.cpp @@ -46,6 +46,45 @@ static bool ctas_skip_data = false; static bool top_level_ddl = true; static ProcessUtility_hook_type prev_process_utility_hook = NULL; +// Get precision and scale from postgres type modifier. +static int +GetPgNumericPrecision(int32 typmod) { + return ((typmod - VARHDRSZ) >> 16) & 0xffff; +} +static int +GetPgNumericScale(int32 typmod) { + return (((typmod - VARHDRSZ) & 0x7ff) ^ 1024) - 1024; +} + +// If column is of type "numeric", check whether it's acceptable for duckdb; +// If not, exception is thrown via `elog`. +static void +ValidateColumnNumericType(Form_pg_attribute &attribute) { + auto &type = attribute->atttypid; + if (type != NUMERICOID) { + return; + } + auto &typmod = attribute->atttypmod; + auto precision = GetPgNumericPrecision(typmod); + auto scale = GetPgNumericScale(typmod); + // duckdb's "numeric" type's max supported precision is 38. + if (typmod == -1 || precision < 0 || scale < 0 || precision > 38) { + elog(ERROR, + "Unsupported type when creating column: type precision %d with scale %d is not supported by duckdb, which " + "only allows maximum precision 38", + precision, scale); + } +} + +// Validate new relation creation, if invalid throw exception via `elog`. +static void +ValidateRelationCreation(TupleDesc desc) { + for (int i = 0; i < desc->natts; i++) { + Form_pg_attribute attr = &desc->attrs[i]; + ValidateColumnNumericType(attr); + } +} + static void DuckdbHandleDDL(Node *parsetree) { if (!pgduckdb::IsExtensionRegistered()) { @@ -253,6 +292,10 @@ DECLARE_PG_FUNCTION(duckdb_create_table_trigger) { elog(ERROR, "Expected single table to be created, but found %" PRIu64, static_cast(SPI_processed)); } + // Check whether new table creation is supported in duckdb. + // TODO(hjiang): Same type validation should be applied to other DDL as well. + ValidateRelationCreation(SPI_tuptable->tupdesc); + HeapTuple tuple = SPI_tuptable->vals[0]; bool isnull; Datum relid_datum = SPI_getbinval(tuple, SPI_tuptable->tupdesc, 1, &isnull); diff --git a/test/regression/expected/array_type_support.out b/test/regression/expected/array_type_support.out index 35c31f3b..0f93cfbc 100644 --- a/test/regression/expected/array_type_support.out +++ b/test/regression/expected/array_type_support.out @@ -199,7 +199,7 @@ SELECT * FROM float8_array_1d; (4 rows) -- NUMERIC (single dimension) -CREATE TABLE numeric_array_1d(a NUMERIC[]); +CREATE TABLE numeric_array_1d(a NUMERIC(2, 1)[]); INSERT INTO numeric_array_1d SELECT CAST(a as NUMERIC[]) FROM (VALUES ('{1.1, 2.2, 3.3}'), (NULL), @@ -373,7 +373,7 @@ SELECT * FROM float8_array_2d; (5 rows) -- NUMERIC (two dimensions) -CREATE TABLE numeric_array_2d(a NUMERIC[][]); +CREATE TABLE numeric_array_2d(a NUMERIC(3, 1)[][]); INSERT INTO numeric_array_2d VALUES ('{{1.1,2.2},{3.3,4.4}}'), ('{{5.5,6.6,7.7},{8.8,9.9,10.1}}'), diff --git a/test/regression/expected/type_support.out b/test/regression/expected/type_support.out index c40ce762..06393041 100644 --- a/test/regression/expected/type_support.out +++ b/test/regression/expected/type_support.out @@ -201,21 +201,6 @@ SELECT * FROM float8_tbl; 4.582345020342342e+20 (3 rows) --- NUMERIC as DOUBLE -CREATE TABLE numeric_as_double(a NUMERIC); -INSERT INTO numeric_as_double SELECT a FROM (VALUES - (0.234234234), - (NULL), - (458234502034234234234.000012) -) t(a); -SELECT * FROM numeric_as_double; - a ------------------------ - 0.234234234 - - 4.582345020342342e+20 -(3 rows) - -- NUMERIC with a physical type of SMALLINT CREATE TABLE smallint_numeric(a NUMERIC(4, 2)); INSERT INTO smallint_numeric SELECT a FROM (VALUES @@ -330,7 +315,6 @@ DROP TABLE timestamp_tbl; DROP TABLE timestamptz_tbl; DROP TABLE float4_tbl; DROP TABLE float8_tbl; -DROP TABLE numeric_as_double; DROP TABLE smallint_numeric; DROP TABLE integer_numeric; DROP TABLE bigint_numeric; diff --git a/test/regression/sql/array_type_support.sql b/test/regression/sql/array_type_support.sql index 79c2e487..bbcd4536 100644 --- a/test/regression/sql/array_type_support.sql +++ b/test/regression/sql/array_type_support.sql @@ -125,7 +125,7 @@ INSERT INTO float8_array_1d SELECT CAST(a as FLOAT8[]) FROM (VALUES SELECT * FROM float8_array_1d; -- NUMERIC (single dimension) -CREATE TABLE numeric_array_1d(a NUMERIC[]); +CREATE TABLE numeric_array_1d(a NUMERIC(2, 1)[]); INSERT INTO numeric_array_1d SELECT CAST(a as NUMERIC[]) FROM (VALUES ('{1.1, 2.2, 3.3}'), (NULL), @@ -226,7 +226,7 @@ INSERT INTO float8_array_2d VALUES SELECT * FROM float8_array_2d; -- NUMERIC (two dimensions) -CREATE TABLE numeric_array_2d(a NUMERIC[][]); +CREATE TABLE numeric_array_2d(a NUMERIC(3, 1)[][]); INSERT INTO numeric_array_2d VALUES ('{{1.1,2.2},{3.3,4.4}}'), ('{{5.5,6.6,7.7},{8.8,9.9,10.1}}'), diff --git a/test/regression/sql/type_support.sql b/test/regression/sql/type_support.sql index 725ee143..aa3eb748 100644 --- a/test/regression/sql/type_support.sql +++ b/test/regression/sql/type_support.sql @@ -90,15 +90,6 @@ INSERT INTO float8_tbl SELECT CAST(a AS FLOAT8) FROM (VALUES ) t(a); SELECT * FROM float8_tbl; --- NUMERIC as DOUBLE -CREATE TABLE numeric_as_double(a NUMERIC); -INSERT INTO numeric_as_double SELECT a FROM (VALUES - (0.234234234), - (NULL), - (458234502034234234234.000012) -) t(a); -SELECT * FROM numeric_as_double; - -- NUMERIC with a physical type of SMALLINT CREATE TABLE smallint_numeric(a NUMERIC(4, 2)); INSERT INTO smallint_numeric SELECT a FROM (VALUES @@ -171,7 +162,6 @@ DROP TABLE timestamp_tbl; DROP TABLE timestamptz_tbl; DROP TABLE float4_tbl; DROP TABLE float8_tbl; -DROP TABLE numeric_as_double; DROP TABLE smallint_numeric; DROP TABLE integer_numeric; DROP TABLE bigint_numeric; From d29be834522cd22648bb6cf180d5cf07b53ab1dc Mon Sep 17 00:00:00 2001 From: hjiang Date: Fri, 6 Dec 2024 10:56:08 +0000 Subject: [PATCH 2/3] Add unsupported type test case --- test/regression/expected/unsupported_types.out | 2 ++ test/regression/schedule | 1 + test/regression/sql/unsupported_types.sql | 1 + 3 files changed, 4 insertions(+) create mode 100644 test/regression/expected/unsupported_types.out create mode 100644 test/regression/sql/unsupported_types.sql diff --git a/test/regression/expected/unsupported_types.out b/test/regression/expected/unsupported_types.out new file mode 100644 index 00000000..9ba6c3f1 --- /dev/null +++ b/test/regression/expected/unsupported_types.out @@ -0,0 +1,2 @@ +CREATE TEMP TABLE numeric_tbl (a NUMERIC) USING duckdb; +ERROR: Unsupported type when creating column: type precision 40 with scale 5 is not supported by duckdb, which only allows maximum precision 38 diff --git a/test/regression/schedule b/test/regression/schedule index 896686b6..ed7b4cd8 100644 --- a/test/regression/schedule +++ b/test/regression/schedule @@ -26,3 +26,4 @@ test: transaction_errors test: secrets test: prepare test: function +test: unsupported_types diff --git a/test/regression/sql/unsupported_types.sql b/test/regression/sql/unsupported_types.sql new file mode 100644 index 00000000..e71f0521 --- /dev/null +++ b/test/regression/sql/unsupported_types.sql @@ -0,0 +1 @@ +CREATE TEMP TABLE numeric_tbl (a NUMERIC) USING duckdb; From 634ad43bcea6fe971f144bb38a6c21509fdd5aeb Mon Sep 17 00:00:00 2001 From: dentiny Date: Sat, 21 Dec 2024 10:42:13 +0000 Subject: [PATCH 3/3] fix sql test --- test/regression/expected/unsupported_types.out | 5 +++-- test/regression/sql/unsupported_types.sql | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/test/regression/expected/unsupported_types.out b/test/regression/expected/unsupported_types.out index 9ba6c3f1..ff0a3abb 100644 --- a/test/regression/expected/unsupported_types.out +++ b/test/regression/expected/unsupported_types.out @@ -1,2 +1,3 @@ -CREATE TEMP TABLE numeric_tbl (a NUMERIC) USING duckdb; -ERROR: Unsupported type when creating column: type precision 40 with scale 5 is not supported by duckdb, which only allows maximum precision 38 +CREATE TEMP TABLE large_numeric_tbl (a NUMERIC) USING duckdb; +INSERT INTO large_numeric_tbl VALUES(856324.111122223333::NUMERIC(40,12)); +ERROR: (PGDuckDB/CreatePlan) Prepared query returned an error: 'Parser Error: Width must be between 1 and 38! diff --git a/test/regression/sql/unsupported_types.sql b/test/regression/sql/unsupported_types.sql index e71f0521..f509e0b8 100644 --- a/test/regression/sql/unsupported_types.sql +++ b/test/regression/sql/unsupported_types.sql @@ -1 +1,2 @@ -CREATE TEMP TABLE numeric_tbl (a NUMERIC) USING duckdb; +CREATE TEMP TABLE large_numeric_tbl (a NUMERIC) USING duckdb; +INSERT INTO large_numeric_tbl VALUES(856324.111122223333::NUMERIC(40,12));