Re: Avoid orphaned objects dependencies, take 3
Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Jeff Davis <pgsql@j-davis.com>, Roman Eskin <r.eskin@arenadata.io>, Michael Paquier <michael@paquier.xyz>, Alexander Lakhin <exclusion@gmail.com>, pgsql-hackers@lists.postgresql.org, Tom Lane <tgl@sss.pgh.pa.us>
Date: 2026-05-18T12:14:34Z
Lists: pgsql-hackers
Attachments
- v21-0001-Avoid-orphaned-objects-dependencies.patch (text/x-diff)
Hi, On Wed, May 13, 2026 at 04:20:21PM -0400, Robert Haas wrote: > On Tue, Apr 28, 2026 at 7:17 AM Bertrand Drouvot > <bertranddrouvot.pg@gmail.com> wrote: > > 0003: Add Assert guard to detect permission check before lock regressions > > > > Add instrumentation under USE_ASSERT_CHECKING to detect cases where object_aclcheck() > > is called on a referenced object before a lock is held on it, which would widen > > the TOCTOU window between the permission check and the dependency recording. > > I really like the idea of having some kind of cross-check system that > can detect future (or current) coding mistakes. Thanks for the feedback! BTW, it detected a new one due to 4793fc41f82, so v21 attached does fix it to make the CI green. > But what I wonder > about this mechanism is: should we instead be insisting that we take a > lock and check permissions on every dependency? Is it an error to > record a dependency on an object without any sort of permissions > check? I'm not sure. For example, currently, without execute privilege on myfunc(), one could create a view like: CREATE VIEW v1 AS SELECT myfunc(a) FROM t1; so that the dependency is recorded. The execution permission is checked when the view is queried. I don't think this example is a bug, so that I'm doubtful about insisting that we take a lock and check permissions on every dependency. > Also, I think the mechanism might not be entirely safe. ProcessUtility > can result in executing user-defined functions which could > theoretically run other DDL and then it seems like this code would get > confused. The assert fires when an aclcheck was tracked and the lock is not held. Since the locks are transaction-scoped, any lock acquired during the statement persists, so the assert passes regardless of nesting. So that looks not possible to get false positives (assert fires) due to user-defined functions running other DDL. The concern would be false negatives (missed detection) due to the reset wiping the outer DDL's entries. I believe, that's theoretically possible only if: - The outer DDL has a P1 bug (aclcheck without lock on object X) - A user-defined function happens to run DDL that also aclchecks and locks the same object X - The user-defined function DDL's lock satisfies the assert for the outer DDL But then, the bug would be caught in normal testing without the user-defined function being present. So I'm not sure how this code could get confused. Do you have an example of what you have in mind? Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com