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
Attachments:
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/viewAttached 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 19073Attachments:
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.
Show quoted text
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/viewAttached 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
Attachments:
create_replace_view-7.patchtext/x-patchDownload+445-296
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?
--
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 19073Neil 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 19073Neil 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