From 607668391a48bcb8ff376979ab9373a4d1b7d08c Mon Sep 17 00:00:00 2001 From: Leo-XM-Zeng Date: Tue, 14 Jan 2025 21:52:26 +0800 Subject: [PATCH 1/5] Support the PostgreSQL domain --- src/pgduckdb_types.cpp | 36 +++++++++++- test/regression/expected/domain.out | 86 +++++++++++++++++++++++++++++ test/regression/schedule | 1 + test/regression/sql/domain.sql | 53 ++++++++++++++++++ 4 files changed, 173 insertions(+), 3 deletions(-) create mode 100644 test/regression/expected/domain.out create mode 100644 test/regression/sql/domain.sql diff --git a/src/pgduckdb_types.cpp b/src/pgduckdb_types.cpp index d8041272..ff8c479e 100644 --- a/src/pgduckdb_types.cpp +++ b/src/pgduckdb_types.cpp @@ -917,10 +917,40 @@ ConvertPostgresToBaseDuckColumnType(Form_pg_attribute &attribute) { duckdb::LogicalType ConvertPostgresToDuckColumnType(Form_pg_attribute &attribute) { + int dimensions = -1; + Oid save_typoid = InvalidOid; + + if (get_typtype(attribute->atttypid) == TYPTYPE_DOMAIN) { + /* If the domain is an array type, you need to obtain the corresponding array dimension information */ + if (type_is_array_domain(attribute->atttypid)) { + HeapTuple typeTuple = SearchSysCache1(TYPEOID, ObjectIdGetDatum(attribute->atttypid)); + dimensions = ((Form_pg_type) GETSTRUCT(typeTuple))->typndims; + ReleaseSysCache(typeTuple); + } + + save_typoid = attribute->atttypid; + /* It is a domain type that needs to be reduced to its base type */ + attribute->atttypid = getBaseType(attribute->atttypid); + } else if (type_is_array(attribute->atttypid)) { + Oid elem_type = get_base_element_type(attribute->atttypid); + if (OidIsValid(elem_type) && get_typtype(elem_type) == TYPTYPE_DOMAIN) { + save_typoid = attribute->atttypid; + /* When the member type of an array is domain, you need to build a base array type */ + attribute->atttypid = get_array_type(getBaseType(elem_type)); + } + } + auto base_type = ConvertPostgresToBaseDuckColumnType(attribute); - auto dimensions = attribute->attndims; - if (dimensions == 0) { - return base_type; + + if (save_typoid != InvalidOid) { + attribute->atttypid = save_typoid; + } + + if (dimensions == -1) { + dimensions = attribute->attndims; + if (dimensions == 0) { + return base_type; + } } for (int i = 0; i < dimensions; i++) { diff --git a/test/regression/expected/domain.out b/test/regression/expected/domain.out new file mode 100644 index 00000000..738363af --- /dev/null +++ b/test/regression/expected/domain.out @@ -0,0 +1,86 @@ +create domain domainvarchar varchar(5); +create domain domainnumeric numeric(8,2); +create domain domainint4 int4; +create domain domaintext text; +-- Test tables using domains +create table basictest + ( testint4 domainint4 + , testtext domaintext + , testvarchar domainvarchar + , testnumeric domainnumeric + ); +INSERT INTO basictest values ('88', 'haha', 'short', '123.12'); -- Good +INSERT INTO basictest values ('88', 'haha', 'short text', '123.12'); -- Bad varchar +ERROR: value too long for type character varying(5) +INSERT INTO basictest values ('88', 'haha', 'short', '123.1212'); -- Truncate numeric +select * from basictest; + testint4 | testtext | testvarchar | testnumeric +----------+----------+-------------+------------- + 88 | haha | short | 123.12 + 88 | haha | short | 123.12 +(2 rows) + +select testtext || testvarchar as concat, testnumeric + 42 as sum +from basictest; + concat | sum +-----------+--------- + hahashort | 165.120 + hahashort | 165.120 +(2 rows) + +select * from basictest where testtext = 'haha'; + testint4 | testtext | testvarchar | testnumeric +----------+----------+-------------+------------- + 88 | haha | short | 123.12 + 88 | haha | short | 123.12 +(2 rows) + +select * from basictest where testvarchar = 'short'; + testint4 | testtext | testvarchar | testnumeric +----------+----------+-------------+------------- + 88 | haha | short | 123.12 + 88 | haha | short | 123.12 +(2 rows) + +-- array_domain +create domain domain_int_array as INT[]; +CREATE TABLE domain_int_array_1d(a domain_int_array); +INSERT INTO domain_int_array_1d SELECT CAST(a as domain_int_array) FROM (VALUES + ('{1, 2, 3}'), + (NULL), + ('{4, 5, NULL, 7}'), + ('{}') +) t(a); +SELECT * FROM domain_int_array_1d; + a +-------------- + {1,2,3} + + {4,5,NULL,7} + {} +(4 rows) + +CREATE TABLE domain_int_array_2d(a domainint4[]); +INSERT INTO domain_int_array_2d SELECT CAST(a as domain_int_array) FROM (VALUES + ('{1, 2, 3}'), + (NULL), + ('{4, 5, NULL, 7}'), + ('{}') +) t(a); +SELECT * FROM domain_int_array_2d; + a +-------------- + {1,2,3} + + {4,5,NULL,7} + {} +(4 rows) + +drop table domain_int_array_2d; +drop table domain_int_array_1d; +drop domain domain_int_array; +drop table basictest; +drop domain domainvarchar restrict; +drop domain domainnumeric restrict; +drop domain domainint4 restrict; +drop domain domaintext; diff --git a/test/regression/schedule b/test/regression/schedule index 18d43899..2383f220 100644 --- a/test/regression/schedule +++ b/test/regression/schedule @@ -14,6 +14,7 @@ test: hugeint_conversion test: read_functions test: duckdb_only_functions test: duckdb_recycle +test: domain test: cte test: create_schema test: create_table_as diff --git a/test/regression/sql/domain.sql b/test/regression/sql/domain.sql new file mode 100644 index 00000000..cd7449cd --- /dev/null +++ b/test/regression/sql/domain.sql @@ -0,0 +1,53 @@ +create domain domainvarchar varchar(5); +create domain domainnumeric numeric(8,2); +create domain domainint4 int4; +create domain domaintext text; + +-- Test tables using domains +create table basictest + ( testint4 domainint4 + , testtext domaintext + , testvarchar domainvarchar + , testnumeric domainnumeric + ); + +INSERT INTO basictest values ('88', 'haha', 'short', '123.12'); -- Good +INSERT INTO basictest values ('88', 'haha', 'short text', '123.12'); -- Bad varchar +INSERT INTO basictest values ('88', 'haha', 'short', '123.1212'); -- Truncate numeric + +select * from basictest; + +select testtext || testvarchar as concat, testnumeric + 42 as sum +from basictest; + +select * from basictest where testtext = 'haha'; +select * from basictest where testvarchar = 'short'; + +-- array_domain +create domain domain_int_array as INT[]; +CREATE TABLE domain_int_array_1d(a domain_int_array); +INSERT INTO domain_int_array_1d SELECT CAST(a as domain_int_array) FROM (VALUES + ('{1, 2, 3}'), + (NULL), + ('{4, 5, NULL, 7}'), + ('{}') +) t(a); +SELECT * FROM domain_int_array_1d; + +CREATE TABLE domain_int_array_2d(a domainint4[]); +INSERT INTO domain_int_array_2d SELECT CAST(a as domain_int_array) FROM (VALUES + ('{1, 2, 3}'), + (NULL), + ('{4, 5, NULL, 7}'), + ('{}') +) t(a); +SELECT * FROM domain_int_array_2d; + +drop table domain_int_array_2d; +drop table domain_int_array_1d; +drop domain domain_int_array; +drop table basictest; +drop domain domainvarchar restrict; +drop domain domainnumeric restrict; +drop domain domainint4 restrict; +drop domain domaintext; From 52de693799cb0fde94af3048b0546977f868f7a1 Mon Sep 17 00:00:00 2001 From: Leo-XM-Zeng Date: Tue, 14 Jan 2025 22:34:01 +0800 Subject: [PATCH 2/5] format --- src/pgduckdb_types.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pgduckdb_types.cpp b/src/pgduckdb_types.cpp index ff8c479e..2b4fa1f6 100644 --- a/src/pgduckdb_types.cpp +++ b/src/pgduckdb_types.cpp @@ -923,8 +923,8 @@ ConvertPostgresToDuckColumnType(Form_pg_attribute &attribute) { if (get_typtype(attribute->atttypid) == TYPTYPE_DOMAIN) { /* If the domain is an array type, you need to obtain the corresponding array dimension information */ if (type_is_array_domain(attribute->atttypid)) { - HeapTuple typeTuple = SearchSysCache1(TYPEOID, ObjectIdGetDatum(attribute->atttypid)); - dimensions = ((Form_pg_type) GETSTRUCT(typeTuple))->typndims; + HeapTuple typeTuple = SearchSysCache1(TYPEOID, ObjectIdGetDatum(attribute->atttypid)); + dimensions = ((Form_pg_type)GETSTRUCT(typeTuple))->typndims; ReleaseSysCache(typeTuple); } From 93ffd1ca6902e858b41d21590aaaf50c9cbc5fdc Mon Sep 17 00:00:00 2001 From: Leo-XM-Zeng Date: Wed, 15 Jan 2025 09:58:51 +0800 Subject: [PATCH 3/5] Obtaining the SysCache requires a global lock --- src/pgduckdb_types.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/pgduckdb_types.cpp b/src/pgduckdb_types.cpp index 2b4fa1f6..1f995205 100644 --- a/src/pgduckdb_types.cpp +++ b/src/pgduckdb_types.cpp @@ -32,6 +32,7 @@ extern "C" { #include "pgduckdb/pgduckdb_filter.hpp" #include "pgduckdb/pgduckdb_detoast.hpp" +#include "pgduckdb/pgduckdb_process_lock.hpp" namespace pgduckdb { @@ -919,6 +920,7 @@ duckdb::LogicalType ConvertPostgresToDuckColumnType(Form_pg_attribute &attribute) { int dimensions = -1; Oid save_typoid = InvalidOid; + std::lock_guard lock(GlobalProcessLock::GetLock()); if (get_typtype(attribute->atttypid) == TYPTYPE_DOMAIN) { /* If the domain is an array type, you need to obtain the corresponding array dimension information */ From b3870d7a0cea72c1943d7ffccc1664a06bbfa99a Mon Sep 17 00:00:00 2001 From: Leo-XM-Zeng Date: Wed, 15 Jan 2025 10:02:00 +0800 Subject: [PATCH 4/5] Add some constraint checking test cases for domain --- test/regression/expected/domain.out | 25 +++++++++++++++++++++---- test/regression/sql/domain.sql | 16 ++++++++++++---- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/test/regression/expected/domain.out b/test/regression/expected/domain.out index 738363af..29235220 100644 --- a/test/regression/expected/domain.out +++ b/test/regression/expected/domain.out @@ -1,7 +1,7 @@ -create domain domainvarchar varchar(5); -create domain domainnumeric numeric(8,2); -create domain domainint4 int4; -create domain domaintext text; +create domain domainvarchar varchar(5) check (value is not null); +create domain domainnumeric numeric(8,2) check (value is not null); +create domain domainint4 int4 check (value > 0); +create domain domaintext text check (value is not null); -- Test tables using domains create table basictest ( testint4 domainint4 @@ -13,6 +13,23 @@ INSERT INTO basictest values ('88', 'haha', 'short', '123.12'); -- Good INSERT INTO basictest values ('88', 'haha', 'short text', '123.12'); -- Bad varchar ERROR: value too long for type character varying(5) INSERT INTO basictest values ('88', 'haha', 'short', '123.1212'); -- Truncate numeric +-- domain check +INSERT INTO basictest values ('-1', 'haha', 'short', '123.1212'); -- Bad int4 +ERROR: value for domain domainint4 violates check constraint "domainint4_check" +INSERT INTO basictest values ('88', NULL, 'short', '123.1212'); -- Bad text +ERROR: value for domain domaintext violates check constraint "domaintext_check" +INSERT INTO basictest values ('88', 'haha', NULL, '123.1212'); -- Bad varchar +ERROR: value for domain domainvarchar violates check constraint "domainvarchar_check" +INSERT INTO basictest values ('88', 'haha', 'short', NULL); -- Bad numeric +ERROR: value for domain domainnumeric violates check constraint "domainnumeric_check" +SELECT 5::domainint4; -- Good + domainint4 +------------ + 5 +(1 row) + +SELECT (-5)::domainint4; -- Bad int4 +ERROR: value for domain domainint4 violates check constraint "domainint4_check" select * from basictest; testint4 | testtext | testvarchar | testnumeric ----------+----------+-------------+------------- diff --git a/test/regression/sql/domain.sql b/test/regression/sql/domain.sql index cd7449cd..1421f63a 100644 --- a/test/regression/sql/domain.sql +++ b/test/regression/sql/domain.sql @@ -1,7 +1,7 @@ -create domain domainvarchar varchar(5); -create domain domainnumeric numeric(8,2); -create domain domainint4 int4; -create domain domaintext text; +create domain domainvarchar varchar(5) check (value is not null); +create domain domainnumeric numeric(8,2) check (value is not null); +create domain domainint4 int4 check (value > 0); +create domain domaintext text check (value is not null); -- Test tables using domains create table basictest @@ -15,6 +15,14 @@ INSERT INTO basictest values ('88', 'haha', 'short', '123.12'); -- Good INSERT INTO basictest values ('88', 'haha', 'short text', '123.12'); -- Bad varchar INSERT INTO basictest values ('88', 'haha', 'short', '123.1212'); -- Truncate numeric +-- domain check +INSERT INTO basictest values ('-1', 'haha', 'short', '123.1212'); -- Bad int4 +INSERT INTO basictest values ('88', NULL, 'short', '123.1212'); -- Bad text +INSERT INTO basictest values ('88', 'haha', NULL, '123.1212'); -- Bad varchar +INSERT INTO basictest values ('88', 'haha', 'short', NULL); -- Bad numeric +SELECT 5::domainint4; -- Good +SELECT (-5)::domainint4; -- Bad int4 + select * from basictest; select testtext || testvarchar as concat, testnumeric + 42 as sum From 4cf80c45e591e305f73e51d76a76b51154679515 Mon Sep 17 00:00:00 2001 From: Leo-XM-Zeng Date: Wed, 15 Jan 2025 10:47:49 +0800 Subject: [PATCH 5/5] Add a SQL that is not currently supported --- test/regression/expected/domain.out | 6 ++++++ test/regression/sql/domain.sql | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/test/regression/expected/domain.out b/test/regression/expected/domain.out index 29235220..a49a6daf 100644 --- a/test/regression/expected/domain.out +++ b/test/regression/expected/domain.out @@ -30,6 +30,12 @@ SELECT 5::domainint4; -- Good SELECT (-5)::domainint4; -- Bad int4 ERROR: value for domain domainint4 violates check constraint "domainint4_check" +-- not support. It will be converted to the following statement +-- SELECT ('-5'::integer)::domainint4 AS domainint4 FROM pgduckdb.xxx.basictest +SELECT (-5)::domainint4 FROM basictest; +WARNING: (PGDuckDB/CreatePlan) Prepared query returned an error: 'Catalog Error: Type with name domainint4 does not exist! +Did you mean "tinyint"? +ERROR: value for domain domainint4 violates check constraint "domainint4_check" select * from basictest; testint4 | testtext | testvarchar | testnumeric ----------+----------+-------------+------------- diff --git a/test/regression/sql/domain.sql b/test/regression/sql/domain.sql index 1421f63a..a4b5395b 100644 --- a/test/regression/sql/domain.sql +++ b/test/regression/sql/domain.sql @@ -23,6 +23,10 @@ INSERT INTO basictest values ('88', 'haha', 'short', NULL); -- Bad numeric SELECT 5::domainint4; -- Good SELECT (-5)::domainint4; -- Bad int4 +-- not support. It will be converted to the following statement +-- SELECT ('-5'::integer)::domainint4 AS domainint4 FROM pgduckdb.xxx.basictest +SELECT (-5)::domainint4 FROM basictest; + select * from basictest; select testtext || testvarchar as concat, testnumeric + 42 as sum