pgsql: Sort the dependent objects before recursing in findDependentObje
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.
Discussion: /messages/by-id/12244.1547854440@sss.pgh.pa.us
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/f1ad067fc3ae3d34dc6b8826913d454cc3fe354f
Modified Files
--------------
src/backend/catalog/dependency.c | 100 ++++++++++++++++++++++----
src/test/regress/expected/alter_table.out | 2 +-
src/test/regress/expected/create_type.out | 8 +--
src/test/regress/expected/domain.out | 4 +-
src/test/regress/expected/matview.out | 8 +--
src/test/regress/expected/updatable_views.out | 4 +-
6 files changed, 98 insertions(+), 28 deletions(-)
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:s0 tclass=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:s0 tclass=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:s0 tclass=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:s0 tclass=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:s0 tclass=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:s0 tclass=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:s0 tclass=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:s0 tclass=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:s0 tclass=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:s0 tclass=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:s0 tclass=db_column name="regtest_schema.regtest_table.cmax"Yay for having to fix tests blindly :(
Greetings,
Andres Freund
Andres Freund <andres@anarazel.de> writes:
On 2019-01-21 18:48:25 +0000, Tom Lane wrote:
Sort the dependent objects before recursing in findDependentObjects().
Seems this affects the sepgsql tests:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=rhinoceros&dt=2019-01-21%2019%3A45%3A02
Hah ... I'd just looked at that and guessed that *you* broke it
with the pluggable-storage changes. But yeah, it looks more like
an order-of-messages issue. Will fix.
Yay for having to fix tests blindly :(
I can do better than that, got a Fedora installation right
over there.
regards, tom lane
Hi,
On 2019-01-21 15:16:59 -0500, Tom Lane wrote:
Andres Freund <andres@anarazel.de> writes:
On 2019-01-21 18:48:25 +0000, Tom Lane wrote:
Sort the dependent objects before recursing in findDependentObjects().
Seems this affects the sepgsql tests:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=rhinoceros&dt=2019-01-21%2019%3A45%3A02Hah ... I'd just looked at that and guessed that *you* broke it
with the pluggable-storage changes. But yeah, it looks more like
an order-of-messages issue. Will fix.
I'm sure I'll break something down the line, but the stuff I pushed so
far is pretty darn boring changes...
Yay for having to fix tests blindly :(
I can do better than that, got a Fedora installation right
over there.
Oh, I just never got around to do the selinux stuff necessary to get
sepgsql working...
Greetings,
Andres Freund
Hi,
On 2019-01-21 15:16:59 -0500, Tom Lane wrote:
Andres Freund <andres@anarazel.de> writes:
On 2019-01-21 18:48:25 +0000, Tom Lane wrote:
Sort the dependent objects before recursing in findDependentObjects().
Seems this affects the sepgsql tests:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=rhinoceros&dt=2019-01-21%2019%3A45%3A02Hah ... I'd just looked at that and guessed that *you* broke it
with the pluggable-storage changes. But yeah, it looks more like
an order-of-messages issue. Will fix.
I'm sure I'll break something down the line, but the stuff I pushed so
far is pretty darn boring changes...
Yay for having to fix tests blindly :(
I can do better than that, got a Fedora installation right
over there.
Oh, I just never got around to do the selinux stuff necessary to get
sepgsql working...
Greetings,
Andres Freund