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: Michael Paquier <michael@paquier.xyz>, Alexander Lakhin <exclusion@gmail.com>, pgsql-hackers@lists.postgresql.org, Tom Lane <tgl@sss.pgh.pa.us>
Date: 2024-06-17T10:50:56Z
Lists: pgsql-hackers
Attachments
- v10-0001-Avoid-orphaned-objects-dependencies.patch (text/x-diff)
Hi, On Thu, Jun 13, 2024 at 04:52:09PM +0000, Bertrand Drouvot wrote: > Hi, > > On Thu, Jun 13, 2024 at 10:49:34AM -0400, Robert Haas wrote: > > On Fri, Jun 7, 2024 at 4:41 AM Bertrand Drouvot > > <bertranddrouvot.pg@gmail.com> wrote: > > > Do you still find the code hard to maintain with v9? > > > > I don't think it substantially changes my concerns as compared with > > the earlier version. > > Thanks for the feedback, I'll give it more thoughts. Please find attached v10 that puts the object locking outside of the dependency code. It's done that way except for: recordDependencyOnExpr() recordDependencyOnSingleRelExpr() makeConfigurationDependencies() The reason is that I think that it would need part of the logic that his inside the above functions to be duplicated and I'm not sure that's worth it. For example, we would probably need to: - make additional calls to find_expr_references_walker() - make additional scan on the config map It's also not done outside of recordDependencyOnCurrentExtension() as: 1. I think it is clear enough that way (as it is clear that the lock is taken on a ExtensionRelationId object class). 2. why to include "commands/extension.h" in more places (locking would depend of "creating_extension" and "CurrentExtensionObject"), while 1.? Remarks: 1. depLockAndCheckObject() and friends in v9 have been renamed to LockNotPinnedObject() and friends (as the vast majority of their calls are now done outside of the dependency code). 2. regarding the concern around RelationRelationId (discussed in [1]), v10 adds a comment "XXX Do we need a lock for RelationRelationId?" at the places we may want to lock this object class. I did not think about it yet (but will do), I only added this comment at multiple places. I think that v10 is easier to follow (as compare to v9) as we can easily see for which object class we'll put a lock on. Thoughts? [1]: https://www.postgresql.org/message-id/Zmv3TPfJAyQXhIdu%40ip-10-97-1-34.eu-west-3.compute.internal Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Avoid orphaned objects dependencies
- 2fbb21170e90 19 (unreleased) landed
- 5100bdbd3ba2 14 (unreleased) landed
- 5fa137727db0 15 (unreleased) landed
- d9bc0d96c247 16 (unreleased) landed
- 3a9909eda207 17 (unreleased) landed
- c8cd3d6976f7 18 (unreleased) landed
-
Don't try to record dependency on a dropped column's datatype
- fd93ee100830 19 (unreleased) landed
- 36b6ed2606e1 14 (unreleased) landed
- ef3d7b15e4cd 15 (unreleased) landed
- d44cd467471c 16 (unreleased) landed
- c1588f92a915 17 (unreleased) landed
- f9d5a52da4ca 18 (unreleased) landed