Re: Basic DOMAIN Support - Mailing list pgsql-patches

From Bruce Momjian
Subject Re: Basic DOMAIN Support
Date
Msg-id 200203071811.g27IBH115888@candle.pha.pa.us
Whole thread Raw
In response to Basic DOMAIN Support  ("Rod Taylor" <rbt@zort.ca>)
List pgsql-patches
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

pgsql-patches by date:

Previous
From: Fernando Nasser
Date:
Subject: Reorder definitions in parsenodes.h
Next
From: "Rod Taylor"
Date:
Subject: Re: Basic DOMAIN Support