Thread: DROP VIEW code question
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
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/
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
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
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