Re: 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(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

--
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