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
On 1 June 2016 at 14:20, Konstantin Knizhnik <k.knizhnik@postgrespro.ru>
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 TABLEIs there some principle problem here or it is just not implemented?
It's not implemented, but patches welcome.
Thom
On Jun 1, 2016, at 4:37 PM, Thom Brown wrote:
On 1 June 2016 at 14:20, Konstantin Knizhnik <k.knizhnik@postgrespro.ru> 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 TABLEIs 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?
On 2 June 2016 at 10:13, konstantin knizhnik <k.knizhnik@postgrespro.ru>
wrote:
On Jun 1, 2016, at 4:37 PM, Thom Brown wrote:
On 1 June 2016 at 14:20, Konstantin Knizhnik <k.knizhnik@postgrespro.ru>
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 TABLEIs 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
On Jun 2, 2016, at 12:29 PM, Thom Brown wrote:
On 2 June 2016 at 10:13, konstantin knizhnik <k.knizhnik@postgrespro.ru> wrote:
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.
Show quoted text
Thom
Attachments:
domain.patchapplication/octet-stream; name=domain.patchDownload
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 4f41766..03a92f0 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -751,11 +751,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,
@@ -1022,6 +1024,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.
*/
@@ -1046,7 +1056,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) */
@@ -1059,6 +1069,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_DOMAIN,/* type-type (base type) */
+ TYPCATEGORY_ARRAY,/* type-category (array) */
+ false,/* array types are never preferred */
+ delimiter,/* array element delimiter */
+ F_ARRAY_IN,/* input procedure */
+ F_ARRAY_OUT,/* output procedure */
+ F_ARRAY_RECV,/* receive procedure */
+ F_ARRAY_SEND,/* send procedure */
+ InvalidOid,/* typmodin procedure */
+ InvalidOid,/* typmodout procedure */
+ F_ARRAY_TYPANALYZE,/* analyze procedure */
+ address.objectId,/* element type ID */
+ true,/* yes this is an array type */
+ InvalidOid,/* no further array type */
+ InvalidOid,/* base type ID */
+ NULL,/* never a default type value */
+ NULL,/* binary default isn't sent either */
+ false,/* never passed by value */
+ alignment,/* see above */
+ 'x',/* ARRAY is always toastable */
+ -1,/* typMod (Domains only) */
+ 0,/* Array dimensions of typbasetype */
+ false,/* Type NOT NULL */
+ domaincoll);/* type's collation */
+
+ pfree(array_type);
+
+ /*
* Process constraints which refer to the domain ID returned by TypeCreate
*/
foreach(listptr, schema)
konstantin knizhnik <k.knizhnik@postgrespro.ru> 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
On 02.06.2016 17:22, Tom Lane wrote:
konstantin knizhnik <k.knizhnik@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:
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
On Thu, Jun 2, 2016 at 10:42 AM, Konstantin Knizhnik <
k.knizhnik@postgrespro.ru> wrote:
On 02.06.2016 17:22, Tom Lane wrote:
konstantin knizhnik <k.knizhnik@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);
On 03.06.2016 02:02, Rod Taylor wrote:
On Thu, Jun 2, 2016 at 10:42 AM, Konstantin Knizhnik
<k.knizhnik@postgrespro.ru <mailto:k.knizhnik@postgrespro.ru>> wrote:On 02.06.2016 17:22, Tom Lane wrote:
konstantin knizhnik <k.knizhnik@postgrespro.ru
<mailto:k.knizhnik@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 0Anyway, 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
Attachments:
domain.patchtext/x-patch; name=domain.patchDownload
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_IN,/* input procedure */
+ F_ARRAY_OUT,/* output procedure */
+ F_ARRAY_RECV,/* receive procedure */
+ F_ARRAY_SEND,/* send procedure */
+ InvalidOid,/* typmodin procedure */
+ InvalidOid,/* typmodout procedure */
+ F_ARRAY_TYPANALYZE,/* analyze procedure */
+ address.objectId,/* element type ID */
+ true,/* yes this is an array type */
+ InvalidOid,/* no further array type */
+ InvalidOid,/* base type ID */
+ NULL,/* never a default type value */
+ NULL,/* binary default isn't sent either */
+ false,/* never passed by value */
+ alignment,/* see above */
+ 'x',/* ARRAY is always toastable */
+ -1,/* typMod (Domains only) */
+ 0,/* Array dimensions of typbasetype */
+ false,/* Type NOT NULL */
+ domaincoll);/* type's collation */
+
+ pfree(array_type);
+
+ /*
* Process constraints which refer to the domain ID returned by TypeCreate
*/
foreach(listptr, schema)
@@ -2846,8 +2898,6 @@ get_rels_with_domain(Oid domainOid, LOCKMODE lockmode)
/* Check for directly dependent types --- must be domains */
if (pg_depend->classid == TypeRelationId)
{
- Assert(get_typtype(pg_depend->objid) == TYPTYPE_DOMAIN);
-
/*
* Recursively add dependent columns to the output list. This is
* a bit inefficient since we may fail to combine RelToCheck
diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c
index c0ca58b..df4674f 100644
--- a/src/backend/executor/execQual.c
+++ b/src/backend/executor/execQual.c
@@ -4807,9 +4807,6 @@ ExecInitExpr(Expr *node, PlanState *parent)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("target type is not an array")));
- /* Arrays over domains aren't supported yet */
- Assert(getBaseType(astate->resultelemtype) ==
- astate->resultelemtype);
astate->elemfunc.fn_oid = InvalidOid; /* not initialized */
astate->amstate = (ArrayMapState *) palloc0(sizeof(ArrayMapState));
state = (ExprState *) astate;
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