Basic DOMAIN Support

"Rod Taylor" <rbt@zort.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@zort.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@zort.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

Show quoted text

-----Original Message-----
From: pgsql-patches-owner@postgresql.org
[mailto:pgsql-patches-owner@postgresql.org]On Behalf Of Rod Taylor
Sent: Monday, 25 February 2002 7:35 AM
To: pgsql-patches@postgresql.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@familyhealth.com.au>
To: "Rod Taylor" <rbt@zort.ca>; <pgsql-patches@postgresql.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@postgresql.org
[mailto:pgsql-patches-owner@postgresql.org]On Behalf Of Rod Taylor
Sent: Monday, 25 February 2002 7:35 AM
To: pgsql-patches@postgresql.org
Subject: [PATCHES] Basic DOMAIN Support

I intend to add other parts of domain support later on (no reason

to

Show quoted text

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@sss.pgh.pa.us>
To: "Rod Taylor" <rbt@zort.ca>
Cc: <pgsql-patches@postgresql.org>
Sent: Sunday, February 24, 2002 8:11 PM
Subject: Re: [PATCHES] Basic DOMAIN Support

"Rod Taylor" <rbt@zort.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,

Show quoted text

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@sss.pgh.pa.us>
To: "Rod Taylor" <rbt@zort.ca>
Cc: <pgsql-patches@postgresql.org>
Sent: Sunday, February 24, 2002 8:11 PM
Subject: Re: [PATCHES] Basic DOMAIN Support

"Rod Taylor" <rbt@zort.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@postgresql.org)

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.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@sss.pgh.pa.us>
To: "Rod Taylor" <rbt@zort.ca>
Cc: <pgsql-patches@postgresql.org>
Sent: Sunday, February 24, 2002 8:11 PM
Subject: Re: [PATCHES] Basic DOMAIN Support

"Rod Taylor" <rbt@zort.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@postgresql.org)

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

Rod, seems there were some problems with this patch, and I have backed
it out of CVS. The issues are that it did not compile cleanly against
current CVS, there were shift-reduce conflicts in the grammar changes,
there were compiler warnings that need fixing, and the patch caused the
regression tests to fail.

Would you please address these issues and submit a new patch. Thanks.

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

pgman wrote:

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@sss.pgh.pa.us>
To: "Rod Taylor" <rbt@zort.ca>
Cc: <pgsql-patches@postgresql.org>
Sent: Sunday, February 24, 2002 8:11 PM
Subject: Re: [PATCHES] Basic DOMAIN Support

"Rod Taylor" <rbt@zort.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@postgresql.org)

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

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

I've corrected the problem with shell type creation, so regression
tests work properly now (with the exception of NOTICE -> INFO /
WARNING changes). I have issues with timestamp tests, but the
failures match current cvs so I'll assume I've not contributed to
them.

The shift / reduce problem was fixed by simply removing the ability to
make types with complex defaults (reverted back to old simple
methods). Appears to still work. Storage is still mixed string /
binary.

I don't see any of the compile warnings other were receiving though.

Anyhow, new version attached.

Also attached is the regression sql set I used for domains. Yes, some
items are supposed to fail (generally marked or obvious).

Sorry about earlier. With any luck, this will do it. Umm.. It should
be mentioned this is the first time I've ever dealt with C, and more
specifically PostgreSQL internals, so tread lightly.
--
Rod Taylor

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

----- Original Message -----
From: "Bruce Momjian" <pgman@candle.pha.pa.us>
To: <pgman@candle.pha.pa.us>
Cc: "Rod Taylor" <rbt@zort.ca>; "Tom Lane" <tgl@sss.pgh.pa.us>;
<pgsql-patches@postgresql.org>
Sent: Thursday, March 07, 2002 1:11 PM
Subject: Re: [PATCHES] Basic DOMAIN Support

Rod, seems there were some problems with this patch, and I have

backed

it out of CVS. The issues are that it did not compile cleanly

against

current CVS, there were shift-reduce conflicts in the grammar

changes,

there were compiler warnings that need fixing, and the patch caused

the

regression tests to fail.

Would you please address these issues and submit a new patch.

Thanks.

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

-------

pgman wrote:

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@sss.pgh.pa.us>
To: "Rod Taylor" <rbt@zort.ca>
Cc: <pgsql-patches@postgresql.org>
Sent: Sunday, February 24, 2002 8:11 PM
Subject: Re: [PATCHES] Basic DOMAIN Support

"Rod Taylor" <rbt@zort.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@postgresql.org)

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

Pennsylvania 19026

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

19026

Show quoted text

Rod Taylor wrote:

I've corrected the problem with shell type creation, so regression
tests work properly now (with the exception of NOTICE -> INFO /
WARNING changes). I have issues with timestamp tests, but the
failures match current cvs so I'll assume I've not contributed to
them.

If you do a CVS update those regression problems should go away. Sorry
about that.

The shift / reduce problem was fixed by simply removing the ability to
make types with complex defaults (reverted back to old simple
methods). Appears to still work. Storage is still mixed string /
binary.

Great.

I don't see any of the compile warnings other were receiving though.

Let me test here and see. Will report back.

Also attached is the regression sql set I used for domains. Yes, some
items are supposed to fail (generally marked or obvious).

Good.

Sorry about earlier. With any luck, this will do it. Umm.. It should
be mentioned this is the first time I've ever dealt with C, and more
specifically PostgreSQL internals, so tread lightly.

If this is your first time dealing with C, I am amazed. It took me
years to get to the level of the patch you submitted.

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

On Thu, 2002-03-07 at 17:18, Rod Taylor wrote:

I've corrected the problem with shell type creation, so regression
tests work properly now (with the exception of NOTICE -> INFO /
WARNING changes). I have issues with timestamp tests, but the
failures match current cvs so I'll assume I've not contributed to
them.

All the regression tests (including the timestamp related ones) pass for
me with current CVS, without the patch applied. With the patch applied,
the "privileges" test fails.

The shift / reduce problem was fixed by simply removing the ability to
make types with complex defaults (reverted back to old simple
methods).

I still see a shift / reduce conflict in gram.y

I don't see any of the compile warnings other were receiving though.

make[3]: Entering directory `/home/nconway/pgsql/src/backend/catalog'
gcc-3.0 -O2 -Wall -Wmissing-prototypes -Wmissing-declarations
-I../../../src/include -c -o heap.o heap.c
heap.c: In function `cookDefault':
heap.c:1904: warning: implicit declaration of function `getBaseType'

make[3]: Entering directory `/home/nconway/pgsql/src/backend/parser'
gcc-3.0 -O2 -Wall -Wmissing-prototypes -Wmissing-declarations
-I../../../src/include -c -o analyze.o analyze.c
gcc-3.0 -O2 -Wall -Wmissing-prototypes -Wmissing-declarations
-I../../../src/include -c -o gram.o gram.c
gram.y: In function `yyparse':
gram.y:3228: warning: assignment from incompatible pointer type
gram.y:3232: warning: assignment from incompatible pointer type

gcc-3.0 -O2 -Wall -Wmissing-prototypes -Wmissing-declarations
-I../../../src/include -c -o parse_coerce.o parse_coerce.c
parse_coerce.c:637:1: warning: no newline at end of file

make[3]: Entering directory `/home/nconway/pgsql/src/backend/nodes'
gcc-3.0 -O2 -Wall -Wmissing-prototypes -Wmissing-declarations
-I../../../src/include -c -o copyfuncs.o copyfuncs.c
copyfuncs.c:2232: warning: `_copyCreateDomainStmt' defined but not used

make[4]: Entering directory
`/home/nconway/pgsql/src/backend/optimizer/prep'
gcc-3.0 -O2 -Wall -Wmissing-prototypes -Wmissing-declarations
-I../../../../src/include -c -o preptlist.o preptlist.c
preptlist.c: In function `build_column_default':
preptlist.c:358: warning: unused variable `typedefault'

Also:

src/backend/catalog/heap.c: spelling: "typically for domiains"

Cheers,

Neil

--
Neil Conway <neilconway@rogers.com>
PGP Key ID: DB3C29FC

Sorry for making this a painful process. pg_depend will be alot
better :) -- chalk it up to a learning experience I suppose.

Bruce: Docs and regress queries still apply.

Anyway, dug out a FreeBSD machine and tested this round on it. No
compile errors or warnings (not from my parts anyway), bison was quite
(see below), and regression tests ran perfectly.

My win2k cygwin environment still has a ton of regress errors
(postmaster crashes mostly)...

The shift / reduce problem was fixed by simply removing the

ability to

make types with complex defaults (reverted back to old simple
methods).

I still see a shift / reduce conflict in gram.y

The primary issue is that I want to do an " '=' c_expr " (through a
couple of steps) for the type default.

I've tried several things to get rid of it, but nothing works -- and I
want an expression (closer to SQL 99 Create Type). Without the '='
between DEFAULT and c_expr everything is fine -- of course that could
break applications that use CREATE TYPE.

Anyway, I copped out and added a %expect 1. I think this is close
enough to the infamous 'if / else' scenario with expressions that it's
warrented (the reason they came up with it).

Perhaps it could be opt_equal and drop the equal sign in 7.4 or
something to remove the conflict entirely?

For the errors, I just realized that if I redirect output to a log
file I'm left with only warnings and errors. Is there a website with
these kinds of tricks listed?

I don't see any of the compile warnings other were receiving

though.

make[3]: Entering directory

`/home/nconway/pgsql/src/backend/catalog'

gcc-3.0 -O2 -Wall -Wmissing-prototypes -Wmissing-declarations
-I../../../src/include -c -o heap.o heap.c
heap.c: In function `cookDefault':
heap.c:1904: warning: implicit declaration of function `getBaseType'

header included.

make[3]: Entering directory `/home/nconway/pgsql/src/backend/parser'
gcc-3.0 -O2 -Wall -Wmissing-prototypes -Wmissing-declarations
-I../../../src/include -c -o analyze.o analyze.c
gcc-3.0 -O2 -Wall -Wmissing-prototypes -Wmissing-declarations
-I../../../src/include -c -o gram.o gram.c
gram.y: In function `yyparse':
gram.y:3228: warning: assignment from incompatible pointer type
gram.y:3232: warning: assignment from incompatible pointer type

Your line numbers are slightly different but I believe they're CREATE
DATABASE WITH OWNER issues.

gcc-3.0 -O2 -Wall -Wmissing-prototypes -Wmissing-declarations
-I../../../src/include -c -o parse_coerce.o parse_coerce.c
parse_coerce.c:637:1: warning: no newline at end of file

Umm... Is this a gcc 3 thing? Either way, line added -- but I still
think thats a silly thing to complain about. I didn't realize
compilers cared about whitespace -- aside from Python anyway.

make[3]: Entering directory `/home/nconway/pgsql/src/backend/nodes'
gcc-3.0 -O2 -Wall -Wmissing-prototypes -Wmissing-declarations
-I../../../src/include -c -o copyfuncs.o copyfuncs.c
copyfuncs.c:2232: warning: `_copyCreateDomainStmt' defined but not

used

Commented out, but left in for potential use.

"Rod Taylor" <rbt@zort.ca> writes:

Anyway, I copped out and added a %expect 1.

Sorry, but we agreed long ago that %expect is *not* an acceptable
alternative to getting the bugs out of your yacc productions.

%expect is okay for stable grammars or ones that are being hacked by
just one or two closely-involved people. The PG grammar is being hit on
all the time by a lot of people with varying yacc skill, and some of the
rearrangements are not trivial. The problem with %expect for us is: all
right, we just ignored N shift/reduce errors --- but are they the same N
errors that someone once long ago decided were okay? It's too risky.

I believe your real problem has nothing to do with the c_expr anyway,
but is that DEFAULT is an allowable ColLabel. I do not recall which
keywords we currently need to accept as def_elem labels, but perhaps
it'd work to back off the lefthand side from ColLabel to ColId or
some such.

regards, tom lane

"Rod Taylor" <rbt@zort.ca> writes:

Anyway, I copped out and added a %expect 1.

Sorry, but we agreed long ago that %expect is *not* an acceptable
alternative to getting the bugs out of your yacc productions.

Just a quick questions. Every time I try to apply one of these patches
(patch < domain.patch) in the pgsql directory checked out from latest CVS, I
get all sorts of bad hunk errors and stuff. Do these matter? How do I
apply a patch? I just thought I'd do a 64bit compile of Rod's code to help
him find errors...

Chris

"Rod Taylor" <rbt@zort.ca> writes:

copyfuncs.c:2232: warning: `_copyCreateDomainStmt' defined but not

used

Commented out, but left in for potential use.

Wrong answer --- what this is reminding you is that you forgot to call
it.

Also, I'm fairly confused by the code in MergeDomainAttributes and
DefineDomain. You seem to have modeled this on table creation, not
type creation, which makes little or no sense to me. Where in the
heck does a domain "inherit" from?

regards, tom lane

"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:

Just a quick questions. Every time I try to apply one of these patches
(patch < domain.patch) in the pgsql directory checked out from latest CVS, I
get all sorts of bad hunk errors and stuff. Do these matter?

Probably.

How do I apply a patch?

I noticed that Rod's message was (rather badly) MIME-encoded. You may
have to undo the encoding to get a clean patch file to apply. Or maybe
it's something else; I've not tried to apply his patches yet, only
eyeballed them. But I imagine Bruce would've bounced the earlier patch
if he had difficulty applying it.

regards, tom lane

It's a cvs diff -c, rather than a straight diff -c against other
sources.

Cvs diff tosses on some additional information which patch complains
about, but it's ok to ignore.

And you can blame MS Outlook for any mime encoding issues of my
email.s
--
Rod Taylor

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

----- Original Message -----
From: "Tom Lane" <tgl@sss.pgh.pa.us>
To: "Christopher Kings-Lynne" <chriskl@familyhealth.com.au>
Cc: "Rod Taylor" <rbt@zort.ca>; "Neil Conway"
<nconway@klamath.dyndns.org>; "Bruce Momjian"
<pgman@candle.pha.pa.us>; "PostgreSQL Patches"
<pgsql-patches@postgresql.org>
Sent: Thursday, March 07, 2002 10:01 PM
Subject: Re: [PATCHES] Basic DOMAIN Support

"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:

Just a quick questions. Every time I try to apply one of these

patches

(patch < domain.patch) in the pgsql directory checked out from

latest CVS, I

get all sorts of bad hunk errors and stuff. Do these matter?

Probably.

How do I apply a patch?

I noticed that Rod's message was (rather badly) MIME-encoded. You

may

have to undo the encoding to get a clean patch file to apply. Or

maybe

it's something else; I've not tried to apply his patches yet, only
eyeballed them. But I imagine Bruce would've bounced the earlier

patch

Show quoted text

if he had difficulty applying it.

regards, tom lane