Thread: DROP VIEW code question

DROP VIEW code question

From
Mark Hollomon
Date:
In tcop/ulitity.c we have the following code fragment:

case VIEW:
{char       *viewName = stmt->name;char       *ruleName;
ruleName = MakeRetrieveViewRuleName(viewName);relationName = RewriteGetRuleEventRel(ruleName);

This looks like an expensive no-op to me.
if viewname == "myview"
then ruleName == "_RETmyview" (+/- multibyte aware truncation)
then relationName == "myview"

Is this code doing something that I'm missing?

Also

"DROP TABLE x, y, z" is allowed, but

"DROP VIEW x, y, z" is not.

Any reason other than historical?
-- 
Mark Hollomon


Re: DROP VIEW code question

From
Peter Eisentraut
Date:
Mark Hollomon writes:

> Also
> 
> "DROP TABLE x, y, z" is allowed, but
> 
> "DROP VIEW x, y, z" is not.
> 
> Any reason other than historical?

I don't know how it looks now, but the "DROP TABLE x, y, z" was pretty
broken a while ago.  For example, if there was some sort of dependency
between the tables (foreign keys?) it would abort and leave an
inconsistent state.  I'm not very fond of this extension, but keep the
issue in mind.

-- 
Peter Eisentraut      peter_e@gmx.net       http://yi.org/peter-e/



Re: DROP VIEW code question

From
Tom Lane
Date:
Mark Hollomon <mhh@mindspring.com> writes:
> In tcop/ulitity.c we have the following code fragment:
> case VIEW:
> {
>     char       *viewName = stmt->name;
>     char       *ruleName;

>     ruleName = MakeRetrieveViewRuleName(viewName);
>     relationName = RewriteGetRuleEventRel(ruleName);

> This looks like an expensive no-op to me.
> if viewname == "myview"
> then ruleName == "_RETmyview" (+/- multibyte aware truncation)
> then relationName == "myview"

> Is this code doing something that I'm missing?

It's probably done that way for symmetry with the DROP RULE case.
I don't see any big need to change it --- DROP VIEW is hardly a
performance-critical path.  And it *does* help ensure that what
you are dropping is a view not a plain table.

> Also
> "DROP TABLE x, y, z" is allowed, but
> "DROP VIEW x, y, z" is not.
> Any reason other than historical?

No, not that I can think of.  If you want to fix that, go for it.
You might consider merging DropStmt and RemoveStmt into one parsenode
type that has both a list and an object-type field.   I see no real
good reason why they're separate ...
        regards, tom lane


Re: DROP VIEW code question

From
Tom Lane
Date:
Peter Eisentraut <peter_e@gmx.net> writes:
> I don't know how it looks now, but the "DROP TABLE x, y, z" was pretty
> broken a while ago.  For example, if there was some sort of dependency
> between the tables (foreign keys?) it would abort and leave an
> inconsistent state.  I'm not very fond of this extension, but keep the
> issue in mind.

This is just a special case of the generic problem that you can't
roll back a DROP TABLE.  That'll be fixed by 7.1, so I see no reason
not to allow the more convenient syntax.

BTW, Mark, the reason utility.c implements T_DropStmt with two loops
is presumably to try to avoid the rollback-drop-table problem; but it's
inherently bogus because not all error conditions can be checked there.
You could fold the two loops into one loop, and/or remove any checks
that are redundant with RemoveRelation itself.
        regards, tom lane


Re: DROP VIEW code question

From
Mark Hollomon
Date:
On Tuesday 17 October 2000 16:33, Tom Lane wrote:
> Mark Hollomon <mhh@mindspring.com> writes:
> > In tcop/ulitity.c we have the following code fragment:
> > case VIEW:
> > {
> >     char       *viewName = stmt->name;
> >     char       *ruleName;
> >
> >     ruleName = MakeRetrieveViewRuleName(viewName);
> >     relationName = RewriteGetRuleEventRel(ruleName);
> >
> > This looks like an expensive no-op to me.
> > if viewname == "myview"
> > then ruleName == "_RETmyview" (+/- multibyte aware truncation)
> > then relationName == "myview"
> >
> > Is this code doing something that I'm missing?
>
> It's probably done that way for symmetry with the DROP RULE case.
> I don't see any big need to change it --- DROP VIEW is hardly a
> performance-critical path.  And it *does* help ensure that what
> you are dropping is a view not a plain table.

Yes, prior to the separate relkind for views, it was necessary for that.

I just didn't see a need now.

>
> > Also
> > "DROP TABLE x, y, z" is allowed, but
> > "DROP VIEW x, y, z" is not.
> > Any reason other than historical?
>
> No, not that I can think of.  If you want to fix that, go for it.
> You might consider merging DropStmt and RemoveStmt into one parsenode
> type that has both a list and an object-type field.   I see no real
> good reason why they're separate ...

Ok.

-- 
Mark Hollomon