disallow LOCK on a view

Started by Mark Hollomonover 25 years ago40 messages
#1Mark Hollomon
mhh@mindspring.com
1 attachment(s)

This patch is for the TODO item

* Disallow LOCK on view

src/backend/commands/command.c is the only affected file

--
Mark Hollomon
mhh@mindspring.com

Attachments:

lock.patchtext/plain; charset=us-asciiDownload
*** command.c.orig	Tue Aug 29 10:39:42 2000
--- command.c	Tue Aug 29 10:56:21 2000
***************
*** 54,59 ****
--- 54,62 ----
  
  
  static bool needs_toast_table(Relation rel);
+ static bool is_view(char *relname, char *command);
+ 
+ 
  
  
  /* --------------------------------
***************
*** 1086,1094 ****
  AlterTableAddConstraint(char *relationName,
  						bool inh, Node *newConstraint)
  {
- 	char rulequery[41+NAMEDATALEN]; 
- 	void *qplan;
- 	char nulls[1]="";
  
  	if (newConstraint == NULL)
  		elog(ERROR, "ALTER TABLE / ADD CONSTRAINT passed invalid constraint.");
--- 1089,1094 ----
***************
*** 1099,1117 ****
  #endif
  
  	/* check to see if the table to be constrained is a view. */
! 	sprintf(rulequery, "select * from pg_views where viewname='%s'", relationName);
! 	if (SPI_connect()!=SPI_OK_CONNECT)
! 		elog(ERROR, "ALTER TABLE: Unable to determine if %s is a view - SPI_connect failure..", relationName);
!         qplan=SPI_prepare(rulequery, 0, NULL);
! 	if (!qplan)
! 		elog(ERROR, "ALTER TABLE: Unable to determine if %s is a view - SPI_prepare failure.", relationName);
! 	qplan=SPI_saveplan(qplan);
! 	if (SPI_execp(qplan, NULL, nulls, 1)!=SPI_OK_SELECT) 
! 		elog(ERROR, "ALTER TABLE: Unable to determine if %s is a view - SPI_execp failure.", relationName);
!         if (SPI_processed != 0)
!                 elog(ERROR, "ALTER TABLE: Cannot add constraints to views.");
!         if (SPI_finish() != SPI_OK_FINISH)
!                 elog(NOTICE, "SPI_finish() failed in ALTER TABLE");
  		
  	switch (nodeTag(newConstraint))
  	{
--- 1099,1106 ----
  #endif
  
  	/* check to see if the table to be constrained is a view. */
! 	if (is_view(relationName, "ALTER TABLE"))
! 		 elog(ERROR, "ALTER TABLE: Cannot add constraints to views.");
  		
  	switch (nodeTag(newConstraint))
  	{
***************
*** 1254,1272 ****
  				}
  
  				/* check to see if the referenced table is a view. */
! 				sprintf(rulequery, "select * from pg_views where viewname='%s'", fkconstraint->pktable_name);
! 				if (SPI_connect()!=SPI_OK_CONNECT)
! 					elog(ERROR, "ALTER TABLE: Unable to determine if %s is a view.", relationName);
! 			        qplan=SPI_prepare(rulequery, 0, NULL);
! 				if (!qplan)
! 					elog(ERROR, "ALTER TABLE: Unable to determine if %s is a view.", relationName);
! 				qplan=SPI_saveplan(qplan);
! 				if (SPI_execp(qplan, NULL, nulls, 1)!=SPI_OK_SELECT) 
! 					elog(ERROR, "ALTER TABLE: Unable to determine if %s is a view.", relationName);
! 			        if (SPI_processed != 0)
! 			                elog(ERROR, "ALTER TABLE: Cannot add constraints to views.");
! 			        if (SPI_finish() != SPI_OK_FINISH)
! 			                elog(NOTICE, "SPI_finish() failed in RI_FKey_check()");
  
  				/*
  				 * Grab an exclusive lock on the pk table, so that someone
--- 1243,1250 ----
  				}
  
  				/* check to see if the referenced table is a view. */
! 				if (is_view(fkconstraint->pktable_name, "ALTER TABLE"))
! 				 elog(ERROR, "ALTER TABLE: Cannot add constraints to views.");
  
  				/*
  				 * Grab an exclusive lock on the pk table, so that someone
***************
*** 1635,1640 ****
--- 1613,1621 ----
  	Relation	rel;
  	int			aclresult;
  
+ 	if (is_view(lockstmt->relname, "LOCK TABLE")) 
+ 			elog(ERROR, "LOCK TABLE: cannot lock a view");
+ 
  	rel = heap_openr(lockstmt->relname, NoLock);
  
  	if (lockstmt->mode == AccessShareLock)
***************
*** 1650,1652 ****
--- 1631,1659 ----
  	heap_close(rel, NoLock);	/* close rel, keep lock */
  }
  
+ 
+ static
+ bool
+ is_view (char * relname, char *command)
+ {
+ 	bool retval;
+ 	char rulequery[41+NAMEDATALEN]; 
+ 	void *qplan;
+ 	char nulls[1]="";
+ 
+ 	sprintf(rulequery, "select * from pg_views where viewname='%s'", relname);
+ 	if (SPI_connect()!=SPI_OK_CONNECT)
+ 		elog(ERROR, "%s: Unable to determine if %s is a view - SPI_connect failure..", command, relname);
+ 	qplan=SPI_prepare(rulequery, 0, NULL);
+ 	if (!qplan)
+ 		elog(ERROR, "%s: Unable to determine if %s is a view - SPI_prepare failure.", command, relname);
+ 	qplan=SPI_saveplan(qplan);
+ 	if (SPI_execp(qplan, NULL, nulls, 1)!=SPI_OK_SELECT) 
+ 		elog(ERROR, "%s: Unable to determine if %s is a view - SPI_execp failure.", command, relname);
+ 
+ 	retval = (SPI_processed != 0);
+   if (SPI_finish() != SPI_OK_FINISH)
+ 	 elog(NOTICE, "SPI_finish() failed in %s", command);
+ 
+   return retval;
+ }
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Mark Hollomon (#1)
Backend-internal SPI operations

Mark Hollomon <mhh@mindspring.com> writes:

sprintf(rulequery, "select * from pg_views where viewname='%s'", relname);
[ evaluate query via SPI ]

I really dislike seeing backend utility operations built atop SPI.
Quite aside from the (lack of) speed, there are all sorts of nasty
traps that can come from runtime evaluation of query strings. The
most obvious example in this case is what if relname contains a quote
mark? Or backslash?

The permanent memory leak induced by SPI_saveplan() is another good
reason not to do it this way.

Finally, once one has written a nice neat little is_view() query
function, there's a strong temptation to just use it from anywhere,
without thought for the side-effects it might have like grabbing/
releasing locks, CommandCounterIncrement(), etc. There are many
places in the backend where the side-effects of doing a full query
evaluation would be harmful.

Mark's patch is OK as is, since it's merely relocating some poorly
written code and not trying to fix it, but someone ought to think
about fixing the code.

regards, tom lane

#3Mark Hollomon
mhh@nortelnetworks.com
In reply to: Mark Hollomon (#1)
Re: Backend-internal SPI operations

Tom Lane wrote:

Mark's patch is OK as is, since it's merely relocating some poorly
written code and not trying to fix it, but someone ought to think
about fixing the code.

I'll take a crack at it.

Just out of curiousity, is there technical reason there isn't
a (say) relisview attribute to pg_class?

--

Mark Hollomon
mhh@nortelnetworks.com
ESN 451-9008 (302)454-9008

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Mark Hollomon (#3)
Re: Backend-internal SPI operations

"Mark Hollomon" <mhh@nortelnetworks.com> writes:

Just out of curiousity, is there technical reason there isn't
a (say) relisview attribute to pg_class?

That might indeed be the most reasonable way to attack it, rather
than having to go messing about looking for a matching rule.
(Jan, any thoughts here?)

Adding a column to a core system table like pg_class is a good
exercise for the student ;-) ... it's not exactly automated,
and you have to find all the places that need to be updated.
You might want to keep notes and prepare a writeup for the
developer's FAQ. I thought of that the last time I did something
similar, but it was only at the end that I realized I should've
been keeping notes to start with.

regards, tom lane

#5Jan Wieck
janwieck@Yahoo.com
In reply to: Tom Lane (#4)
Re: Backend-internal SPI operations

Tom Lane wrote:

"Mark Hollomon" <mhh@nortelnetworks.com> writes:

Just out of curiousity, is there technical reason there isn't
a (say) relisview attribute to pg_class?

That might indeed be the most reasonable way to attack it, rather
than having to go messing about looking for a matching rule.
(Jan, any thoughts here?)

The right way IMHO would be to give views another relkind.
Then we could easily

1. detect if the final query after rewriting still tries to
INSERT/UPDATE/DELETE a view - i.e. "missing rewrite
rule(s)".

2. disable things like LOCK etc.

The problem here is, that the relkind must change at rule
creation/drop time. Fortunately rules on SELECT are totally
restricted to VIEW's since 6.4, and I don't see any reason to
change this.

And it's time to make more use of the relkind attribute. For
7.2, when we want to have tuple-set returns for functions, we
might want to have structures as well (we talked about that
already, Tom). A structure is just a row/type description. A
function, returning a tuple or set of tuples, can return this
type or set of type as well as any other existing table/view
structure. So to create a function returning a set of tuples,
which have a structure different from any existing table,
someone creates a named structure, then the function
returning tuples of that type. These structures are just
entries in pg_class, pg_attribute and pg_type. There is no
file or any rules, triggers etc. attached to them. They just
describe a typle that can be built in memory.

Adding a column to a core system table like pg_class is a good
exercise for the student ;-) ... it's not exactly automated,
and you have to find all the places that need to be updated.
You might want to keep notes and prepare a writeup for the
developer's FAQ. I thought of that the last time I did something
similar, but it was only at the end that I realized I should've
been keeping notes to start with.

Meetoo :-}

Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck@Yahoo.com #

#6Mark Hollomon
mhh@nortelnetworks.com
In reply to: Jan Wieck (#5)
Re: Backend-internal SPI operations

Jan Wieck wrote:

Tom Lane wrote:

"Mark Hollomon" <mhh@nortelnetworks.com> writes:

Just out of curiousity, is there technical reason there isn't
a (say) relisview attribute to pg_class?

That might indeed be the most reasonable way to attack it, rather
than having to go messing about looking for a matching rule.
(Jan, any thoughts here?)

The right way IMHO would be to give views another relkind.
Then we could easily

1. detect if the final query after rewriting still tries to
INSERT/UPDATE/DELETE a view - i.e. "missing rewrite
rule(s)".

This appeals to me. The current silent no-op behavior of INSERT/DELETE on a view
is annoying.

--

Mark Hollomon
mhh@nortelnetworks.com
ESN 451-9008 (302)454-9008

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Mark Hollomon (#6)
Re: Backend-internal SPI operations

The right way IMHO would be to give views another relkind.

This appeals to me.

I like it too. Aside from the advantages Jan mentioned, we could also
refrain from creating an underlying file for a view, which would be
nice to avoid cluttering the database directory.

regards, tom lane

#8Mark Hollomon
mhh@nortelnetworks.com
In reply to: Jan Wieck (#5)
Re: Backend-internal SPI operations

Tom Lane wrote:

The right way IMHO would be to give views another relkind.

This appeals to me.

I like it too. Aside from the advantages Jan mentioned, we could also
refrain from creating an underlying file for a view, which would be
nice to avoid cluttering the database directory.

Excellent. I think we have a consensus. I'll start coding in that direction.

Anybody have any thoughts on the upgrade ramification of this change?

--

Mark Hollomon
mhh@nortelnetworks.com
ESN 451-9008 (302)454-9008

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Mark Hollomon (#8)
Re: Backend-internal SPI operations

Jan Wieck <janwieck@Yahoo.com> writes:

From memory I think views are created as CREATE TABLE, with
an internal DefineRuleStmt, and dumped as CREATE TABLE,
CREATE RULE for sure. So the CREATE/DROP RULE would need to
remove/recreate the tables file (plus toast file and index)
if you want it to be consistent. Don't think you want that -
do you?

But that's only true because it's such a pain in the neck for pg_dump
to discover that a table is a view. If this could easily be told from
inspection of pg_class, then it'd be no problem to dump views as
CREATE VIEW statements in the first place --- obviously better, no?

However the initial version upgrade would be a problem, since dump
files out of existing releases would contain CREATE TABLE & RULE
commands instead of CREATE VIEW. I guess what would happen is that
views reloaded that way wouldn't really be views, they'd be tables
with rules attached. Grumble.

How about this:
CREATE RULE of an on-select-instead rule changes table's
relkind to 'view'. We don't need to drop the underlying
table file, though (just leave it be, it'll go away at
next initdb).

DROP RULE of a view's on-select-instead is not allowed.
You have to drop the whole view instead.

regards, tom lane

#10Jan Wieck
janwieck@Yahoo.com
In reply to: Tom Lane (#7)
Re: Backend-internal SPI operations

Tom Lane wrote:

The right way IMHO would be to give views another relkind.

This appeals to me.

I like it too. Aside from the advantages Jan mentioned, we could also
refrain from creating an underlying file for a view, which would be
nice to avoid cluttering the database directory.

From memory I think views are created as CREATE TABLE, with
an internal DefineRuleStmt, and dumped as CREATE TABLE,
CREATE RULE for sure. So the CREATE/DROP RULE would need to
remove/recreate the tables file (plus toast file and index)
if you want it to be consistent. Don't think you want that -
do you?

Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck@Yahoo.com #

#11Mike Mascari
mascarm@mascari.com
In reply to: Jan Wieck (#10)
Re: Backend-internal SPI operations

Tom Lane wrote:

Jan Wieck <janwieck@Yahoo.com> writes:

From memory I think views are created as CREATE TABLE, with
an internal DefineRuleStmt, and dumped as CREATE TABLE,
CREATE RULE for sure. So the CREATE/DROP RULE would need to
remove/recreate the tables file (plus toast file and index)
if you want it to be consistent. Don't think you want that -
do you?

But that's only true because it's such a pain in the neck for pg_dump
to discover that a table is a view. If this could easily be told from
inspection of pg_class, then it'd be no problem to dump views as
CREATE VIEW statements in the first place --- obviously better, no?

The fact that views can be created by a separate table/rule
sequence allows pg_dump to properly dump views which are based
upon functions, or views which may have dependencies on other
tables/views. The new pg_dump dumps in oid order in an attempt to
resolve 95% of the dependency problems, but it could never solve
a circular dependency. I was thinking that with:

(a) The creation of an ALTER FUNCTION name(args) SET ...

and

(b) Allow for functions to be created like:

CREATE FUNCTION foo(int) RETURNS int AS NULL;

which would return NULL as a result.

A complex schema with views based upon functions, tables, and
other views, and functions based upon views could be properly
dumped by dumping:

1. Function Prototypes (CREATE FUNCTION ... AS NULL)
2. Types
3. Aggregates
4. Operators
5. Sequences
6. Tables

...DATA...

7. Triggers
8. Function Implementations (ALTER FUNCTION ... SET)
9. Rules (including Views)
10. Indexes
11. Comments :-)

Wouldn't this be a "correct" dump?

Mike Mascari

#12Mike Mascari
mascarm@mascari.com
In reply to: Jan Wieck (#10)
Re: Backend-internal SPI operations

Some idiot wrote:

The fact that views can be created by a separate table/rule
sequence allows pg_dump to properly dump views which are based
upon functions, or views which may have dependencies on other
tables/views. The new pg_dump dumps in oid order in an attempt to
resolve 95% of the dependency problems, but it could never solve
a circular dependency. I was thinking that with:

(a) The creation of an ALTER FUNCTION name(args) SET ...

and

(b) Allow for functions to be created like:

CREATE FUNCTION foo(int) RETURNS int AS NULL;

which would return NULL as a result.

A complex schema with views based upon functions, tables, and
other views, and functions based upon views could be properly
dumped by dumping:

1. Function Prototypes (CREATE FUNCTION ... AS NULL)
2. Types
3. Aggregates
4. Operators
5. Sequences
6. Tables

...more idiocy follows...

Sorry. I forgot about function prototypes with arguments of
user-defined types. Seems there's no magic bullet. :-(

Mike Mascari

#13Mark Hollomon
mhh@nortelnetworks.com
In reply to: Jan Wieck (#10)
Re: Backend-internal SPI operations

Mike Mascari wrote:

Tom Lane wrote:

Jan Wieck <janwieck@Yahoo.com> writes:

From memory I think views are created as CREATE TABLE, with
an internal DefineRuleStmt, and dumped as CREATE TABLE,
CREATE RULE for sure. So the CREATE/DROP RULE would need to
remove/recreate the tables file (plus toast file and index)
if you want it to be consistent. Don't think you want that -
do you?

But that's only true because it's such a pain in the neck for pg_dump
to discover that a table is a view. If this could easily be told from
inspection of pg_class, then it'd be no problem to dump views as
CREATE VIEW statements in the first place --- obviously better, no?

The fact that views can be created by a separate table/rule
sequence allows pg_dump to properly dump views which are based
upon functions, or views which may have dependencies on other
tables/views.

I don't see this. a 'CREATE VIEW' cannot reference a function that
did not exist at the time it was executed. The only way to get in
trouble, that I see, is a DROP/CREATE RULE. But I think
the proposal is not to allow this to happen if the rule is the
select rule for a view.

The reason that pg_dump used the table/rule sequence was that historically
it was hard to figure out that a tuple in pg_class really represented a
view.

But I could be mistaken.

--

Mark Hollomon
mhh@nortelnetworks.com
ESN 451-9008 (302)454-9008

#14Mark Hollomon
mhh@nortelnetworks.com
In reply to: Jan Wieck (#5)
Re: New relkind for views

"Hollomon, Mark" wrote:

Do we still want to be able to inherit from views?

Also:

Currently a view may be dropped with either 'DROP VIEW'
or 'DROP TABLE'. Should this be changed?

--

Mark Hollomon
mhh@nortelnetworks.com
ESN 451-9008 (302)454-9008

#15Tom Lane
tgl@sss.pgh.pa.us
In reply to: Mike Mascari (#12)
Re: Backend-internal SPI operations

Mike Mascari <mascarm@mascari.com> writes:

1. Function Prototypes (CREATE FUNCTION ... AS NULL)
2. Types

Sorry. I forgot about function prototypes with arguments of
user-defined types. Seems there's no magic bullet. :-(

Not necessarily --- there's a shell-type (or type forward reference,
if you prefer) feature that exists to handle exactly that apparent
circularity. Otherwise you could never define a user-defined type at
all, since you have to define its I/O procedures before you can do
CREATE TYPE.

I didn't study your proposal in detail, but it might work.

regards, tom lane

#16Jan Wieck
janwieck@Yahoo.com
In reply to: Mark Hollomon (#13)
Re: Backend-internal SPI operations

Mark Hollomon wrote:

Mike Mascari wrote:

Tom Lane wrote:

Jan Wieck <janwieck@Yahoo.com> writes:

From memory I think views are created as CREATE TABLE, with
an internal DefineRuleStmt, and dumped as CREATE TABLE,
CREATE RULE for sure. So the CREATE/DROP RULE would need to
remove/recreate the tables file (plus toast file and index)
if you want it to be consistent. Don't think you want that -
do you?

But that's only true because it's such a pain in the neck for pg_dump
to discover that a table is a view. If this could easily be told from
inspection of pg_class, then it'd be no problem to dump views as
CREATE VIEW statements in the first place --- obviously better, no?

The fact that views can be created by a separate table/rule
sequence allows pg_dump to properly dump views which are based
upon functions, or views which may have dependencies on other
tables/views.

I don't see this. a 'CREATE VIEW' cannot reference a function that
did not exist at the time it was executed. The only way to get in
trouble, that I see, is a DROP/CREATE RULE. But I think
the proposal is not to allow this to happen if the rule is the
select rule for a view.

The reason that pg_dump used the table/rule sequence was that historically
it was hard to figure out that a tuple in pg_class really represented a
view.

But I could be mistaken.

Yep, you are.

The reason why we dump views as table+rule is that
historically we wheren't able to dump views and rules at all.
We only store the parsetree representation of rules, since
epoch. Then, someone wrote a little backend function that's
able to backparse these rule actions. It got enhanced by a
couple of other smart guys and got used by pg_dump. At that
time, it was right to dump views as table+rule, because
pg_dump didn't do anything in OID order. So views using sql
functions using views in turn wouldn't be dumpable otherwise.
And it was easier too because it was already done after
dumping rules at the end. No need to do anything else for
views :-)

So far about history, now the future.

Dumping views as CREATE VIEW is cleaner. It is possible now,
since we dump the objects in OID order. So I like it. I see
no problem with Tom's solution, changing the relkind and
removing the files at CREATE RULE time for a couple of
releases. And yes, dropping the SELECT rule from a view must
be forbidden. As defining triggers, constraints and the like
for them should be.

Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck@Yahoo.com #

#17Mark Hollomon
mhh@nortelnetworks.com
In reply to: Jan Wieck (#16)
Re: Backend-internal SPI operations

Jan Wieck wrote:

Mark Hollomon wrote:

But I could be mistaken.

Yep, you are.

D'oh.

So far about history, now the future.

Dumping views as CREATE VIEW is cleaner. It is possible now,
since we dump the objects in OID order. So I like it. I see
no problem with Tom's solution, changing the relkind and
removing the files at CREATE RULE time for a couple of
releases. And yes, dropping the SELECT rule from a view must
be forbidden. As defining triggers, constraints and the like
for them should be.

Alright. To recap.

1. CREATE VIEW sets relkind to RELKIND_VIEW
2. CREATE RULE ... AS ON SELECT DO INSTEAD ... sets relkind to RELKIND_VIEW
and deletes any relation files.

q: If we find an index, should we drop it, or complain, or ignore it?
q: Should the code check to see if the relation is empty (no valid tuples)?

3. DROP RULE complains if dropping the select rule for a view.
4. ALTER TABLE complains if run against a view.

Anything else?

--

Mark Hollomon
mhh@nortelnetworks.com
ESN 451-9008 (302)454-9008

#18Tom Lane
tgl@sss.pgh.pa.us
In reply to: Mark Hollomon (#17)
Re: Backend-internal SPI operations

"Mark Hollomon" <mhh@nortelnetworks.com> writes:

2. CREATE RULE ... AS ON SELECT DO INSTEAD ... sets relkind to RELKIND_VIEW
and deletes any relation files.

q: If we find an index, should we drop it, or complain, or ignore it?
q: Should the code check to see if the relation is empty (no valid tuples)?

I think we can ignore indexes. However, it seems like a wise move to
refuse to convert a nonempty table to view status, *especially* if we
are going to blow away the physical file. Otherwise mistyping the
relation name in a CREATE RULE could be disastrous (what? you wanted
that data?)

regards, tom lane

#19Zeugswetter Andreas SB
ZeugswetterA@wien.spardat.at
In reply to: Tom Lane (#18)
AW: Backend-internal SPI operations

Tom Lane wrote:

Mark's patch is OK as is, since it's merely relocating some poorly
written code and not trying to fix it, but someone ought to think
about fixing the code.

I'll take a crack at it.

Just out of curiousity, is there technical reason there isn't
a (say) relisview attribute to pg_class?

I have been arguing ages for a different relkind for real views
that are created with "create view ...". This would obliviate the
actual file that is currently needed for each view too.

Imho that would be a better solution than an additional flag.

Andreas

#20Zeugswetter Andreas SB
ZeugswetterA@wien.spardat.at
In reply to: Zeugswetter Andreas SB (#19)
AW: Backend-internal SPI operations

The problem here is, that the relkind must change at rule
creation/drop time. Fortunately rules on SELECT are totally
restricted to VIEW's since 6.4, and I don't see any reason to
change this.

I don't see why a real view should still be createable by the old
create table then create rule way. Then the relkind would never
need to change. The only place that would need correction is
pg_dump to dump create view statements.

Imho we should not limit select rules to views by design.
With a different relkind for views this is not necessary anyway.

Andreas

#21Tom Lane
tgl@sss.pgh.pa.us
In reply to: Zeugswetter Andreas SB (#20)
Re: AW: Backend-internal SPI operations

Zeugswetter Andreas SB <ZeugswetterA@wien.spardat.at> writes:

I don't see why a real view should still be createable by the old
create table then create rule way.

Because we'll need to be able to read dump files created by existing
versions of pg_dump. If we don't cope with CREATE TABLE + CREATE RULE
then restored-from-dump views won't really be views and will act
differently from freshly created views. Avoiding the resulting
support headaches is worth a little bit of ugliness in the code.

regards, tom lane

#22Ross J. Reedstrom
reedstrm@rice.edu
In reply to: Tom Lane (#21)
Re: AW: Backend-internal SPI operations

On Thu, Aug 31, 2000 at 10:19:36AM -0400, Tom Lane wrote:

Zeugswetter Andreas SB <ZeugswetterA@wien.spardat.at> writes:

I don't see why a real view should still be createable by the old
create table then create rule way.

Because we'll need to be able to read dump files created by existing
versions of pg_dump. If we don't cope with CREATE TABLE + CREATE RULE
then restored-from-dump views won't really be views and will act
differently from freshly created views. Avoiding the resulting
support headaches is worth a little bit of ugliness in the code.

So, this'd be a one release only sort of hack, for compatability? It'd
be born deprecated? Hmm, is someone keeping track of all the features
that have been deprecated, so they can be stripped out later?

Also, sounds like something for Lamar's future pg_upgrade rewrite.

Ross
--
Ross J. Reedstrom, Ph.D., <reedstrm@rice.edu>
NSBRI Research Scientist/Programmer
Computer and Information Technology Institute
Rice University, 6100 S. Main St., Houston, TX 77005

#23Zeugswetter Andreas SB
ZeugswetterA@wien.spardat.at
In reply to: Ross J. Reedstrom (#22)
AW: AW: Backend-internal SPI operations

Zeugswetter Andreas SB <ZeugswetterA@wien.spardat.at> writes:

I don't see why a real view should still be createable by the old
create table then create rule way.

Because we'll need to be able to read dump files created by existing
versions of pg_dump. If we don't cope with CREATE TABLE + CREATE RULE
then restored-from-dump views won't really be views and will act
differently from freshly created views. Avoiding the resulting
support headaches is worth a little bit of ugliness in the code.

Well, since an on select do instead rule on a whole table is offhand
only good for a view I do agree. (At least I can't think of another use)

The only worry I have is that it should not destroy the possibility for real

on select do instead rules on single attributes.

Andreas

#24Jan Wieck
janwieck@Yahoo.com
In reply to: Zeugswetter Andreas SB (#23)
Re: AW: AW: Backend-internal SPI operations

Zeugswetter Andreas SB wrote:

Zeugswetter Andreas SB <ZeugswetterA@wien.spardat.at> writes:

I don't see why a real view should still be createable by the old
create table then create rule way.

Because we'll need to be able to read dump files created by existing
versions of pg_dump. If we don't cope with CREATE TABLE + CREATE RULE
then restored-from-dump views won't really be views and will act
differently from freshly created views. Avoiding the resulting
support headaches is worth a little bit of ugliness in the code.

Well, since an on select do instead rule on a whole table is offhand
only good for a view I do agree. (At least I can't think of another use)

The only worry I have is that it should not destroy the possibility for real

on select do instead rules on single attributes.

Andreas,

The rule system we got from Berkeley was "brittle and broken"
(as the original docs say).

6.4 was the first release with a rule system that was useful
for things other than views.

With 6.4 I restricted rules ON SELECT to:

- must be INSTEAD
- must NOT be conditional
- must have exactly one SELECT query as action
- the action must return a targetlist identical to the
table layout they are fired for
- must be named "_RET<tablename>".

Yes, that is a VIEW, a whole VIEW and nothing but a VIEW, so
help me God.

We don't have any other ON SELECT rules than views - for
years now. And I'm not sure if the concept of single
attribute rules ON SELECT could ever be implemented in the
rewriter. It's already complicated enough with view rules,
and I remember the sleepless nights well enough not to try.

Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck@Yahoo.com #

#25Jan Wieck
janwieck@Yahoo.com
In reply to: Jan Wieck (#24)
Re: Backend-internal SPI operations

Peter Eisentraut wrote:

Jan Wieck writes:

And it's time to make more use of the relkind attribute. For
7.2, when we want to have tuple-set returns for functions, we
might want to have structures as well

After the fabled query-tree redesign, couldn't you implement views simply
like this:

SELECT * FROM my_view;

becomes

SELECT * FROM (view_definition);

Hmmm, don't know what you mean with that.

Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck@Yahoo.com #

#26Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jan Wieck (#25)
Re: Backend-internal SPI operations

Peter Eisentraut <peter_e@gmx.net> writes:

then it becomes
SELECT * FROM (SELECT a, b, c FROM my_table);
which would presumably be possible with the new query-tree.

Right, that's exactly how we've been planning to fix the problems
with grouped views and so forth.

I am beginning to think that this may have to happen for 7.1, else
view inside ISO-style JOIN constructs aren't going to work right.
Further news as it happens...

regards, tom lane

#27Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#26)
Re: Backend-internal SPI operations

Jan Wieck <janwieck@Yahoo.com> writes:

I suggest let's get what we have out of the door now and
don't work under pressure on these complex things.

Pressure? I've got a month, I thought. Should be plenty of time.

regards, tom lane

#28Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#27)
Re: Backend-internal SPI operations

Jan Wieck <janwieck@Yahoo.com> writes:

Hmm - too simple - real life is harder. So to what do you
expand the query

SELECT a, c, d FROM my_view, other_table
WHERE my_view.a = other_table.a
AND other_table.x = 'foo';

SELECT a, c, d
FROM (SELECT a, b, c FROM my_table) AS my_view, other_table
WHERE my_view.a = other_table.a
AND other_table.x = 'foo';

I'm still not detecting a problem here ... if selecting from a view
*doesn't* act exactly like a sub-SELECT, it'd be broken IMHO.

We're not that far away from being able to do this, and it looks more
attractive to work on that than to hack the rewriter into an even
greater state of unintelligibility ...

regards, tom lane

#29Jan Wieck
janwieck@Yahoo.com
In reply to: Tom Lane (#28)
Re: Backend-internal SPI operations

Peter Eisentraut wrote:

Jan Wieck writes:

Hmmm, don't know what you mean with that.

If I define a view

CREATE my_view AS SELECT a, b, c FROM my_table;

and then do

SELECT * FROM my_view;

then it becomes

SELECT * FROM (SELECT a, b, c FROM my_table);

which would presumably be possible with the new query-tree.

Hmm - too simple - real life is harder. So to what do you
expand the query

SELECT a, c, d FROM my_view, other_table
WHERE my_view.a = other_table.a
AND other_table.x = 'foo';

And then have a little more complex "my_view", maybe a join
with it's own WHERE clause.

Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck@Yahoo.com #

#30Jan Wieck
janwieck@Yahoo.com
In reply to: Tom Lane (#26)
Re: Backend-internal SPI operations

Tom Lane wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

then it becomes
SELECT * FROM (SELECT a, b, c FROM my_table);
which would presumably be possible with the new query-tree.

Right, that's exactly how we've been planning to fix the problems
with grouped views and so forth.

I am beginning to think that this may have to happen for 7.1, else
view inside ISO-style JOIN constructs aren't going to work right.
Further news as it happens...

You really want to start on that NOW?

I suggest let's get what we have out of the door now and
don't work under pressure on these complex things.

Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck@Yahoo.com #

#31Jan Wieck
janwieck@Yahoo.com
In reply to: Tom Lane (#28)
Re: Backend-internal SPI operations

Tom Lane wrote:

Jan Wieck <janwieck@Yahoo.com> writes:

Hmm - too simple - real life is harder. So to what do you
expand the query

SELECT a, c, d FROM my_view, other_table
WHERE my_view.a = other_table.a
AND other_table.x = 'foo';

SELECT a, c, d
FROM (SELECT a, b, c FROM my_table) AS my_view, other_table
WHERE my_view.a = other_table.a
AND other_table.x = 'foo';

I'm still not detecting a problem here ... if selecting from a view
*doesn't* act exactly like a sub-SELECT, it'd be broken IMHO.

I do. The qualification does not restrict the subselect in
any way. So it'll be a sequential scan - no?

Imagine my_table has 10,000,000 rows and other_table is
small. With an index on my_table.a and the rewriting we do
today there's a good chance to end up with index lookups in
my_table for all the other_table matches of x = 'foo'.

Of course, after all the view must behave like a subselect.
But please only logical - not physical!

So the hard part of the NEW rewriter will be to detect which
qualifications can be moved/duplicated down into which
subselects (tuple sources) to restrict scans.

We're not that far away from being able to do this, and it looks more
attractive to work on that than to hack the rewriter into an even
greater state of unintelligibility ...

Then again, let's get 7.1 out as is and do the full querytree
redesign for 7.2. It looks easy, but I fear it's more or less
like an iceberg.

Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck@Yahoo.com #

#32Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jan Wieck (#31)
Re: Backend-internal SPI operations

Jan Wieck <janwieck@yahoo.com> writes:

So the hard part of the NEW rewriter will be to detect which
qualifications can be moved/duplicated down into which
subselects (tuple sources) to restrict scans.

Actually, what I was envisioning was pulling the subselect's guts *up*
into the main query (collapsing out the sub-Query node) if the sub-Query
is simple enough --- that is, no grouping/sorting/aggregates/etc. The
nice thing about that is we can start with a very simple method that
only deals with easy cases. The hard cases will still *work*.
I consider that an improvement over the current situation, where even
simple cases are nightmarishly difficult to implement (as you well know)
and the hard cases don't work. Worst case is that some
intermediate-complexity examples might lose performance for a while
until we build up a smart subquery-merging algorithm, but that seems
a price worth paying.

Then again, let's get 7.1 out as is

Has the release schedule moved up without my knowing about it?
I don't feel any urgent need to freeze development now...

and do the full querytree
redesign for 7.2. It looks easy, but I fear it's more or less
like an iceberg.

The original reason for this effort was to do a trial balloon that would
give us more knowledge about how to do it right for 7.2. The more I get
into it, the more I realize what a good idea that was. I'm not sure how
much of what I'm doing now will be completely discarded in the 7.2
cycle, but I do know that I understand the issues a lot better than
I did a week ago...

regards, tom lane

#33Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Mark Hollomon (#1)
Re: disallow LOCK on a view

Applied. Thanks.

This patch is for the TODO item

* Disallow LOCK on view

src/backend/commands/command.c is the only affected file

--
Mark Hollomon
mhh@mindspring.com

[ Attachment, skipping... ]

-- 
  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
#34Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Mark Hollomon (#14)
Re: Re: New relkind for views

"Hollomon, Mark" wrote:

Do we still want to be able to inherit from views?

Also:

Currently a view may be dropped with either 'DROP VIEW'
or 'DROP TABLE'. Should this be changed?

I say let them drop it with either one.

-- 
  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
#35The Hermit Hacker
scrappy@hub.org
In reply to: Bruce Momjian (#34)
Re: Re: New relkind for views

On Mon, 16 Oct 2000, Bruce Momjian wrote:

"Hollomon, Mark" wrote:

Do we still want to be able to inherit from views?

Also:

Currently a view may be dropped with either 'DROP VIEW'
or 'DROP TABLE'. Should this be changed?

I say let them drop it with either one.

I kinda like the 'drop index with drop index', 'drop table with drop
table' and 'drop view with drop view' groupings ... at least you are
pretty sure you haven't 'oopsed' in the process :)

#36Bruce Momjian
pgman@candle.pha.pa.us
In reply to: The Hermit Hacker (#35)
Re: Re: New relkind for views]

On Mon, 16 Oct 2000, Bruce Momjian wrote:

"Hollomon, Mark" wrote:

Do we still want to be able to inherit from views?

Also:

Currently a view may be dropped with either 'DROP VIEW'
or 'DROP TABLE'. Should this be changed?

I say let them drop it with either one.

I kinda like the 'drop index with drop index', 'drop table with drop
table' and 'drop view with drop view' groupings ... at least you are
pretty sure you haven't 'oopsed' in the process :)

Good point. Oops is bad.

-- 
  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
#37Mark Hollomon
mhh@mindspring.com
In reply to: The Hermit Hacker (#35)
Re: Re: New relkind for views

On Mon, Oct 16, 2000 at 08:41:43PM -0300, The Hermit Hacker wrote:

On Mon, 16 Oct 2000, Bruce Momjian wrote:

"Hollomon, Mark" wrote:

Do we still want to be able to inherit from views?

Also:

Currently a view may be dropped with either 'DROP VIEW'
or 'DROP TABLE'. Should this be changed?

I say let them drop it with either one.

I kinda like the 'drop index with drop index', 'drop table with drop
table' and 'drop view with drop view' groupings ... at least you are
pretty sure you haven't 'oopsed' in the process :)

So the vote is now tied. Any other opinions

--
Mark Hollomon
mhh@mindspring.com

#38Tom Lane
tgl@sss.pgh.pa.us
In reply to: Mark Hollomon (#37)
Re: Re: New relkind for views

Mark Hollomon <mhh@mindspring.com> writes:

I say let them drop it with either one.

I kinda like the 'drop index with drop index', 'drop table with drop
table' and 'drop view with drop view' groupings ... at least you are
pretty sure you haven't 'oopsed' in the process :)

So the vote is now tied. Any other opinions

I vote for the fascist approach (command must agree with actual type
of object). Seems safest. Please make sure the error message is
helpful though, like "Use DROP SEQUENCE to drop a sequence".

regards, tom lane

#39Zeugswetter Andreas SB
ZeugswetterA@wien.spardat.at
In reply to: Tom Lane (#38)
AW: Re: New relkind for views

Currently a view may be dropped with either 'DROP VIEW'
or 'DROP TABLE'. Should this be changed?

I say let them drop it with either one.

I kinda like the 'drop index with drop index', 'drop table with drop
table' and 'drop view with drop view' groupings ... at least you are
pretty sure you haven't 'oopsed' in the process :)

So the vote is now tied. Any other opinions

Same here: allow "drop view" only

Andreas

#40Mark Hollomon
mhh@mindspring.com
In reply to: Tom Lane (#38)
Re: Re: New relkind for views

On Monday 16 October 2000 20:56, Tom Lane wrote:

Mark Hollomon <mhh@mindspring.com> writes:

I say let them drop it with either one.

I kinda like the 'drop index with drop index', 'drop table with drop
table' and 'drop view with drop view' groupings ... at least you are
pretty sure you haven't 'oopsed' in the process :)

So the vote is now tied. Any other opinions

I vote for the fascist approach (command must agree with actual type
of object). Seems safest. Please make sure the error message is
helpful though, like "Use DROP SEQUENCE to drop a sequence".

Since Bruce changed his vote, it is now 3 to 0 for fascism.

I'll see what I can do.

--
Mark Hollomon