Hello
2013/11/27 Tom Lane <t...@sss.pgh.pa.us>
> Dean Rasheed <dean.a.rash...@gmail.com> writes:
> > Actually the IF EXISTS in DROP TABLE now applies to the schema as
> > well. Unfortunately there is currently no consistency across the
> > various DROP commands --- some tolerate a non-existent schema, while
> > others error out.
>
> Yeah. I think now that we've had this discussion, we should make them
> all tolerate a non-existent schema. I'm fine with having that happen
> over a series of patches rather than all at once though.
>
> > Also amongst those that tolerate a non-existent
> > schema, the resulting notices are not consistent --- some report the
> > schema-qualified object name, while others just report the local
> > object name.
>
> Less excited about this part, but on the whole I'd vote for the "schema
> "no_such_schema" does not exist" wording in cases where the schema isn't
> there. The other way is less specific for no very good reason.
>
can be used this behave (see attached patch, please)?
if it is correct, I'll work on second patch, that unify check and notices
for other DROP IF EXISTS statements.
Regards
Pavel
>
> regards, tom lane
>
commit a2de2b402247fded7feba90bf299e91ee14aacca
Author: Pavel Stehule <pavel.steh...@gooddata.com>
Date: Thu Nov 28 08:39:43 2013 +0100
initial
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 9011190..234673f 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -942,26 +942,30 @@ get_object_address_relobject(ObjectType objtype, List *objname,
/* Extract relation name and open relation. */
relname = list_truncate(list_copy(objname), nnames - 1);
- relation = heap_openrv(makeRangeVarFromNameList(relname),
- AccessShareLock);
- reloid = RelationGetRelid(relation);
+ relation = heap_openrv_extended(makeRangeVarFromNameList(relname),
+ AccessShareLock,
+ missing_ok);
+
+ reloid = (relation != NULL) ? RelationGetRelid(relation) : InvalidOid;
switch (objtype)
{
case OBJECT_RULE:
address.classId = RewriteRelationId;
- address.objectId = get_rewrite_oid(reloid, depname, missing_ok);
+ address.objectId = OidIsValid(reloid) ?
+ get_rewrite_oid(reloid, depname, missing_ok) : InvalidOid;
address.objectSubId = 0;
break;
case OBJECT_TRIGGER:
address.classId = TriggerRelationId;
- address.objectId = get_trigger_oid(reloid, depname, missing_ok);
+ address.objectId = OidIsValid(reloid) ?
+ get_trigger_oid(reloid, depname, missing_ok) : InvalidOid;
address.objectSubId = 0;
break;
case OBJECT_CONSTRAINT:
address.classId = ConstraintRelationId;
- address.objectId =
- get_relation_constraint_oid(reloid, depname, missing_ok);
+ address.objectId = OidIsValid(reloid) ?
+ get_relation_constraint_oid(reloid, depname, missing_ok) : InvalidOid;
address.objectSubId = 0;
break;
default:
@@ -975,7 +979,9 @@ get_object_address_relobject(ObjectType objtype, List *objname,
/* Avoid relcache leak when object not found. */
if (!OidIsValid(address.objectId))
{
- heap_close(relation, AccessShareLock);
+ if (relation != NULL)
+ heap_close(relation, AccessShareLock);
+
relation = NULL; /* department of accident prevention */
return address;
}
diff --git a/src/backend/commands/dropcmds.c b/src/backend/commands/dropcmds.c
index b32ad3a..191aa17 100644
--- a/src/backend/commands/dropcmds.c
+++ b/src/backend/commands/dropcmds.c
@@ -23,6 +23,7 @@
#include "catalog/pg_class.h"
#include "catalog/pg_proc.h"
#include "commands/defrem.h"
+#include "lib/stringinfo.h"
#include "miscadmin.h"
#include "nodes/makefuncs.h"
#include "parser/parse_type.h"
@@ -32,6 +33,12 @@
static void does_not_exist_skipping(ObjectType objtype,
List *objname, List *objargs);
+static void prepare_notice_recheck_parent(const char *message,
+ List *objname,
+ const char **msg,
+ char **name,
+ char **args);
+
/*
* Drop one or more objects.
*
@@ -125,6 +132,53 @@ RemoveObjects(DropStmt *stmt)
}
/*
+ * Prepare text of "does not exist, skipping" message for triggers and rules,
+ * where we check schema name first, then table name, and as last we raise
+ * message about missing object.
+ */
+static void
+prepare_notice_recheck_parent(const char *message, List *objname,
+ const char **msg,
+ char **name,
+ char **args)
+{
+ RangeVar *rel;
+
+ rel = makeRangeVarFromNameList(list_truncate(list_copy(objname),
+ list_length(objname) - 1));
+
+ if (rel->schemaname != NULL && !OidIsValid(LookupNamespaceNoError(rel->schemaname)))
+ {
+ *msg = gettext_noop("schema \"%s\" does not exists, skipping");
+ *name = rel->schemaname;
+ *args = NULL;
+ }
+ else if (!OidIsValid(RangeVarGetRelid(rel, NoLock, true)))
+ {
+ *msg = gettext_noop("relation \"%s\" does not exists, skipping");
+ *name = rel->relname;
+ *args = NULL;
+ }
+ else
+ {
+ StringInfoData string;
+
+ *msg = message;
+ *name = strVal(llast(objname));
+
+ initStringInfo(&string);
+ if (rel->schemaname != NULL)
+ {
+ appendStringInfoString(&string, rel->schemaname);
+ appendStringInfoChar(&string, '.');
+ }
+ appendStringInfoString(&string, rel->relname);
+
+ *args = string.data;
+ }
+}
+
+/*
* Generate a NOTICE stating that the named object was not found, and is
* being skipped. This is only relevant when "IF EXISTS" is used; otherwise,
* get_object_address() will throw an ERROR.
@@ -201,20 +255,18 @@ does_not_exist_skipping(ObjectType objtype, List *objname, List *objargs)
(TypeName *) linitial(objargs)));
break;
case OBJECT_TRIGGER:
- msg = gettext_noop("trigger \"%s\" for table \"%s\" does not exist, skipping");
- name = strVal(llast(objname));
- args = NameListToString(list_truncate(list_copy(objname),
- list_length(objname) - 1));
+ prepare_notice_recheck_parent("trigger \"%s\" for table \"%s\" does not exist, skipping",
+ objname,
+ &msg, &name, &args);
break;
case OBJECT_EVENT_TRIGGER:
msg = gettext_noop("event trigger \"%s\" does not exist, skipping");
name = NameListToString(objname);
break;
case OBJECT_RULE:
- msg = gettext_noop("rule \"%s\" for relation \"%s\" does not exist, skipping");
- name = strVal(llast(objname));
- args = NameListToString(list_truncate(list_copy(objname),
- list_length(objname) - 1));
+ prepare_notice_recheck_parent("rule \"%s\" for relation \"%s\" does not exist, skipping",
+ objname,
+ &msg, &name, &args);
break;
case OBJECT_FDW:
msg = gettext_noop("foreign-data wrapper \"%s\" does not exist, skipping");
diff --git a/src/test/regress/expected/drop_if_exists.out b/src/test/regress/expected/drop_if_exists.out
index 8599401..663b438 100644
--- a/src/test/regress/expected/drop_if_exists.out
+++ b/src/test/regress/expected/drop_if_exists.out
@@ -173,7 +173,11 @@ NOTICE: trigger "test_trigger_exists" for table "test_exists" does not exist, s
DROP TRIGGER test_trigger_exists ON no_such_table;
ERROR: relation "no_such_table" does not exist
DROP TRIGGER IF EXISTS test_trigger_exists ON no_such_table;
-ERROR: relation "no_such_table" does not exist
+NOTICE: relation "no_such_table" does not exists, skipping
+DROP TRIGGER test_trigger_exists ON no_such_schema.no_such_table;
+ERROR: schema "no_such_schema" does not exist
+DROP TRIGGER IF EXISTS test_trigger_exists ON no_such_schema.no_such_table;
+NOTICE: schema "no_such_schema" does not exists, skipping
CREATE TRIGGER test_trigger_exists
BEFORE UPDATE ON test_exists
FOR EACH ROW EXECUTE PROCEDURE suppress_redundant_updates_trigger();
@@ -186,7 +190,11 @@ NOTICE: rule "test_rule_exists" for relation "test_exists" does not exist, skip
DROP RULE test_rule_exists ON no_such_table;
ERROR: relation "no_such_table" does not exist
DROP RULE IF EXISTS test_rule_exists ON no_such_table;
-ERROR: relation "no_such_table" does not exist
+NOTICE: relation "no_such_table" does not exists, skipping
+DROP RULE test_rule_exists ON no_such_schema.no_such_table;
+ERROR: schema "no_such_schema" does not exist
+DROP RULE IF EXISTS test_rule_exists ON no_such_schema.no_such_table;
+NOTICE: schema "no_such_schema" does not exists, skipping
CREATE RULE test_rule_exists AS ON INSERT TO test_exists
DO INSTEAD
INSERT INTO test_exists VALUES (NEW.a, NEW.b || NEW.a::text);
diff --git a/src/test/regress/sql/drop_if_exists.sql b/src/test/regress/sql/drop_if_exists.sql
index 6330566..8977357 100644
--- a/src/test/regress/sql/drop_if_exists.sql
+++ b/src/test/regress/sql/drop_if_exists.sql
@@ -182,6 +182,9 @@ DROP TRIGGER IF EXISTS test_trigger_exists ON test_exists;
DROP TRIGGER test_trigger_exists ON no_such_table;
DROP TRIGGER IF EXISTS test_trigger_exists ON no_such_table;
+DROP TRIGGER test_trigger_exists ON no_such_schema.no_such_table;
+DROP TRIGGER IF EXISTS test_trigger_exists ON no_such_schema.no_such_table;
+
CREATE TRIGGER test_trigger_exists
BEFORE UPDATE ON test_exists
FOR EACH ROW EXECUTE PROCEDURE suppress_redundant_updates_trigger();
@@ -194,6 +197,9 @@ DROP RULE IF EXISTS test_rule_exists ON test_exists;
DROP RULE test_rule_exists ON no_such_table;
DROP RULE IF EXISTS test_rule_exists ON no_such_table;
+DROP RULE test_rule_exists ON no_such_schema.no_such_table;
+DROP RULE IF EXISTS test_rule_exists ON no_such_schema.no_such_table;
+
CREATE RULE test_rule_exists AS ON INSERT TO test_exists
DO INSTEAD
INSERT INTO test_exists VALUES (NEW.a, NEW.b || NEW.a::text);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
