Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitf1ad067

Browse files
committed
Sort the dependent objects before recursing in findDependentObjects().
Historically, the notices output by DROP CASCADE tended to come outin uncertain order, and in some cases you might get different claimsabout which object depends on which other one. This is because wejust traversed the dependency tree in the order in which pg_dependentries are seen, and nbtree has never promised anything about theorder of equal-keyed index entries. We've put up with that for years,hacking regression tests when necessary to prevent them from emittingunstable output. However, it's a problem for pending work that willchange nbtree's behavior for equal keys, as that causes unexpectedchanges in the regression test results.Hence, adjust findDependentObjects to sort the results of eachindexscan before processing them. The sort is on descending OID ofthe dependent objects, hence more or less reverse creation order.While this rule could still result in bogus regression test failuresif an OID wraparound occurred mid-test, that seems unlikely to happenin any plausible development or packaging-test scenario.This is enough to ensure output stability for ordinary DROP CASCADEcommands, but not for DROP OWNED BY, because that has a differentcode path with the same problem. We might later choose to sort inthe DROP OWNED BY code as well, but this patch doesn't do so.I've also not done anything about reverting the existing hacks tosuppress unstable DROP CASCADE output in specific regression tests.It might be worth undoing those, but it seems like a distinct question.The first indexscan loop in findDependentObjects is not touched,meaning there is a hazard of unstable error reports from that too.However, said hazard is not the fault of that code: it was designedon the assumption that there could be at most one "owning" objectto complain about, and that assumption does not seem unreasonable.The recent patch that added the possibility of multipleDEPENDENCY_INTERNAL_AUTO links broke that assumption, but we shouldfix that situation not band-aid around it. That's a matter foranother patch, though.Discussion:https://postgr.es/m/12244.1547854440@sss.pgh.pa.us
1 parentfcea1e1 commitf1ad067

File tree

6 files changed

+98
-28
lines changed

6 files changed

+98
-28
lines changed

‎src/backend/catalog/dependency.c

Lines changed: 85 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,13 @@ typedef struct ObjectAddressStack
124124
structObjectAddressStack*next;/* next outer stack level */
125125
}ObjectAddressStack;
126126

127+
/* temporary storage in findDependentObjects */
128+
typedefstruct
129+
{
130+
ObjectAddressobj;/* object to be deleted --- MUST BE FIRST */
131+
intsubflags;/* flags to pass down when recursing to obj */
132+
}ObjectAddressAndFlags;
133+
127134
/* for find_expr_references_walker */
128135
typedefstruct
129136
{
@@ -472,6 +479,9 @@ findDependentObjects(const ObjectAddress *object,
472479
SysScanDescscan;
473480
HeapTupletup;
474481
ObjectAddressotherObject;
482+
ObjectAddressAndFlags*dependentObjects;
483+
intnumDependentObjects;
484+
intmaxDependentObjects;
475485
ObjectAddressStackmystack;
476486
ObjectAddressExtraextra;
477487

@@ -704,12 +714,17 @@ findDependentObjects(const ObjectAddress *object,
704714
systable_endscan(scan);
705715

706716
/*
707-
* Now recurse to any dependent objects. We must visit them first since
708-
* they have to be deleted before the current object.
717+
* Next, identify all objects that directly depend on the current object.
718+
* To ensure predictable deletion order, we collect them up in
719+
* dependentObjects and sort the list before actually recursing. (The
720+
* deletion order would be valid in any case, but doing this ensures
721+
* consistent output from DROP CASCADE commands, which is helpful for
722+
* regression testing.)
709723
*/
710-
mystack.object=object;/* set up a new stack level */
711-
mystack.flags=objflags;
712-
mystack.next=stack;
724+
maxDependentObjects=128;/* arbitrary initial allocation */
725+
dependentObjects= (ObjectAddressAndFlags*)
726+
palloc(maxDependentObjects*sizeof(ObjectAddressAndFlags));
727+
numDependentObjects=0;
713728

714729
ScanKeyInit(&key[0],
715730
Anum_pg_depend_refclassid,
@@ -762,7 +777,10 @@ findDependentObjects(const ObjectAddress *object,
762777
continue;
763778
}
764779

765-
/* Recurse, passing objflags indicating the dependency type */
780+
/*
781+
* We do need to delete it, so identify objflags to be passed down,
782+
* which depend on the dependency type.
783+
*/
766784
switch (foundDep->deptype)
767785
{
768786
caseDEPENDENCY_NORMAL:
@@ -798,16 +816,55 @@ findDependentObjects(const ObjectAddress *object,
798816
break;
799817
}
800818

801-
findDependentObjects(&otherObject,
802-
subflags,
819+
/* And add it to the pending-objects list */
820+
if (numDependentObjects >=maxDependentObjects)
821+
{
822+
/* enlarge array if needed */
823+
maxDependentObjects *=2;
824+
dependentObjects= (ObjectAddressAndFlags*)
825+
repalloc(dependentObjects,
826+
maxDependentObjects*sizeof(ObjectAddressAndFlags));
827+
}
828+
829+
dependentObjects[numDependentObjects].obj=otherObject;
830+
dependentObjects[numDependentObjects].subflags=subflags;
831+
numDependentObjects++;
832+
}
833+
834+
systable_endscan(scan);
835+
836+
/*
837+
* Now we can sort the dependent objects into a stable visitation order.
838+
* It's safe to use object_address_comparator here since the obj field is
839+
* first within ObjectAddressAndFlags.
840+
*/
841+
if (numDependentObjects>1)
842+
qsort((void*)dependentObjects,numDependentObjects,
843+
sizeof(ObjectAddressAndFlags),
844+
object_address_comparator);
845+
846+
/*
847+
* Now recurse to the dependent objects. We must visit them first since
848+
* they have to be deleted before the current object.
849+
*/
850+
mystack.object=object;/* set up a new stack level */
851+
mystack.flags=objflags;
852+
mystack.next=stack;
853+
854+
for (inti=0;i<numDependentObjects;i++)
855+
{
856+
ObjectAddressAndFlags*depObj=dependentObjects+i;
857+
858+
findDependentObjects(&depObj->obj,
859+
depObj->subflags,
803860
flags,
804861
&mystack,
805862
targetObjects,
806863
pendingObjects,
807864
depRel);
808865
}
809866

810-
systable_endscan(scan);
867+
pfree(dependentObjects);
811868

812869
/*
813870
* Finally, we can add the target object to targetObjects. Be careful to
@@ -2109,18 +2166,31 @@ object_address_comparator(const void *a, const void *b)
21092166
constObjectAddress*obja= (constObjectAddress*)a;
21102167
constObjectAddress*objb= (constObjectAddress*)b;
21112168

2112-
if (obja->classId<objb->classId)
2169+
/*
2170+
* Primary sort key is OID descending. Most of the time, this will result
2171+
* in putting newer objects before older ones, which is likely to be the
2172+
* right order to delete in.
2173+
*/
2174+
if (obja->objectId>objb->objectId)
21132175
return-1;
2114-
if (obja->classId>objb->classId)
2115-
return1;
21162176
if (obja->objectId<objb->objectId)
2177+
return1;
2178+
2179+
/*
2180+
* Next sort on catalog ID, in case identical OIDs appear in different
2181+
* catalogs. Sort direction is pretty arbitrary here.
2182+
*/
2183+
if (obja->classId<objb->classId)
21172184
return-1;
2118-
if (obja->objectId>objb->objectId)
2185+
if (obja->classId>objb->classId)
21192186
return1;
21202187

21212188
/*
2122-
* We sort the subId as an unsigned int so that 0 will come first. See
2123-
* logic in eliminate_duplicate_dependencies.
2189+
* Last, sort on object subId.
2190+
*
2191+
* We sort the subId as an unsigned int so that 0 (the whole object) will
2192+
* come first. This is essential for eliminate_duplicate_dependencies,
2193+
* and is also the best order for findDependentObjects.
21242194
*/
21252195
if ((unsignedint)obja->objectSubId< (unsignedint)objb->objectSubId)
21262196
return-1;

‎src/test/regress/expected/alter_table.out

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2583,10 +2583,10 @@ DETAIL: drop cascades to table alter2.t1
25832583
drop cascades to view alter2.v1
25842584
drop cascades to function alter2.plus1(integer)
25852585
drop cascades to type alter2.posint
2586-
drop cascades to operator family alter2.ctype_hash_ops for access method hash
25872586
drop cascades to type alter2.ctype
25882587
drop cascades to function alter2.same(alter2.ctype,alter2.ctype)
25892588
drop cascades to operator alter2.=(alter2.ctype,alter2.ctype)
2589+
drop cascades to operator family alter2.ctype_hash_ops for access method hash
25902590
drop cascades to conversion alter2.ascii_to_utf8
25912591
drop cascades to text search parser alter2.prs
25922592
drop cascades to text search configuration alter2.cfg

‎src/test/regress/expected/create_type.out

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -161,13 +161,13 @@ DROP FUNCTION base_fn_out(opaque); -- error
161161
ERROR: function base_fn_out(opaque) does not exist
162162
DROP TYPE base_type; -- error
163163
ERROR: cannot drop type base_type because other objects depend on it
164-
DETAIL: functionbase_fn_out(base_type) depends on type base_type
165-
functionbase_fn_in(cstring) depends on type base_type
164+
DETAIL: functionbase_fn_in(cstring) depends on type base_type
165+
functionbase_fn_out(base_type) depends on type base_type
166166
HINT: Use DROP ... CASCADE to drop the dependent objects too.
167167
DROP TYPE base_type CASCADE;
168168
NOTICE: drop cascades to 2 other objects
169-
DETAIL: drop cascades to functionbase_fn_out(base_type)
170-
drop cascades to functionbase_fn_in(cstring)
169+
DETAIL: drop cascades to functionbase_fn_in(cstring)
170+
drop cascades to functionbase_fn_out(base_type)
171171
-- Check usage of typmod with a user-defined type
172172
-- (we have borrowed numeric's typmod functions)
173173
CREATE TEMP TABLE mytab (foo widget(42,13,7)); -- should fail

‎src/test/regress/expected/domain.out

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -645,8 +645,8 @@ alter domain dnotnulltest drop not null;
645645
update domnotnull set col1 = null;
646646
drop domain dnotnulltest cascade;
647647
NOTICE: drop cascades to 2 other objects
648-
DETAIL: drop cascades to columncol1 of table domnotnull
649-
drop cascades to columncol2 of table domnotnull
648+
DETAIL: drop cascades to columncol2 of table domnotnull
649+
drop cascades to columncol1 of table domnotnull
650650
-- Test ALTER DOMAIN .. DEFAULT ..
651651
create table domdeftest (col1 ddef1);
652652
insert into domdeftest default values;

‎src/test/regress/expected/matview.out

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -311,12 +311,12 @@ SELECT type, m.totamt AS mtot, v.totamt AS vtot FROM mvtest_tm m LEFT JOIN mvtes
311311
DROP TABLE mvtest_t;
312312
ERROR: cannot drop table mvtest_t because other objects depend on it
313313
DETAIL: view mvtest_tv depends on table mvtest_t
314+
materialized view mvtest_mvschema.mvtest_tvm depends on view mvtest_tv
315+
materialized view mvtest_tvmm depends on materialized view mvtest_mvschema.mvtest_tvm
314316
view mvtest_tvv depends on view mvtest_tv
315317
materialized view mvtest_tvvm depends on view mvtest_tvv
316318
view mvtest_tvvmv depends on materialized view mvtest_tvvm
317319
materialized view mvtest_bb depends on view mvtest_tvvmv
318-
materialized view mvtest_mvschema.mvtest_tvm depends on view mvtest_tv
319-
materialized view mvtest_tvmm depends on materialized view mvtest_mvschema.mvtest_tvm
320320
materialized view mvtest_tm depends on table mvtest_t
321321
materialized view mvtest_tmm depends on materialized view mvtest_tm
322322
HINT: Use DROP ... CASCADE to drop the dependent objects too.
@@ -327,12 +327,12 @@ BEGIN;
327327
DROP TABLE mvtest_t CASCADE;
328328
NOTICE: drop cascades to 9 other objects
329329
DETAIL: drop cascades to view mvtest_tv
330+
drop cascades to materialized view mvtest_mvschema.mvtest_tvm
331+
drop cascades to materialized view mvtest_tvmm
330332
drop cascades to view mvtest_tvv
331333
drop cascades to materialized view mvtest_tvvm
332334
drop cascades to view mvtest_tvvmv
333335
drop cascades to materialized view mvtest_bb
334-
drop cascades to materialized view mvtest_mvschema.mvtest_tvm
335-
drop cascades to materialized view mvtest_tvmm
336336
drop cascades to materialized view mvtest_tm
337337
drop cascades to materialized view mvtest_tmm
338338
ROLLBACK;

‎src/test/regress/expected/updatable_views.out

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -334,18 +334,18 @@ DETAIL: drop cascades to view ro_view1
334334
drop cascades to view ro_view17
335335
drop cascades to view ro_view2
336336
drop cascades to view ro_view3
337+
drop cascades to view ro_view4
337338
drop cascades to view ro_view5
338339
drop cascades to view ro_view6
339340
drop cascades to view ro_view7
340341
drop cascades to view ro_view8
341342
drop cascades to view ro_view9
342343
drop cascades to view ro_view11
343344
drop cascades to view ro_view13
345+
drop cascades to view rw_view14
344346
drop cascades to view rw_view15
345347
drop cascades to view rw_view16
346348
drop cascades to view ro_view20
347-
drop cascades to view ro_view4
348-
drop cascades to view rw_view14
349349
DROP VIEW ro_view10, ro_view12, ro_view18;
350350
DROP SEQUENCE uv_seq CASCADE;
351351
NOTICE: drop cascades to view ro_view19

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp