Re: [HACKERS] array of domain types
On 6/3/16 11:50 AM, Konstantin Knizhnik wrote: Originally I was mostly interested in domains as kind of typedefs: convenient way to assign type to some particular kind of columns, for example object reference used in ORM. There are two main goals of using domain here: 1. Be able to easily change representation of object identifier, for example from integer to bigint. 2. Detect all columns containing references (distinguish them from columns containing just normal integers). I do not see any other mechanism in PostgreSQL which can address this problem (for example user defined type can not help here). I wonder if it is possible to support arrays of domain which do not have constraints? Or such partial support is worser than prohibiting arrays of domains at all? I don't know that domains without constraints gets you terribly much. At that point you could just create a brand new type using all the existing infrastructure (though admittedly that's a LOT more work than CREATE DOMAIN). I definitely think that domains should work the way you're envisioning. To me, they should be the exact same thing as any other type, except that they have constraints attached and a different named. You should be able to use them everywhere and in every way that you currently use a type. Ideally you'd even be able to create casts against them. I'm not suggesting you try and fix all those things at once, but I don't think we should add only partial support for arrays of domains. If you can have a domain array, it should work exactly how you'd expect, including all of the constraint checking. Before focusing further on the code, I think you should focus on adding appropriate regression tests to make sure things work correctly. I'm not sure what's currently tested, but what comes to mind is making certain that constraints work with a domain array when used both by themselves and as part of a composite type: - as an argument to a function - inside a sql function - as a plpgsql variable - inside a plpgsql function - as a table column So that's 5 x 2 (once for domain[], once for create type blah(x domain[])) test cases. There might be some other cases that are missing (what cast testing needs to happen?) -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.com 855-TREBLE2 (855-873-2532) mobile: 512-569-9461 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] array of domain types
On 03.06.2016 02:02, Rod Taylor wrote: On Thu, Jun 2, 2016 at 10:42 AM, Konstantin Knizhnik mailto:k.knizh...@postgrespro.ru>> wrote: On 02.06.2016 17:22, Tom Lane wrote: konstantin knizhnik mailto:k.knizh...@postgrespro.ru>> writes: Attached please find patch for DefineDomain function. You didn't attach the patch, Sorry, but I did attached the patch - I see the attachment in my mail received from the group. Multidimensional arrays work fine: knizhnik=# SELECT '{{14},{20}}'::teenager[][]; ERROR: value for domain teenager violates check constraint "teenager_check" LINE 1: SELECT '{{14},{20}}'::teenager[][]; ^ knizhnik=# SELECT '{{14},{19}}'::teenager[][]; teenager - {{14},{19}} (1 row) knizhnik=# SELECT ('{{14},{19}}'::teenager[][])[1][1]; teenager -- 14 (1 row) Domain of array of domain also works: I applied the domain.patch from above on HEAD, and all I get is cache lookup failures. The type_sanity regression test fails too. postgres=# CREATE DOMAIN teenager AS int CHECK (VALUE BETWEEN 13 AND 20); CREATE DOMAIN postgres=# CREATE DOMAIN teenager_groups AS teenager[]; CREATE DOMAIN postgres=# CREATE TABLE x (col teenager_groups); ERROR: cache lookup failed for type 0 Anyway, if that worked for me I would have done this which I expect will succeed when it shouldn't. INSERT INTO x VALUES (ARRAY[13,14,20]); ALTER DOMAIN teenager DROP CONSTRAINT teenager_check; ALTER DOMAIN teenager ADD CHECK (VALUE BETWEEN 13 AND 19); Sorry, the problem is more difficult than I originally expected:( Attached patch passes all regression tests and correctly handle conversion of arrays. But constraints are not checked for table columns. I failed to locate place where this check should be inserted... Originally I was mostly interested in domains as kind of typedefs: convenient way to assign type to some particular kind of columns, for example object reference used in ORM. There are two main goals of using domain here: 1. Be able to easily change representation of object identifier, for example from integer to bigint. 2. Detect all columns containing references (distinguish them from columns containing just normal integers). I do not see any other mechanism in PostgreSQL which can address this problem (for example user defined type can not help here). I wonder if it is possible to support arrays of domain which do not have constraints? Or such partial support is worser than prohibiting arrays of domains at all? -- Konstantin Knizhnik Postgres Professional: http://www.postgrespro.com The Russian Postgres Company diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 71d4df9..e227fa8 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -752,11 +752,13 @@ DefineDomain(CreateDomainStmt *stmt) ListCell *listptr; Oid basetypeoid; Oid old_type_oid; + Oid array_oid; Oid domaincoll; Form_pg_type baseType; int32 basetypeMod; Oid baseColl; ObjectAddress address; + char *array_type; /* Convert list of names to a name and namespace */ domainNamespace = QualifiedNameGetCreationNamespace(stmt->domainname, @@ -1023,6 +1025,14 @@ DefineDomain(CreateDomainStmt *stmt) } } + + /* + * OK, we're done checking, time to make the type. We must assign the + * array type OID ahead of calling TypeCreate, since the base type and + * array type each refer to the other. + */ + array_oid = AssignTypeArrayOid(); + /* * Have TypeCreate do all the real work. */ @@ -1047,7 +1057,7 @@ DefineDomain(CreateDomainStmt *stmt) analyzeProcedure, /* analyze procedure */ InvalidOid, /* no array element type */ false, /* this isn't an array */ - InvalidOid, /* no arrays for domains (yet) */ + array_oid, /* array type we are about to create */ basetypeoid, /* base type ID */ defaultValue, /* default type value (text) */ defaultValueBin, /* default type value (binary) */ @@ -1060,6 +1070,48 @@ DefineDomain(CreateDomainStmt *stmt) domaincoll); /* type's collation */ /* + * Create the array type that goes with it. + */ + array_type = makeArrayTypeName(domainName, domainNamespace); + +/* alignment must be 'i' or 'd' for arrays */ + alignment = (alignment == 'd') ? 'd' : 'i'; + + TypeCreate(array_oid,/* force assignment of this type OID */ + array_type,/* type name */ + domainNamespace,/* namespace */ + InvalidOid,/* relation oid (n/a here) */ + 0,/* relation kind (ditto) */ + GetUserId(),/* owner's ID */ + -1,/* internal size (always varlena) */ + TYPTYPE_BASE,/* type-type (base type) */ + TYPCATEGORY_ARRAY,/* type-category (array) */ + false,/* array types are never preferred */ + delimiter,/* array element delimiter */ + F_ARRAY
Re: [HACKERS] array of domain types
On Thu, Jun 2, 2016 at 10:42 AM, Konstantin Knizhnik < k.knizh...@postgrespro.ru> wrote: > On 02.06.2016 17:22, Tom Lane wrote: > >> konstantin knizhnik writes: >> >>> Attached please find patch for DefineDomain function. >>> >> You didn't attach the patch, >> > > Sorry, but I did attached the patch - I see the attachment in my mail > received from the group. > Multidimensional arrays work fine: > > knizhnik=# SELECT '{{14},{20}}'::teenager[][]; > ERROR: value for domain teenager violates check constraint > "teenager_check" > LINE 1: SELECT '{{14},{20}}'::teenager[][]; >^ > knizhnik=# SELECT '{{14},{19}}'::teenager[][]; > teenager > - > {{14},{19}} > (1 row) > > knizhnik=# SELECT ('{{14},{19}}'::teenager[][])[1][1]; > teenager > -- >14 > (1 row) > > > Domain of array of domain also works: > > I applied the domain.patch from above on HEAD, and all I get is cache lookup failures. The type_sanity regression test fails too. postgres=# CREATE DOMAIN teenager AS int CHECK (VALUE BETWEEN 13 AND 20); CREATE DOMAIN postgres=# CREATE DOMAIN teenager_groups AS teenager[]; CREATE DOMAIN postgres=# CREATE TABLE x (col teenager_groups); ERROR: cache lookup failed for type 0 Anyway, if that worked for me I would have done this which I expect will succeed when it shouldn't. INSERT INTO x VALUES (ARRAY[13,14,20]); ALTER DOMAIN teenager DROP CONSTRAINT teenager_check; ALTER DOMAIN teenager ADD CHECK (VALUE BETWEEN 13 AND 19);
Re: [HACKERS] array of domain types
On 02.06.2016 17:22, Tom Lane wrote: konstantin knizhnik writes: Attached please find patch for DefineDomain function. You didn't attach the patch, Sorry, but I did attached the patch - I see the attachment in my mail received from the group. Multidimensional arrays work fine: knizhnik=# SELECT '{{14},{20}}'::teenager[][]; ERROR: value for domain teenager violates check constraint "teenager_check" LINE 1: SELECT '{{14},{20}}'::teenager[][]; ^ knizhnik=# SELECT '{{14},{19}}'::teenager[][]; teenager - {{14},{19}} (1 row) knizhnik=# SELECT ('{{14},{19}}'::teenager[][])[1][1]; teenager -- 14 (1 row) Domain of array of domain also works: knizhnik=# create domain teenager_groups as teenager[]; CREATE DOMAIN knizhnik=# SELECT '{{14},{19}}'::teenager_groups; teenager_groups - {{14},{19}} (1 row) knizhnik=# SELECT '{{14},{20}}'::teenager_groups; ERROR: value for domain teenager violates check constraint "teenager_check" LINE 1: SELECT '{{14},{20}}'::teenager_groups; but in any case, I would be astonished if there is no work required beyond creating the matching array type. The reverse case (domains over arrays) has half a dozen special cases required to make it work smoothly. Probably the considerations on this side are totally different, but it's hard to believe there aren't any. One case that seems likely to be pretty squishy is an array of a domain over an array type. One would wish to be able to do foo[2][4] to extract an element of the contained array. That won't work as-is because the notation will be taken as a multi-dimensional subscript, but I would expect that (foo[2])[4] should work. Does it? Does ruleutils.c always include the necessary parens when reverse-listing such a construct? Is it possible to assign to such a sub-element, and if so, do the domain constraints get checked properly? Domain over an array that is of a domain type might be another fun case. regards, tom lane -- Konstantin Knizhnik Postgres Professional: http://www.postgrespro.com The Russian Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] array of domain types
konstantin knizhnik writes: > Attached please find patch for DefineDomain function. You didn't attach the patch, but in any case, I would be astonished if there is no work required beyond creating the matching array type. The reverse case (domains over arrays) has half a dozen special cases required to make it work smoothly. Probably the considerations on this side are totally different, but it's hard to believe there aren't any. One case that seems likely to be pretty squishy is an array of a domain over an array type. One would wish to be able to do foo[2][4] to extract an element of the contained array. That won't work as-is because the notation will be taken as a multi-dimensional subscript, but I would expect that (foo[2])[4] should work. Does it? Does ruleutils.c always include the necessary parens when reverse-listing such a construct? Is it possible to assign to such a sub-element, and if so, do the domain constraints get checked properly? Domain over an array that is of a domain type might be another fun case. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] array of domain types
On Jun 2, 2016, at 12:29 PM, Thom Brown wrote:On 2 June 2016 at 10:13, konstantin knizhnikwrote:Yes, it doesn't work:# CREATE DOMAIN teenager AS int CHECK (VALUE BETWEEN 13 AND 19);CREATE DOMAIN# SELECT 14::teenager; teenager -- 14(1 row)# SELECT 20::teenager;ERROR: value for domain teenager violates check constraint "teenager_check"# SELECT '{14,20}'::teenager[]; teenager -- {14,20}(1 row)That last one should fail.Yes, I see.This approach was wrong.Attached please find patch for DefineDomain function. domain.patch Description: Binary data Thom
Re: [HACKERS] array of domain types
On 2 June 2016 at 10:13, konstantin knizhnik wrote: > > On Jun 1, 2016, at 4:37 PM, Thom Brown wrote: > > On 1 June 2016 at 14:20, Konstantin Knizhnik > wrote: > >> I wonder why domain types can not be used for specification of array >> element: >> >> create domain objref as bigint; >> create table foo(x objref[]); >> ERROR: type "objref[]" does not exist >> create table foo(x bigint[]); >> CREATE TABLE >> >> Is there some principle problem here or it is just not implemented? > > > It's not implemented, but patches welcome. > > Thom > > > The patch is trivial: just use typbasetype in get_array_type if typtype > is TYPTYPE_DOMAIN: > > diff --git a/src/backend/utils/cache/lsyscache.c > b/src/backend/utils/cache/lsyscache.c > index cb26d79..ecfbb20 100644 > --- a/src/backend/utils/cache/lsyscache.c > +++ b/src/backend/utils/cache/lsyscache.c > @@ -2486,7 +2486,18 @@ get_array_type(Oid typid) > if (HeapTupleIsValid(tp)) > { > result = ((Form_pg_type) GETSTRUCT(tp))->typarray; > - ReleaseSysCache(tp); > + if (result == InvalidOid && ((Form_pg_type) > GETSTRUCT(tp))->typtype == TYPTYPE_DOMAIN) { > + typid = ((Form_pg_type) > GETSTRUCT(tp))->typbasetype; > + ReleaseSysCache(tp); > + tp = SearchSysCache1(TYPEOID, > ObjectIdGetDatum(typid)); > + if (HeapTupleIsValid(tp)) > + { > + result = ((Form_pg_type) > GETSTRUCT(tp))->typarray; > + ReleaseSysCache(tp); > + } > + } else { > + ReleaseSysCache(tp); > + } > } > return result; > } > > > Any problems with it? > > Yes, it doesn't work: # CREATE DOMAIN teenager AS int CHECK (VALUE BETWEEN 13 AND 19); CREATE DOMAIN # SELECT 14::teenager; teenager -- 14 (1 row) # SELECT 20::teenager; ERROR: value for domain teenager violates check constraint "teenager_check" # SELECT '{14,20}'::teenager[]; teenager -- {14,20} (1 row) That last one should fail. Thom
Re: [HACKERS] array of domain types
On Jun 1, 2016, at 4:37 PM, Thom Brown wrote: > On 1 June 2016 at 14:20, Konstantin Knizhnik > wrote: > I wonder why domain types can not be used for specification of array element: > > create domain objref as bigint; > create table foo(x objref[]); > ERROR: type "objref[]" does not exist > create table foo(x bigint[]); > CREATE TABLE > > Is there some principle problem here or it is just not implemented? > > It's not implemented, but patches welcome. > > Thom The patch is trivial: just use typbasetype in get_array_type if typtype is TYPTYPE_DOMAIN: diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c index cb26d79..ecfbb20 100644 --- a/src/backend/utils/cache/lsyscache.c +++ b/src/backend/utils/cache/lsyscache.c @@ -2486,7 +2486,18 @@ get_array_type(Oid typid) if (HeapTupleIsValid(tp)) { result = ((Form_pg_type) GETSTRUCT(tp))->typarray; - ReleaseSysCache(tp); + if (result == InvalidOid && ((Form_pg_type) GETSTRUCT(tp))->typtype == TYPTYPE_DOMAIN) { + typid = ((Form_pg_type) GETSTRUCT(tp))->typbasetype; + ReleaseSysCache(tp); + tp = SearchSysCache1(TYPEOID, ObjectIdGetDatum(typid)); + if (HeapTupleIsValid(tp)) + { + result = ((Form_pg_type) GETSTRUCT(tp))->typarray; + ReleaseSysCache(tp); + } + } else { + ReleaseSysCache(tp); + } } return result; } Any problems with it?
Re: [HACKERS] array of domain types
On 1 June 2016 at 14:20, Konstantin Knizhnik wrote: > I wonder why domain types can not be used for specification of array > element: > > create domain objref as bigint; > create table foo(x objref[]); > ERROR: type "objref[]" does not exist > create table foo(x bigint[]); > CREATE TABLE > > Is there some principle problem here or it is just not implemented? It's not implemented, but patches welcome. Thom
[HACKERS] array of domain types
I wonder why domain types can not be used for specification of array element: create domain objref as bigint; create table foo(x objref[]); ERROR: type "objref[]" does not exist create table foo(x bigint[]); CREATE TABLE Is there some principle problem here or it is just not implemented? -- Konstantin Knizhnik Postgres Professional: http://www.postgrespro.com The Russian Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers