Re: Basic DOMAIN Support

I intend to add other parts of domain support later on (no reason to
hold up committing this though) but would appreciate some feedback
about what I've done.

What's there works, however I intend to finish it off with CHECK
and -- if I can figure out a good way -- REFERENCES.

Implements:
CREATE DOMAIN domain type [NULL | NOT NULL] [DEFAULT expression];
COMMENT ON DOMAIN domain IS '';
DROP DOMAIN domain [RESTRICT | CASCADE]; -- Doesn't actually restrict
due to pg_depends

Affects:
Types can be specified as NOT NULL. No interface is available to set
this for any type other than a domain however. Types may also use a
complex expression (b_expr) for their default.

Various Tasks (output from psql for some simple operations involving
domains):

NOTE: For DEFAULT NULL to have any effect in table creation the
default actually needs to be stored.

Since Type defaults have overridden NULL in the past, I left it so
domains would as well.

Below are some tests I used to check the implementation.

## DOMAIN TEST ##
create domain domainvarchar varchar(15);
create domain domainnumeric numeric(8,2);
create domain domainint4 int4;
create domain domaintext text;

-- Test tables using domains
create table basictest
( testint4 domainint4
, realint4 int4
, testtext domaintext
, realtext text
, testvarchar domainvarchar
, realvarchar varchar(15)
, testnumeric domainnumeric
, realnumeric numeric(8,2)
);

INSERT INTO basictest values ('88', '88', 'haha', 'haha', 'short
text', 'short text', '123.12', '123.12');
select * from basictest;

create domain dnotnull varchar(15) NOT NULL;
create domain dnull varchar(15) NULL;

-- NOT NULL in the domain cannot be overridden
create table nulltest
( col1 dnotnull
, col2 dnotnull NULL
, col3 dnull NOT NULL
, col4 dnull
);
INSERT INTO nulltest DEFAULT VALUES;
INSERT INTO nulltest values ('a', 'b', 'c', 'd'); -- Good
INSERT INTO nulltest values (NULL, 'b', 'c', 'd');
INSERT INTO nulltest values ('a', NULL, 'c', 'd');
INSERT INTO nulltest values ('a', 'b', NULL, 'd');
INSERT INTO nulltest values ('a', 'b', 'c', NULL); -- Good
select * from nulltest;

create domain ddef1 int4 DEFAULT 3;
create domain ddef2 numeric(8,6) DEFAULT random();
-- Type mixing, function returns int8
create domain ddef3 text DEFAULT random();
create sequence ddef4_seq;
create domain ddef4 int4 DEFAULT nextval(cast('ddef4_seq' as text));

create table defaulttest
( col1 ddef1
, col2 ddef2
, col3 ddef3
, col4 ddef4
, col5 ddef1 DEFAULT NULL
, col6 ddef2 DEFAULT '88.1'
, col7 ddef4 DEFAULT random() * 8000
);
insert into defaulttest default values;
insert into defaulttest default values;
insert into defaulttest default values;
select * from defaulttest;

## PSQL OUTPUT ##

newdb=# -- Test Comment / Drop
newdb=# create domain domaindroptest int4;
CREATE DOMAIN
newdb=# comment on domain domaindroptest is 'About to drop this..';
COMMENT
newdb=#
newdb=# select * from pg_type where typname = 'domaindroptest';
typname | typowner | typlen | typprtlen | typbyval | typtype |
typisdefined | typdelim | typrelid | typelem | typinput | typoutput |
typrecei
e | typsend | typalign | typstorage | typnotnull | typmod |
typdefaultbin | typdefault
----------------+----------+--------+-----------+----------+---------+
--------------+----------+----------+---------+----------+-----------+
---------
--+---------+----------+------------+------------+--------+-----------
----+------------
domaindroptest | 1 | 4 | 10 | t | d |
t | , | 0 | 23 | int4in | int4out |
int4in
| int4out | i | p | f | -1 |
|
(1 row)

newdb=#
newdb=# drop domain domaindroptest restrict;
DROP
newdb=#
newdb=# select * from pg_type where typname = 'domaindroptest';
typname | typowner | typlen | typprtlen | typbyval | typtype |
typisdefined | typdelim | typrelid | typelem | typinput | typoutput |
typreceive | ty
send | typalign | typstorage | typnotnull | typmod | typdefaultbin |
typdefault
---------+----------+--------+-----------+----------+---------+-------
-------+----------+----------+---------+----------+-----------+-------
-----+---
-----+----------+------------+------------+--------+---------------+--
----------
(0 rows)

newdb=# -- TEST Domains.
newdb=#
newdb=# create domain domainvarchar varchar(15);
CREATE DOMAIN
newdb=# create domain domainnumeric numeric(8,2);
CREATE DOMAIN
newdb=# create domain domainint4 int4;
CREATE DOMAIN
newdb=# create domain domaintext text;
CREATE DOMAIN
newdb=#
newdb=# -- Test tables using domains
newdb=# create table basictest
newdb-# ( testint4 domainint4
newdb(# , realint4 int4
newdb(# , testtext domaintext
newdb(# , realtext text
newdb(# , testvarchar domainvarchar
newdb(# , realvarchar varchar(15)
newdb(# , testnumeric domainnumeric
newdb(# , realnumeric numeric(8,2)
newdb(# );
CREATE
newdb=#
newdb=# INSERT INTO basictest values ('88', '88', 'haha', 'haha',
'short text', 'short text', '123.12', '123.12');
INSERT 90400 1
newdb=# select * from basictest;
testint4 | realint4 | testtext | realtext | testvarchar | realvarchar
| testnumeric | realnumeric
----------+----------+----------+----------+-------------+------------
-+-------------+-------------
88 | 88 | haha | haha | short text | short text
| 123.12 | 123.12
(1 row)

newdb=#
newdb=# create domain dnotnull varchar(15) NOT NULL;
CREATE DOMAIN
newdb=# create domain dnull varchar(15) NULL;
CREATE DOMAIN
newdb=# -- NOT NULL in the domain cannot be overridden
newdb=# create table nulltest
newdb-# ( col1 dnotnull
newdb(# , col2 dnotnull NULL
newdb(# , col3 dnull NOT NULL
newdb(# , col4 dnull
newdb(# );
CREATE
newdb=# INSERT INTO nulltest DEFAULT VALUES;
ERROR: ExecAppend: Fail to add null value in not null attribute col1
newdb=# INSERT INTO nulltest values ('a', 'b', 'c', 'd'); -- Good
INSERT 90408 1
newdb-# INSERT INTO nulltest values (NULL, 'b', 'c', 'd');
ERROR: ExecAppend: Fail to add null value in not null attribute col1
newdb=# INSERT INTO nulltest values ('a', NULL, 'c', 'd');
ERROR: ExecAppend: Fail to add null value in not null attribute col2
newdb=# INSERT INTO nulltest values ('a', 'b', NULL, 'd');
ERROR: ExecAppend: Fail to add null value in not null attribute col3
newdb=# INSERT INTO nulltest values ('a', 'b', 'c', NULL); -- Good
INSERT 90409 1
newdb-# select * from nulltest;
col1 | col2 | col3 | col4
------+------+------+------
a | b | c | d
a | b | c |
(2 rows)

newdb=# create domain ddef1 int4 DEFAULT 3;
CREATE DOMAIN
newdb=# create domain ddef2 numeric(8,6) DEFAULT random();
CREATE DOMAIN
newdb=# -- Type mixing, function returns int8
newdb=# create domain ddef3 text DEFAULT random();
CREATE DOMAIN
newdb=# create sequence ddef4_seq;
CREATE
newdb=# create domain ddef4 int4 DEFAULT nextval(cast('ddef4_seq' as
text));
CREATE DOMAIN
newdb=#
newdb=# create table defaulttest
newdb-# ( col1 ddef1
newdb(# , col2 ddef2
newdb(# , col3 ddef3
newdb(# , col4 ddef4
newdb(# , col5 ddef1 DEFAULT NULL
newdb(# , col6 ddef2 DEFAULT '88.1'
newdb(# , col7 ddef4 DEFAULT random() * 8000
newdb(# );
CREATE
newdb=# insert into defaulttest default values;
INSERT 90421 1
newdb=# insert into defaulttest default values;
INSERT 90422 1
newdb=# insert into defaulttest default values;
INSERT 90423 1
newdb=# select * from defaulttest;
col1 | col2 | col3 | col4 | col5 | col6 |
col7
------+-------------------+-------------------+------+------+------+--
----
3 | 0.186453586065422 | 0.391880722433273 | 1 | 3 | 88.1 |
1930
3 | 0.999444424174467 | 0.461114872461704 | 2 | 3 | 88.1 |
6024
3 | 0.837450824602251 | 0.632604472633733 | 3 | 3 | 88.1 |
7441
(3 rows)

--
Rod Taylor

Your eyes are weary from staring at the CRT. You feel sleepy. Notice
how restful it is to watch the cursor blink. Close your eyes. The
opinions stated above are yours. You cannot imagine why you ever felt
otherwise.

"Rod Taylor" <rbt(at)zort(dot)ca> writes:
> I intend to add other parts of domain support later on (no reason to
> hold up committing this though) but would appreciate some feedback
> about what I've done.

Still needs some work ...

One serious problem is that I do not think you can get away with reusing
typelem to link domains to base types. All the array code is going to
think that a domain is an array, and proceed to do horribly wrong
things. User applications may think the same thing, so even if you
wanted to change every backend routine that looks at typelem, it
wouldn't be enough. I think the only safe way to proceed is to add a
separate column that links a domain to its base type. This'd also save
you from having to add another meaning to typtype (since a domain could
be recognized by nonzero typbasetype). That should reduce the
likelihood of breaking existing code, and perhaps make life simpler when
it comes time to allow freestanding composite types (why shouldn't a
domain have a composite type as base?).

Come to think of it, even without freestanding composite types it'd be
possible to try to define a domain as a subtype of a composite type,
and to use same as (eg) a function argument or result type. I doubt
you are anywhere near making that behave reasonably, though. Might be
best to disallow it for now.

Speaking of arrays --- have you thought about arrays of domain-type
objects? I'm not sure whether any of the supported features matter for
array elements, but if they do it's not clear how to make it happen.

Another objection is the changes you made in execMain.c. That extra
syscache lookup for every field of every tuple is going to be a rather
nasty performance hit, especially seeing that people will pay it whether
they ever heard of domains or not. And it seems quite unnecessary; if
you copy the domain's notnull bit into the pg_attribute row, then the
existing coding will do fine, no?

I think also that you may have created some subtle changes in the
semantics of type default-value specifications; we'll need to think
if any compatibility problems have been introduced. There are doubtless
hardly any people using the feature, so this is not a serious objection,
but if any change has occurred it should be documented.

Overall, an impressive first cut!

regards, tom lane

> "Rod Taylor" <rbt(at)zort(dot)ca> writes:
> > I intend to add other parts of domain support later on (no reason
to
> > hold up committing this though) but would appreciate some feedback
> > about what I've done.
>
> Still needs some work ...
>
> One serious problem is that I do not think you can get away with
reusing
> typelem to link domains to base types. All the array code is going
to
<-- snip-->
> be recognized by nonzero typbasetype). That should reduce the
> likelihood of breaking existing code, and perhaps make life simpler
when
> it comes time to allow freestanding composite types (why shouldn't a
> domain have a composite type as base?).

Will add pg_type.typbasetype, and can really easily allow domains of
composite types if this is wanted. I don't really understand
composite types, so I have no idea how this would work.

> Speaking of arrays --- have you thought about arrays of domain-type
> objects? I'm not sure whether any of the supported features matter
for
> array elements, but if they do it's not clear how to make it happen.

I wondered about this too, but I've been able to regress the system as
well as use it for other basic tasks. The main reason that a domain
isn't a base type is to prevent a domain of domains. The few books I
have that describe domains state that they shouldn't. Marking it with
a 'd' makes it quite obvious it's a domain and not intended for
grouping into subdomains.

One can make a domain of an array easily enough, then apply
constraints to each segment of it -- but hadn't considered an array of
domains. In the end they can both come out to the same thing. Check
constraints aren't currently available of course, but the below will
work the same.

-- Potentially allowed, but untested (likley broken, but maybe
not...).
create domain dom1 int2[2] check (VALUE[1] > 5 and VALUE[2] > 5);
create table tab1 (arr1 dom1);

or

-- Not allowed, and not easily arranged.
create domain dom2 int2 check (VALUE > 5);
create table tab2 (arr2 dom2[2]);

> Another objection is the changes you made in execMain.c. That extra
> syscache lookup for every field of every tuple is going to be a
rather
> nasty performance hit, especially seeing that people will pay it
whether
> they ever heard of domains or not. And it seems quite unnecessary;
if
> you copy the domain's notnull bit into the pg_attribute row, then
the
> existing coding will do fine, no?

Did that originally with a function called MergeDomainAttributes()
(since removed). The goal was to allow the user to change table
attributes without overriding the domain. The constraints are
'merged' on execution for every other constraint type (default is
wacky though), so don't see why NOT NULL should be special. This
could be done with a new column pg_attribute.attypnotnull and have the
type null / not null data copied there. Of course, ALTER DOMAIN is
going to get rather complex searching for all of those areas.

That said, I rarely see a table without atleast one NOT NULL
constraint (primary key usually) except for perhaps a log, so is it
really a big performance hit? I thought I saved a couple of cycles by
dropping the IF :)

> I think also that you may have created some subtle changes in the
> semantics of type default-value specifications; we'll need to think
> if any compatibility problems have been introduced. There are
doubtless
> hardly any people using the feature, so this is not a serious
objection,
> but if any change has occurred it should be documented.

Where would I put the docs for this? CREATE TYPE ref notes? or is the
history released at release adequate for this?

"Rod Taylor" <rbt(at)zort(dot)ca> writes:
> Did that originally with a function called MergeDomainAttributes()
> (since removed). The goal was to allow the user to change table
> attributes without overriding the domain. The constraints are
> 'merged' on execution for every other constraint type (default is
> wacky though), so don't see why NOT NULL should be special.

It appeared to me that the intention was to merge at table creation
time, not at execution. I would certainly recommend doing it that
way for simplicity and performance reasons. (This does put ALTER DOMAIN
out of reach, but I feel no pain at not supporting that.)

regards, tom lane

Your tests look pretty good - maybe you should make them into a proper
regression test as well? Also, shouldn't there be some modification to
pg_dump to all DOMAINs to be dumped?

Chris

> -----Original Message-----
> From: pgsql-patches-owner(at)postgresql(dot)org
> [mailto:pgsql-patches-owner(at)postgresql(dot)org]On Behalf Of Rod Taylor
> Sent: Monday, 25 February 2002 7:35 AM
> To: pgsql-patches(at)postgresql(dot)org
> Subject: [PATCHES] Basic DOMAIN Support
>
>
> I intend to add other parts of domain support later on (no reason to
> hold up committing this though) but would appreciate some feedback
> about what I've done.
>
> What's there works, however I intend to finish it off with CHECK
> and -- if I can figure out a good way -- REFERENCES.
>
>
> Implements:
> CREATE DOMAIN domain type [NULL | NOT NULL] [DEFAULT expression];
> COMMENT ON DOMAIN domain IS '';
> DROP DOMAIN domain [RESTRICT | CASCADE]; -- Doesn't actually restrict
> due to pg_depends
>
> Affects:
> Types can be specified as NOT NULL. No interface is available to set
> this for any type other than a domain however. Types may also use a
> complex expression (b_expr) for their default.
>
> Various Tasks (output from psql for some simple operations involving
> domains):
>
> NOTE: For DEFAULT NULL to have any effect in table creation the
> default actually needs to be stored.
>
> Since Type defaults have overridden NULL in the past, I left it so
> domains would as well.
>
> Below are some tests I used to check the implementation.
>
> ## DOMAIN TEST ##
> create domain domainvarchar varchar(15);
> create domain domainnumeric numeric(8,2);
> create domain domainint4 int4;
> create domain domaintext text;
>
> -- Test tables using domains
> create table basictest
> ( testint4 domainint4
> , realint4 int4
> , testtext domaintext
> , realtext text
> , testvarchar domainvarchar
> , realvarchar varchar(15)
> , testnumeric domainnumeric
> , realnumeric numeric(8,2)
> );
>
> INSERT INTO basictest values ('88', '88', 'haha', 'haha', 'short
> text', 'short text', '123.12', '123.12');
> select * from basictest;
>
> create domain dnotnull varchar(15) NOT NULL;
> create domain dnull varchar(15) NULL;
>
> -- NOT NULL in the domain cannot be overridden
> create table nulltest
> ( col1 dnotnull
> , col2 dnotnull NULL
> , col3 dnull NOT NULL
> , col4 dnull
> );
> INSERT INTO nulltest DEFAULT VALUES;
> INSERT INTO nulltest values ('a', 'b', 'c', 'd'); -- Good
> INSERT INTO nulltest values (NULL, 'b', 'c', 'd');
> INSERT INTO nulltest values ('a', NULL, 'c', 'd');
> INSERT INTO nulltest values ('a', 'b', NULL, 'd');
> INSERT INTO nulltest values ('a', 'b', 'c', NULL); -- Good
> select * from nulltest;
>
>
> create domain ddef1 int4 DEFAULT 3;
> create domain ddef2 numeric(8,6) DEFAULT random();
> -- Type mixing, function returns int8
> create domain ddef3 text DEFAULT random();
> create sequence ddef4_seq;
> create domain ddef4 int4 DEFAULT nextval(cast('ddef4_seq' as text));
>
> create table defaulttest
> ( col1 ddef1
> , col2 ddef2
> , col3 ddef3
> , col4 ddef4
> , col5 ddef1 DEFAULT NULL
> , col6 ddef2 DEFAULT '88.1'
> , col7 ddef4 DEFAULT random() * 8000
> );
> insert into defaulttest default values;
> insert into defaulttest default values;
> insert into defaulttest default values;
> select * from defaulttest;
>
> ## PSQL OUTPUT ##
>
> newdb=# -- Test Comment / Drop
> newdb=# create domain domaindroptest int4;
> CREATE DOMAIN
> newdb=# comment on domain domaindroptest is 'About to drop this..';
> COMMENT
> newdb=#
> newdb=# select * from pg_type where typname = 'domaindroptest';
> typname | typowner | typlen | typprtlen | typbyval | typtype |
> typisdefined | typdelim | typrelid | typelem | typinput | typoutput |
> typrecei
> e | typsend | typalign | typstorage | typnotnull | typmod |
> typdefaultbin | typdefault
> ----------------+----------+--------+-----------+----------+---------+
> --------------+----------+----------+---------+----------+-----------+
> ---------
> --+---------+----------+------------+------------+--------+-----------
> ----+------------
> domaindroptest | 1 | 4 | 10 | t | d |
> t | , | 0 | 23 | int4in | int4out |
> int4in
> | int4out | i | p | f | -1 |
> |
> (1 row)
>
> newdb=#
> newdb=# drop domain domaindroptest restrict;
> DROP
> newdb=#
> newdb=# select * from pg_type where typname = 'domaindroptest';
> typname | typowner | typlen | typprtlen | typbyval | typtype |
> typisdefined | typdelim | typrelid | typelem | typinput | typoutput |
> typreceive | ty
> send | typalign | typstorage | typnotnull | typmod | typdefaultbin |
> typdefault
> ---------+----------+--------+-----------+----------+---------+-------
> -------+----------+----------+---------+----------+-----------+-------
> -----+---
> -----+----------+------------+------------+--------+---------------+--
> ----------
> (0 rows)
>
> newdb=# -- TEST Domains.
> newdb=#
> newdb=# create domain domainvarchar varchar(15);
> CREATE DOMAIN
> newdb=# create domain domainnumeric numeric(8,2);
> CREATE DOMAIN
> newdb=# create domain domainint4 int4;
> CREATE DOMAIN
> newdb=# create domain domaintext text;
> CREATE DOMAIN
> newdb=#
> newdb=# -- Test tables using domains
> newdb=# create table basictest
> newdb-# ( testint4 domainint4
> newdb(# , realint4 int4
> newdb(# , testtext domaintext
> newdb(# , realtext text
> newdb(# , testvarchar domainvarchar
> newdb(# , realvarchar varchar(15)
> newdb(# , testnumeric domainnumeric
> newdb(# , realnumeric numeric(8,2)
> newdb(# );
> CREATE
> newdb=#
> newdb=# INSERT INTO basictest values ('88', '88', 'haha', 'haha',
> 'short text', 'short text', '123.12', '123.12');
> INSERT 90400 1
> newdb=# select * from basictest;
> testint4 | realint4 | testtext | realtext | testvarchar | realvarchar
> | testnumeric | realnumeric
> ----------+----------+----------+----------+-------------+------------
> -+-------------+-------------
> 88 | 88 | haha | haha | short text | short text
> | 123.12 | 123.12
> (1 row)
>
> newdb=#
> newdb=# create domain dnotnull varchar(15) NOT NULL;
> CREATE DOMAIN
> newdb=# create domain dnull varchar(15) NULL;
> CREATE DOMAIN
> newdb=# -- NOT NULL in the domain cannot be overridden
> newdb=# create table nulltest
> newdb-# ( col1 dnotnull
> newdb(# , col2 dnotnull NULL
> newdb(# , col3 dnull NOT NULL
> newdb(# , col4 dnull
> newdb(# );
> CREATE
> newdb=# INSERT INTO nulltest DEFAULT VALUES;
> ERROR: ExecAppend: Fail to add null value in not null attribute col1
> newdb=# INSERT INTO nulltest values ('a', 'b', 'c', 'd'); -- Good
> INSERT 90408 1
> newdb-# INSERT INTO nulltest values (NULL, 'b', 'c', 'd');
> ERROR: ExecAppend: Fail to add null value in not null attribute col1
> newdb=# INSERT INTO nulltest values ('a', NULL, 'c', 'd');
> ERROR: ExecAppend: Fail to add null value in not null attribute col2
> newdb=# INSERT INTO nulltest values ('a', 'b', NULL, 'd');
> ERROR: ExecAppend: Fail to add null value in not null attribute col3
> newdb=# INSERT INTO nulltest values ('a', 'b', 'c', NULL); -- Good
> INSERT 90409 1
> newdb-# select * from nulltest;
> col1 | col2 | col3 | col4
> ------+------+------+------
> a | b | c | d
> a | b | c |
> (2 rows)
>
> newdb=# create domain ddef1 int4 DEFAULT 3;
> CREATE DOMAIN
> newdb=# create domain ddef2 numeric(8,6) DEFAULT random();
> CREATE DOMAIN
> newdb=# -- Type mixing, function returns int8
> newdb=# create domain ddef3 text DEFAULT random();
> CREATE DOMAIN
> newdb=# create sequence ddef4_seq;
> CREATE
> newdb=# create domain ddef4 int4 DEFAULT nextval(cast('ddef4_seq' as
> text));
> CREATE DOMAIN
> newdb=#
> newdb=# create table defaulttest
> newdb-# ( col1 ddef1
> newdb(# , col2 ddef2
> newdb(# , col3 ddef3
> newdb(# , col4 ddef4
> newdb(# , col5 ddef1 DEFAULT NULL
> newdb(# , col6 ddef2 DEFAULT '88.1'
> newdb(# , col7 ddef4 DEFAULT random() * 8000
> newdb(# );
> CREATE
> newdb=# insert into defaulttest default values;
> INSERT 90421 1
> newdb=# insert into defaulttest default values;
> INSERT 90422 1
> newdb=# insert into defaulttest default values;
> INSERT 90423 1
> newdb=# select * from defaulttest;
> col1 | col2 | col3 | col4 | col5 | col6 |
> col7
> ------+-------------------+-------------------+------+------+------+--
> ----
> 3 | 0.186453586065422 | 0.391880722433273 | 1 | 3 | 88.1 |
> 1930
> 3 | 0.999444424174467 | 0.461114872461704 | 2 | 3 | 88.1 |
> 6024
> 3 | 0.837450824602251 | 0.632604472633733 | 3 | 3 | 88.1 |
> 7441
> (3 rows)
>
>
> --
> Rod Taylor
>
> Your eyes are weary from staring at the CRT. You feel sleepy. Notice
> how restful it is to watch the cursor blink. Close your eyes. The
> opinions stated above are yours. You cannot imagine why you ever felt
> otherwise.
>
>

Yes, and yes -- although I completely forgot about pg_dump which would
have been amusing to see the comments of someone moving a db with
domains ;)

Can I assume it'll be safe for pgdump to output the domains near the
top?

I'm going to implement Toms requests -- take a long nap -- and see
about regressions and pg_dump tomorrow.

I love having sequence lookups in a domain used across several tables.
That one feature (in my case for a 'global' transaction id) has made
the whole thing worth while. 50 some tables, one serial. Typing
nextval all the time in CREATE TABLE was getting annoying.

BTW, Toronto is full of nuts. The hockey game ended many hours ago
and they're still running up and down the streets cheering. It's also
getting annoying.
--
Rod Taylor

This message represents the official view of the voices in my head

----- Original Message -----
From: "Christopher Kings-Lynne" <chriskl(at)familyhealth(dot)com(dot)au>
To: "Rod Taylor" <rbt(at)zort(dot)ca>; <pgsql-patches(at)postgresql(dot)org>
Sent: Sunday, February 24, 2002 10:32 PM
Subject: RE: [PATCHES] Basic DOMAIN Support

> Your tests look pretty good - maybe you should make them into a
proper
> regression test as well? Also, shouldn't there be some modification
to
> pg_dump to all DOMAINs to be dumped?
>
> Chris
>
>
> > -----Original Message-----
> > From: pgsql-patches-owner(at)postgresql(dot)org
> > [mailto:pgsql-patches-owner(at)postgresql(dot)org]On Behalf Of Rod Taylor
> > Sent: Monday, 25 February 2002 7:35 AM
> > To: pgsql-patches(at)postgresql(dot)org
> > Subject: [PATCHES] Basic DOMAIN Support
> >
> >
> > I intend to add other parts of domain support later on (no reason
to
> > hold up committing this though) but would appreciate some feedback
> > about what I've done.
> >
> > What's there works, however I intend to finish it off with CHECK
> > and -- if I can figure out a good way -- REFERENCES.

> Yes, and yes -- although I completely forgot about pg_dump which would
> have been amusing to see the comments of someone moving a db with
> domains ;)
>
> Can I assume it'll be safe for pgdump to output the domains near the
> top?

Does pg_dump dump CREATE TYPE's and CREATE OPERATOR's somewhere? Maybe it
should be near that?

> I'm going to implement Toms requests -- take a long nap -- and see
> about regressions and pg_dump tomorrow.

Chris

Ok. Updated patch attached.

- domain.patch -> source patch against pgsql in cvs
- drop_domain.sgml and create_domain.sgml -> New doc/src/sgml/ref docs
- dominfo.txt -> basic domain related queries I used for testing

Enables domains of array elements -> CREATE DOMAIN dom int4[3][2];

Uses a typbasetype column to describe the origin of the domain.

Copies data to attnotnull rather than processing in execMain().

Some documentation differences from earlier.

If this is approved, I'll start working on pg_dump, and a \dD <domain>
option in psql, and regression tests. I don't really feel like doing
those until the system table structure settles for pg_type.

CHECKS when added, will also be copied to to the table attributes. FK
Constraints (if I ever figure out how) will be done similarly. Both
will lbe handled by MergeDomainAttributes() which is called shortly
before MergeAttributes().

Any other recommendations?

--
Rod Taylor

This message represents the official view of the voices in my head

----- Original Message -----
From: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "Rod Taylor" <rbt(at)zort(dot)ca>
Cc: <pgsql-patches(at)postgresql(dot)org>
Sent: Sunday, February 24, 2002 8:11 PM
Subject: Re: [PATCHES] Basic DOMAIN Support

> "Rod Taylor" <rbt(at)zort(dot)ca> writes:
> > I intend to add other parts of domain support later on (no reason
to
> > hold up committing this though) but would appreciate some feedback
> > about what I've done.
>
> Still needs some work ...
>
> One serious problem is that I do not think you can get away with
reusing
> typelem to link domains to base types. All the array code is going
to
> think that a domain is an array, and proceed to do horribly wrong
> things. User applications may think the same thing, so even if you
> wanted to change every backend routine that looks at typelem, it
> wouldn't be enough. I think the only safe way to proceed is to add
a
> separate column that links a domain to its base type. This'd also
save
> you from having to add another meaning to typtype (since a domain
could
> be recognized by nonzero typbasetype). That should reduce the
> likelihood of breaking existing code, and perhaps make life simpler
when
> it comes time to allow freestanding composite types (why shouldn't a
> domain have a composite type as base?).
>
> Come to think of it, even without freestanding composite types it'd
be
> possible to try to define a domain as a subtype of a composite type,
> and to use same as (eg) a function argument or result type. I doubt
> you are anywhere near making that behave reasonably, though. Might
be
> best to disallow it for now.
>
> Speaking of arrays --- have you thought about arrays of domain-type
> objects? I'm not sure whether any of the supported features matter
for
> array elements, but if they do it's not clear how to make it happen.
>
> Another objection is the changes you made in execMain.c. That extra
> syscache lookup for every field of every tuple is going to be a
rather
> nasty performance hit, especially seeing that people will pay it
whether
> they ever heard of domains or not. And it seems quite unnecessary;
if
> you copy the domain's notnull bit into the pg_attribute row, then
the
> existing coding will do fine, no?
>
> I think also that you may have created some subtle changes in the
> semantics of type default-value specifications; we'll need to think
> if any compatibility problems have been introduced. There are
doubtless
> hardly any people using the feature, so this is not a serious
objection,
> but if any change has occurred it should be documented.
>
>
> Overall, an impressive first cut!
>
> regards, tom lane
>

Your patch has been added to the PostgreSQL unapplied patches list at:

http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

---------------------------------------------------------------------------

Rod Taylor wrote:
> Ok. Updated patch attached.
>
> - domain.patch -> source patch against pgsql in cvs
> - drop_domain.sgml and create_domain.sgml -> New doc/src/sgml/ref docs
> - dominfo.txt -> basic domain related queries I used for testing
>
> Enables domains of array elements -> CREATE DOMAIN dom int4[3][2];
>
> Uses a typbasetype column to describe the origin of the domain.
>
> Copies data to attnotnull rather than processing in execMain().
>
> Some documentation differences from earlier.
>
> If this is approved, I'll start working on pg_dump, and a \dD <domain>
> option in psql, and regression tests. I don't really feel like doing
> those until the system table structure settles for pg_type.
>
>
> CHECKS when added, will also be copied to to the table attributes. FK
> Constraints (if I ever figure out how) will be done similarly. Both
> will lbe handled by MergeDomainAttributes() which is called shortly
> before MergeAttributes().
>
>
> Any other recommendations?
>
> --
> Rod Taylor
>
> This message represents the official view of the voices in my head
>
> ----- Original Message -----
> From: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
> To: "Rod Taylor" <rbt(at)zort(dot)ca>
> Cc: <pgsql-patches(at)postgresql(dot)org>
> Sent: Sunday, February 24, 2002 8:11 PM
> Subject: Re: [PATCHES] Basic DOMAIN Support
>
>
> > "Rod Taylor" <rbt(at)zort(dot)ca> writes:
> > > I intend to add other parts of domain support later on (no reason
> to
> > > hold up committing this though) but would appreciate some feedback
> > > about what I've done.
> >
> > Still needs some work ...
> >
> > One serious problem is that I do not think you can get away with
> reusing
> > typelem to link domains to base types. All the array code is going
> to
> > think that a domain is an array, and proceed to do horribly wrong
> > things. User applications may think the same thing, so even if you
> > wanted to change every backend routine that looks at typelem, it
> > wouldn't be enough. I think the only safe way to proceed is to add
> a
> > separate column that links a domain to its base type. This'd also
> save
> > you from having to add another meaning to typtype (since a domain
> could
> > be recognized by nonzero typbasetype). That should reduce the
> > likelihood of breaking existing code, and perhaps make life simpler
> when
> > it comes time to allow freestanding composite types (why shouldn't a
> > domain have a composite type as base?).
> >
> > Come to think of it, even without freestanding composite types it'd
> be
> > possible to try to define a domain as a subtype of a composite type,
> > and to use same as (eg) a function argument or result type. I doubt
> > you are anywhere near making that behave reasonably, though. Might
> be
> > best to disallow it for now.
> >
> > Speaking of arrays --- have you thought about arrays of domain-type
> > objects? I'm not sure whether any of the supported features matter
> for
> > array elements, but if they do it's not clear how to make it happen.
> >
> > Another objection is the changes you made in execMain.c. That extra
> > syscache lookup for every field of every tuple is going to be a
> rather
> > nasty performance hit, especially seeing that people will pay it
> whether
> > they ever heard of domains or not. And it seems quite unnecessary;
> if
> > you copy the domain's notnull bit into the pg_attribute row, then
> the
> > existing coding will do fine, no?
> >
> > I think also that you may have created some subtle changes in the
> > semantics of type default-value specifications; we'll need to think
> > if any compatibility problems have been introduced. There are
> doubtless
> > hardly any people using the feature, so this is not a serious
> objection,
> > but if any change has occurred it should be documented.
> >
> >
> > Overall, an impressive first cut!
> >
> > regards, tom lane
> >

[ Attachment, skipping... ]

[ Attachment, skipping... ]

[ Attachment, skipping... ]

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

Patch applied. Thanks.

---------------------------------------------------------------------------

Rod Taylor wrote:
> Ok. Updated patch attached.
>
> - domain.patch -> source patch against pgsql in cvs
> - drop_domain.sgml and create_domain.sgml -> New doc/src/sgml/ref docs
> - dominfo.txt -> basic domain related queries I used for testing
>
> Enables domains of array elements -> CREATE DOMAIN dom int4[3][2];
>
> Uses a typbasetype column to describe the origin of the domain.
>
> Copies data to attnotnull rather than processing in execMain().
>
> Some documentation differences from earlier.
>
> If this is approved, I'll start working on pg_dump, and a \dD <domain>
> option in psql, and regression tests. I don't really feel like doing
> those until the system table structure settles for pg_type.
>
>
> CHECKS when added, will also be copied to to the table attributes. FK
> Constraints (if I ever figure out how) will be done similarly. Both
> will lbe handled by MergeDomainAttributes() which is called shortly
> before MergeAttributes().
>
>
> Any other recommendations?
>
> --
> Rod Taylor
>
> This message represents the official view of the voices in my head
>
> ----- Original Message -----
> From: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
> To: "Rod Taylor" <rbt(at)zort(dot)ca>
> Cc: <pgsql-patches(at)postgresql(dot)org>
> Sent: Sunday, February 24, 2002 8:11 PM
> Subject: Re: [PATCHES] Basic DOMAIN Support
>
>
> > "Rod Taylor" <rbt(at)zort(dot)ca> writes:
> > > I intend to add other parts of domain support later on (no reason
> to
> > > hold up committing this though) but would appreciate some feedback
> > > about what I've done.
> >
> > Still needs some work ...
> >
> > One serious problem is that I do not think you can get away with
> reusing
> > typelem to link domains to base types. All the array code is going
> to
> > think that a domain is an array, and proceed to do horribly wrong
> > things. User applications may think the same thing, so even if you
> > wanted to change every backend routine that looks at typelem, it
> > wouldn't be enough. I think the only safe way to proceed is to add
> a
> > separate column that links a domain to its base type. This'd also
> save
> > you from having to add another meaning to typtype (since a domain
> could
> > be recognized by nonzero typbasetype). That should reduce the
> > likelihood of breaking existing code, and perhaps make life simpler
> when
> > it comes time to allow freestanding composite types (why shouldn't a
> > domain have a composite type as base?).
> >
> > Come to think of it, even without freestanding composite types it'd
> be
> > possible to try to define a domain as a subtype of a composite type,
> > and to use same as (eg) a function argument or result type. I doubt
> > you are anywhere near making that behave reasonably, though. Might
> be
> > best to disallow it for now.
> >
> > Speaking of arrays --- have you thought about arrays of domain-type
> > objects? I'm not sure whether any of the supported features matter
> for
> > array elements, but if they do it's not clear how to make it happen.
> >
> > Another objection is the changes you made in execMain.c. That extra
> > syscache lookup for every field of every tuple is going to be a
> rather
> > nasty performance hit, especially seeing that people will pay it
> whether
> > they ever heard of domains or not. And it seems quite unnecessary;
> if
> > you copy the domain's notnull bit into the pg_attribute row, then
> the
> > existing coding will do fine, no?
> >
> > I think also that you may have created some subtle changes in the
> > semantics of type default-value specifications; we'll need to think
> > if any compatibility problems have been introduced. There are
> doubtless
> > hardly any people using the feature, so this is not a serious
> objection,
> > but if any change has occurred it should be documented.
> >
> >
> > Overall, an impressive first cut!
> >
> > regards, tom lane
> >

[ Attachment, skipping... ]

[ Attachment, skipping... ]

[ Attachment, skipping... ]

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026