Thread: create or replace rule/view (fwd)
Re-sent. No one else seems to have received it. Bruce: copied you in to make sure. Gavin ---------- Forwarded message ---------- Date: Mon, 12 Aug 2002 03:22:08 +1000 (EST) From: Gavin Sherry <swm@linuxworld.com.au> To: pgsql-patches@postgresql.org Subject: create or replace rule/view Attached is a patch implementing: create or replace rule ... create or replace view ... It passes all regression tests. There's only one really sketchy part of the patch: UpdateAttributeTuples(). This routine is fairly dangerous since it simply removes a given relid's pg_attribute entries and creates a new set basic on a given TupleDesc. Naturally, it is only useful for views. It is important to have this functionality so that you can do something like: template1=# create view abc as select 'test'::text as a; CREATE VIEW template1=# select * from abc; a ------ test (1 row) template1=# create or replace view abc as select '1'::int2 as a; CREATE VIEW template1=# select * from abc; a --- 1 (1 row) Regardless, the patch may need some cleaning up. Unfortunately, I don't have enough time on my hands over the coming week. Considering beta is coming up I thought it best to submit and see what happens. Gavin
Attachment
Can someone address Gavin's concern about UpdateAttributeTuples() so we can get this patch into 7.3? I know a few people have been asking for it. --------------------------------------------------------------------------- Gavin Sherry wrote: > Re-sent. > > No one else seems to have received it. > > Bruce: copied you in to make sure. > > Gavin > > ---------- Forwarded message ---------- > Date: Mon, 12 Aug 2002 03:22:08 +1000 (EST) > From: Gavin Sherry <swm@linuxworld.com.au> > To: pgsql-patches@postgresql.org > Subject: create or replace rule/view > > Attached is a patch implementing: > > create or replace rule ... > create or replace view ... > > It passes all regression tests. There's only one really sketchy part of > the patch: UpdateAttributeTuples(). This routine is fairly dangerous since > it simply removes a given relid's pg_attribute entries and creates a new > set basic on a given TupleDesc. Naturally, it is only useful for views. > > It is important to have this functionality so that you can do something > like: > > template1=# create view abc as select 'test'::text as a; > CREATE VIEW > template1=# select * from abc; > a > ------ > test > (1 row) > > template1=# create or replace view abc as select '1'::int2 as a; > CREATE VIEW > template1=# select * from abc; > a > --- > 1 > (1 row) > > Regardless, the patch may need some cleaning up. Unfortunately, I don't > have enough time on my hands over the coming week. Considering beta is > coming up I thought it best to submit and see what happens. > > Gavin Content-Description: [ Attachment, skipping... ] -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
REPLACE VIEW shouldn't change the visible structure at all. It defeats the purpose. So, removing that entire portion and throwing an error when the user tries would be much more appropriate. The example shown would break nearly every function, subview, etc. that was using it at the time. On Tue, 2002-08-27 at 14:50, Bruce Momjian wrote: > > Can someone address Gavin's concern about UpdateAttributeTuples() so we > can get this patch into 7.3? I know a few people have been asking for > it. > > > --------------------------------------------------------------------------- > > Gavin Sherry wrote: > > Re-sent. > > > > No one else seems to have received it. > > > > Bruce: copied you in to make sure. > > > > Gavin > > > > ---------- Forwarded message ---------- > > Date: Mon, 12 Aug 2002 03:22:08 +1000 (EST) > > From: Gavin Sherry <swm@linuxworld.com.au> > > To: pgsql-patches@postgresql.org > > Subject: create or replace rule/view > > > > Attached is a patch implementing: > > > > create or replace rule ... > > create or replace view ... > > > > It passes all regression tests. There's only one really sketchy part of > > the patch: UpdateAttributeTuples(). This routine is fairly dangerous since > > it simply removes a given relid's pg_attribute entries and creates a new > > set basic on a given TupleDesc. Naturally, it is only useful for views. > > > > It is important to have this functionality so that you can do something > > like: > > > > template1=# create view abc as select 'test'::text as a; > > CREATE VIEW > > template1=# select * from abc; > > a > > ------ > > test > > (1 row) > > > > template1=# create or replace view abc as select '1'::int2 as a; > > CREATE VIEW > > template1=# select * from abc; > > a > > --- > > 1 > > (1 row) > > > > Regardless, the patch may need some cleaning up. Unfortunately, I don't > > have enough time on my hands over the coming week. Considering beta is > > coming up I thought it best to submit and see what happens. > > > > Gavin > > Content-Description: > > [ Attachment, skipping... ] > > -- > Bruce Momjian | http://candle.pha.pa.us > pgman@candle.pha.pa.us | (610) 359-1001 > + If your life is a hard drive, | 13 Roberts Road > + Christ can be your backup. | Newtown Square, Pennsylvania 19073 > ---- > > > ---------------------------(end of broadcast)--------------------------- > TIP 2: you can get off all lists at once with the unregister command > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
Rod Taylor <rbt@zort.ca> writes:
> REPLACE VIEW shouldn't change the visible structure at all. It defeats
> the purpose.
Exactly. The proposed patch does completely the wrong thing :-(
regards, tom lane
Rod Taylor <rbt@zort.ca> writes:
> REPLACE VIEW shouldn't change the visible structure at all. It defeats
> the purpose.
So what changes to the view should be allowed? Adding new attributes,
but not removing any or changing the properties of an attribute?
(e.g. data type).
In which case, the logic would be:
- construct TupleDesc based on ColumnDefs in view definition
- spin through the pg_attribute tuples that match the relid of
the view we're replacing
- if we find a pg_attribute tuple that is not in the
TupleDesc, bail out
- if we find a pg_attribute tuple that is in the
TupleDesc but has a different data-type (or other
properties), bail out
- if we find a TupleDesc attr that is not in
pg_attribute, add it to pg_atribute
Just want to make sure I understand Rod/Tom's objection before I
implement anything.
(BTW, Gavin asked me to finish off the patch for him -- I'd like to
see this in 7.3....)
Cheers,
Neil
--
Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC
On Wed, 2002-08-28 at 15:47, Neil Conway wrote: > Rod Taylor <rbt@zort.ca> writes: > > REPLACE VIEW shouldn't change the visible structure at all. It defeats > > the purpose. > > So what changes to the view should be allowed? Adding new attributes, > but not removing any or changing the properties of an attribute? > (e.g. data type). Same as with a table. I'm not even sure about adding attributes should be allowed. Consider another view that was a select * of the first. Since we have no way to find the original view query, we don't know whether it was select * or each individual column to cascade the add. Your logic looked good Perhaps a good chunk of the ALTER TABLE commands could be ported to views as well (drop column, etc.) to allow restrict / cascade to be appropriately specified.
Neil Conway <neilc@samurai.com> writes:
> Rod Taylor <rbt@zort.ca> writes:
>> REPLACE VIEW shouldn't change the visible structure at all. It defeats
>> the purpose.
> So what changes to the view should be allowed?
You can redefine the query that supports the view. You cannot add,
remove, rename, or change the datatypes of any columns of the view.
It occurs to me that we have similar problems if one does ALTER TABLE on
a table or composite type that is being used as a function argument or
result type. Possibly we could check for this by looking to see if
there are any dependencies recorded against the table's pg_type row.
regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > You can redefine the query that supports the view. You cannot add, > remove, rename, or change the datatypes of any columns of the view. I've attached a revision of Gavin's patch that implements this behavior (sorry it took so long, I've been busy with getting ready to move into university). I also did some review of Gavin's code: updated copyfuncs & equalfuncs, removed some superfluous function parameters, fixed some indentation, and write some regression tests. It probably still needs some review, however. Cheers, Neil -- Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC
Attachment
Neil Conway <neilc@samurai.com> writes: > I've attached a revision of Gavin's patch that implements this > behavior What's the status of this patch? i.e. Is it likely to be applied before the beta? Cheers, Neil -- Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC
Your patch has been added to the PostgreSQL unapplied patches list at:
http://207.106.42.251/cgi-bin/pgpatches
I will try to apply it within the next 48 hours.
---------------------------------------------------------------------------
Neil Conway wrote:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
> > You can redefine the query that supports the view. You cannot add,
> > remove, rename, or change the datatypes of any columns of the view.
>
> I've attached a revision of Gavin's patch that implements this
> behavior (sorry it took so long, I've been busy with getting ready to
> move into university). I also did some review of Gavin's code: updated
> copyfuncs & equalfuncs, removed some superfluous function parameters,
> fixed some indentation, and write some regression tests.
>
> It probably still needs some review, however.
>
> Cheers,
>
> Neil
>
> --
> Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://archives.postgresql.org
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Neil Conway <neilc@samurai.com> writes:
> What's the status of this patch? i.e. Is it likely to be applied
> before the beta?
If I can get it reviewed tonight then I'll put it in, otherwise it's
probably not going in. Right now I'm in the midst of reworking the
HeapTupleHeader changes, which turn out to contain a number of bad
ideas :-( ... wish I'd gotten around to reviewing them before.
regards, tom lane
OK, let me know what to do. I have brought up the issue of freeze time and how much work we are going to do post-freeze before beta1 packaging. Let's see what answer we get on that. --------------------------------------------------------------------------- Tom Lane wrote: > Neil Conway <neilc@samurai.com> writes: > > What's the status of this patch? i.e. Is it likely to be applied > > before the beta? > > If I can get it reviewed tonight then I'll put it in, otherwise it's > probably not going in. Right now I'm in the midst of reworking the > HeapTupleHeader changes, which turn out to contain a number of bad > ideas :-( ... wish I'd gotten around to reviewing them before. > > regards, tom lane > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Neil Conway <neilc@samurai.com> writes:
> What's the status of this patch? i.e. Is it likely to be applied
> before the beta?
It's committed with revisions (view.c was doing it the very hard way,
and was still missing a bunch of tests...)
I note it still requires documentation updates; would someone provide
those? I'm beat and am going out to see if I can find something to eat
...
regards, tom lane