Skip to content

Commit 0cefb50

Browse files
committed
Refactor the handling of the various DropStmt variants so that when multiple
objects are specified, we drop them all in a single performMultipleDeletions call. This makes the RESTRICT/CASCADE checks more relaxed: it's not counted as a cascade if one of the later objects has a dependency on an earlier one. NOTICE messages about such cases go away, too. In passing, fix the permissions check for DROP CONVERSION, which for some reason was never made role-aware, and omitted the namespace-owner exemption too. Alex Hunsaker, with further fiddling by me.
1 parent 95ce4ee commit 0cefb50

20 files changed

+657
-673
lines changed

src/backend/catalog/dependency.c

+9-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/catalog/dependency.c,v 1.75 2008/06/11 21:53:48 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/catalog/dependency.c,v 1.76 2008/06/14 18:04:33 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -259,6 +259,10 @@ performMultipleDeletions(const ObjectAddresses *objects,
259259
ObjectAddresses *targetObjects;
260260
int i;
261261

262+
/* No work if no objects... */
263+
if (objects->numrefs <= 0)
264+
return;
265+
262266
/*
263267
* We save some cycles by opening pg_depend just once and passing the
264268
* Relation pointer down to all the recursive deletion steps.
@@ -295,11 +299,14 @@ performMultipleDeletions(const ObjectAddresses *objects,
295299

296300
/*
297301
* Check if deletion is allowed, and report about cascaded deletes.
302+
*
303+
* If there's exactly one object being deleted, report it the same
304+
* way as in performDeletion(), else we have to be vaguer.
298305
*/
299306
reportDependentObjects(targetObjects,
300307
behavior,
301308
NOTICE,
302-
NULL);
309+
(objects->numrefs == 1 ? objects->refs : NULL));
303310

304311
/*
305312
* Delete all the objects in the proper order.

src/backend/catalog/pg_conversion.c

+1-36
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/catalog/pg_conversion.c,v 1.43 2008/05/12 00:00:47 alvherre Exp $
11+
* $PostgreSQL: pgsql/src/backend/catalog/pg_conversion.c,v 1.44 2008/06/14 18:04:33 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -18,7 +18,6 @@
1818
#include "access/sysattr.h"
1919
#include "catalog/dependency.h"
2020
#include "catalog/indexing.h"
21-
#include "catalog/namespace.h"
2221
#include "catalog/pg_conversion.h"
2322
#include "catalog/pg_conversion_fn.h"
2423
#include "catalog/pg_namespace.h"
@@ -138,40 +137,6 @@ ConversionCreate(const char *conname, Oid connamespace,
138137
return oid;
139138
}
140139

141-
/*
142-
* ConversionDrop
143-
*
144-
* Drop a conversion after doing permission checks.
145-
*/
146-
void
147-
ConversionDrop(Oid conversionOid, DropBehavior behavior)
148-
{
149-
HeapTuple tuple;
150-
ObjectAddress object;
151-
152-
tuple = SearchSysCache(CONVOID,
153-
ObjectIdGetDatum(conversionOid),
154-
0, 0, 0);
155-
if (!HeapTupleIsValid(tuple))
156-
elog(ERROR, "cache lookup failed for conversion %u", conversionOid);
157-
158-
if (!superuser() &&
159-
((Form_pg_conversion) GETSTRUCT(tuple))->conowner != GetUserId())
160-
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CONVERSION,
161-
NameStr(((Form_pg_conversion) GETSTRUCT(tuple))->conname));
162-
163-
ReleaseSysCache(tuple);
164-
165-
/*
166-
* Do the deletion
167-
*/
168-
object.classId = ConversionRelationId;
169-
object.objectId = conversionOid;
170-
object.objectSubId = 0;
171-
172-
performDeletion(&object, behavior);
173-
}
174-
175140
/*
176141
* RemoveConversionById
177142
*

src/backend/commands/conversioncmds.c

+60-18
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/conversioncmds.c,v 1.33 2008/03/27 03:57:33 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/conversioncmds.c,v 1.34 2008/06/14 18:04:33 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -99,31 +99,73 @@ CreateConversionCommand(CreateConversionStmt *stmt)
9999
* DROP CONVERSION
100100
*/
101101
void
102-
DropConversionCommand(List *name, DropBehavior behavior, bool missing_ok)
102+
DropConversionsCommand(DropStmt *drop)
103103
{
104-
Oid conversionOid;
104+
ObjectAddresses *objects;
105+
ListCell *cell;
105106

106-
conversionOid = FindConversionByName(name);
107-
if (!OidIsValid(conversionOid))
107+
/*
108+
* First we identify all the conversions, then we delete them in a single
109+
* performMultipleDeletions() call. This is to avoid unwanted
110+
* DROP RESTRICT errors if one of the conversions depends on another.
111+
* (Not that that is very likely, but we may as well do this consistently.)
112+
*/
113+
objects = new_object_addresses();
114+
115+
foreach(cell, drop->objects)
108116
{
109-
if (!missing_ok)
110-
{
111-
ereport(ERROR,
112-
(errcode(ERRCODE_UNDEFINED_OBJECT),
113-
errmsg("conversion \"%s\" does not exist",
114-
NameListToString(name))));
115-
}
116-
else
117+
List *name = (List *) lfirst(cell);
118+
Oid conversionOid;
119+
HeapTuple tuple;
120+
Form_pg_conversion con;
121+
ObjectAddress object;
122+
123+
conversionOid = FindConversionByName(name);
124+
125+
if (!OidIsValid(conversionOid))
117126
{
118-
ereport(NOTICE,
119-
(errmsg("conversion \"%s\" does not exist, skipping",
120-
NameListToString(name))));
127+
if (!drop->missing_ok)
128+
{
129+
ereport(ERROR,
130+
(errcode(ERRCODE_UNDEFINED_OBJECT),
131+
errmsg("conversion \"%s\" does not exist",
132+
NameListToString(name))));
133+
}
134+
else
135+
{
136+
ereport(NOTICE,
137+
(errmsg("conversion \"%s\" does not exist, skipping",
138+
NameListToString(name))));
139+
}
140+
continue;
121141
}
122142

123-
return;
143+
tuple = SearchSysCache(CONVOID,
144+
ObjectIdGetDatum(conversionOid),
145+
0, 0, 0);
146+
if (!HeapTupleIsValid(tuple))
147+
elog(ERROR, "cache lookup failed for conversion %u",
148+
conversionOid);
149+
con = (Form_pg_conversion) GETSTRUCT(tuple);
150+
151+
/* Permission check: must own conversion or its namespace */
152+
if (!pg_conversion_ownercheck(conversionOid, GetUserId()) &&
153+
!pg_namespace_ownercheck(con->connamespace, GetUserId()))
154+
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CONVERSION,
155+
NameStr(con->conname));
156+
157+
object.classId = ConversionRelationId;
158+
object.objectId = conversionOid;
159+
object.objectSubId = 0;
160+
161+
add_exact_object_address(&object, objects);
162+
163+
ReleaseSysCache(tuple);
124164
}
125165

126-
ConversionDrop(conversionOid, behavior);
166+
performMultipleDeletions(objects, drop->behavior);
167+
168+
free_object_addresses(objects);
127169
}
128170

129171
/*

src/backend/commands/indexcmds.c

+1-29
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/indexcmds.c,v 1.176 2008/05/12 20:01:59 alvherre Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/indexcmds.c,v 1.177 2008/06/14 18:04:33 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -21,7 +21,6 @@
2121
#include "access/transam.h"
2222
#include "access/xact.h"
2323
#include "catalog/catalog.h"
24-
#include "catalog/dependency.h"
2524
#include "catalog/heap.h"
2625
#include "catalog/index.h"
2726
#include "catalog/indexing.h"
@@ -1256,33 +1255,6 @@ relationHasPrimaryKey(Relation rel)
12561255
return result;
12571256
}
12581257

1259-
1260-
/*
1261-
* RemoveIndex
1262-
* Deletes an index.
1263-
*/
1264-
void
1265-
RemoveIndex(RangeVar *relation, DropBehavior behavior)
1266-
{
1267-
Oid indOid;
1268-
char relkind;
1269-
ObjectAddress object;
1270-
1271-
indOid = RangeVarGetRelid(relation, false);
1272-
relkind = get_rel_relkind(indOid);
1273-
if (relkind != RELKIND_INDEX)
1274-
ereport(ERROR,
1275-
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
1276-
errmsg("\"%s\" is not an index",
1277-
relation->relname)));
1278-
1279-
object.classId = RelationRelationId;
1280-
object.objectId = indOid;
1281-
object.objectSubId = 0;
1282-
1283-
performDeletion(&object, behavior);
1284-
}
1285-
12861258
/*
12871259
* ReindexIndex
12881260
* Recreate a specific index.

src/backend/commands/schemacmds.c

+57-38
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/schemacmds.c,v 1.49 2008/01/03 21:23:15 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/schemacmds.c,v 1.50 2008/06/14 18:04:33 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -148,57 +148,76 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString)
148148

149149

150150
/*
151-
* RemoveSchema
152-
* Removes a schema.
151+
* RemoveSchemas
152+
* Implements DROP SCHEMA.
153153
*/
154154
void
155-
RemoveSchema(List *names, DropBehavior behavior, bool missing_ok)
155+
RemoveSchemas(DropStmt *drop)
156156
{
157-
char *namespaceName;
158-
Oid namespaceId;
159-
ObjectAddress object;
157+
ObjectAddresses *objects;
158+
ListCell *cell;
160159

161-
if (list_length(names) != 1)
162-
ereport(ERROR,
163-
(errcode(ERRCODE_SYNTAX_ERROR),
164-
errmsg("schema name cannot be qualified")));
165-
namespaceName = strVal(linitial(names));
166-
167-
namespaceId = GetSysCacheOid(NAMESPACENAME,
168-
CStringGetDatum(namespaceName),
169-
0, 0, 0);
170-
if (!OidIsValid(namespaceId))
160+
/*
161+
* First we identify all the schemas, then we delete them in a single
162+
* performMultipleDeletions() call. This is to avoid unwanted
163+
* DROP RESTRICT errors if one of the schemas depends on another.
164+
*/
165+
objects = new_object_addresses();
166+
167+
foreach(cell, drop->objects)
171168
{
172-
if (!missing_ok)
173-
{
169+
List *names = (List *) lfirst(cell);
170+
char *namespaceName;
171+
Oid namespaceId;
172+
ObjectAddress object;
173+
174+
if (list_length(names) != 1)
174175
ereport(ERROR,
175-
(errcode(ERRCODE_UNDEFINED_SCHEMA),
176-
errmsg("schema \"%s\" does not exist", namespaceName)));
177-
}
178-
else
176+
(errcode(ERRCODE_SYNTAX_ERROR),
177+
errmsg("schema name cannot be qualified")));
178+
namespaceName = strVal(linitial(names));
179+
180+
namespaceId = GetSysCacheOid(NAMESPACENAME,
181+
CStringGetDatum(namespaceName),
182+
0, 0, 0);
183+
184+
if (!OidIsValid(namespaceId))
179185
{
180-
ereport(NOTICE,
181-
(errmsg("schema \"%s\" does not exist, skipping",
182-
namespaceName)));
186+
if (!drop->missing_ok)
187+
{
188+
ereport(ERROR,
189+
(errcode(ERRCODE_UNDEFINED_SCHEMA),
190+
errmsg("schema \"%s\" does not exist",
191+
namespaceName)));
192+
}
193+
else
194+
{
195+
ereport(NOTICE,
196+
(errmsg("schema \"%s\" does not exist, skipping",
197+
namespaceName)));
198+
}
199+
continue;
183200
}
184201

185-
return;
186-
}
202+
/* Permission check */
203+
if (!pg_namespace_ownercheck(namespaceId, GetUserId()))
204+
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_NAMESPACE,
205+
namespaceName);
187206

188-
/* Permission check */
189-
if (!pg_namespace_ownercheck(namespaceId, GetUserId()))
190-
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_NAMESPACE,
191-
namespaceName);
207+
object.classId = NamespaceRelationId;
208+
object.objectId = namespaceId;
209+
object.objectSubId = 0;
210+
211+
add_exact_object_address(&object, objects);
212+
}
192213

193214
/*
194-
* Do the deletion. Objects contained in the schema are removed by means
195-
* of their dependency links to the schema.
215+
* Do the deletions. Objects contained in the schema(s) are removed by
216+
* means of their dependency links to the schema.
196217
*/
197-
object.classId = NamespaceRelationId;
198-
object.objectId = namespaceId;
199-
object.objectSubId = 0;
218+
performMultipleDeletions(objects, drop->behavior);
200219

201-
performDeletion(&object, behavior);
220+
free_object_addresses(objects);
202221
}
203222

204223

0 commit comments

Comments
 (0)