Re: Basic DOMAIN Support - Mailing list pgsql-patches

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
>