Re: [HACKERS] array of domain types

2016-06-03 Thread Jim Nasby 

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

2016-06-03 Thread Konstantin Knizhnik 

On 03.06.2016 02:02, Rod Taylor wrote:



On Thu, Jun 2, 2016 at 10:42 AM, Konstantin Knizhnik 
> 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);



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

Re: [HACKERS] array of domain types

2016-06-02 Thread Rod Taylor 
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

2016-06-02 Thread Konstantin Knizhnik 

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

2016-06-02 Thread Tom Lane 
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

2016-06-02 Thread konstantin knizhnik 
On Jun 2, 2016, at 12:29 PM, Thom Brown wrote:On 2 June 2016 at 10:13, konstantin knizhnik  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.

domain.patch
Description: Binary data
Thom

Re: [HACKERS] array of domain types

2016-06-02 Thread Thom Brown 
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

2016-06-02 Thread konstantin knizhnik 

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

2016-06-01 Thread Thom Brown 
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

2016-06-01 Thread Konstantin Knizhnik 
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