Re: pgsql: Sort the dependent objects before recursing infindDependentObje - Mailing list pgsql-committers
From | Andres Freund |
---|---|
Subject | Re: pgsql: Sort the dependent objects before recursing infindDependentObje |
Date | |
Msg-id | 20190121201134.dyx6anto6akflh5d@alap3.anarazel.de Whole thread Raw |
In response to | pgsql: Sort the dependent objects before recursing infindDependentObje (Tom Lane <tgl@sss.pgh.pa.us>) |
Responses |
Re: pgsql: Sort the dependent objects before recursing in findDependentObje
|
List | pgsql-committers |
On 2019-01-21 18:48:25 +0000, Tom Lane wrote: > Sort the dependent objects before recursing in findDependentObjects(). > > Historically, the notices output by DROP CASCADE tended to come out > in uncertain order, and in some cases you might get different claims > about which object depends on which other one. This is because we > just traversed the dependency tree in the order in which pg_depend > entries are seen, and nbtree has never promised anything about the > order of equal-keyed index entries. We've put up with that for years, > hacking regression tests when necessary to prevent them from emitting > unstable output. However, it's a problem for pending work that will > change nbtree's behavior for equal keys, as that causes unexpected > changes in the regression test results. > > Hence, adjust findDependentObjects to sort the results of each > indexscan before processing them. The sort is on descending OID of > the dependent objects, hence more or less reverse creation order. > While this rule could still result in bogus regression test failures > if an OID wraparound occurred mid-test, that seems unlikely to happen > in any plausible development or packaging-test scenario. > > This is enough to ensure output stability for ordinary DROP CASCADE > commands, but not for DROP OWNED BY, because that has a different > code path with the same problem. We might later choose to sort in > the DROP OWNED BY code as well, but this patch doesn't do so. > > I've also not done anything about reverting the existing hacks to > suppress 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 designed > on the assumption that there could be at most one "owning" object > to complain about, and that assumption does not seem unreasonable. > The recent patch that added the possibility of multiple > DEPENDENCY_INTERNAL_AUTO links broke that assumption, but we should > fix that situation not band-aid around it. That's a matter for > another patch, though. Seems this affects the sepgsql tests: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=rhinoceros&dt=2019-01-21%2019%3A45%3A02 --- /opt/src/pgsql-git/build-farm-root/HEAD/pgsql.build/contrib/sepgsql/expected/ddl.out 2018-11-20 21:45:02.786933457-0800 +++ /opt/src/pgsql-git/build-farm-root/HEAD/pgsql.build/contrib/sepgsql/results/ddl.out 2019-01-21 11:51:23.178651387-0800 @@ -421,10 +421,10 @@ LOG: SELinux: allowed { drop } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_table_t:s0tclass=db_column name="regtest_schema.regtest_ptable.q" DROP TABLE regtest_table; LOG: SELinux: allowed { remove_name } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_schema_t:s0tclass=db_schema name="regtest_schema" -LOG: SELinux: allowed { drop } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_seq_t:s0tclass=db_sequence name="regtest_schema.regtest_table_x_seq" -LOG: SELinux: allowed { remove_name } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_schema_t:s0tclass=db_schema name="regtest_schema" LOG: SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_table_t:s0tclass=db_table name="regtest_schema.regtest_table" LOG: SELinux: allowed { remove_name } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_schema_t:s0tclass=db_schema name="regtest_schema" +LOG: SELinux: allowed { drop } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_seq_t:s0tclass=db_sequence name="regtest_schema.regtest_table_x_seq" +LOG: SELinux: allowed { remove_name } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_schema_t:s0tclass=db_schema name="regtest_schema" LOG: SELinux: allowed { drop } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_table_t:s0tclass=db_table name="regtest_schema.regtest_table" LOG: SELinux: allowed { drop } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_table_t:s0tclass=db_column name="regtest_schema.regtest_table.tableoid" LOG: SELinux: allowed { drop } scontext=unconfined_u:unconfined_r:sepgsql_regtest_superuser_t:s0 tcontext=unconfined_u:object_r:sepgsql_table_t:s0tclass=db_column name="regtest_schema.regtest_table.cmax" Yay for having to fix tests blindly :( Greetings, Andres Freund
pgsql-committers by date: