Message-ID: <3963e573-4852-4d25-b741-7c5c7e1f9140@iki.fi>
Date: Wed, 27 May 2026 18:53:35 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Avoid orphaned objects dependencies, take 3
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, 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>
References: 
 <CA+TgmobxpkwwjfM4EkZxyKpJ3S0CqQPXPkqQn4YmCHBeMDypdQ@mail.gmail.com>
 <aD2u/GR/yaRkBJdJ@ip-10-97-1-34.eu-west-3.compute.internal>
 <CA+TgmoZh8yXoQ2AF-VFSTswhin0o+BZ78AaOsWZskCW1GHBd6g@mail.gmail.com>
 <aEJ+oNANWutG8aDv@ip-10-97-1-34.eu-west-3.compute.internal>
 <9f5f0dd4-96d5-4d90-9436-62d96e0298f9@arenadata.io> <ad/awXLnMuc6Rf5M@bdtpg>
 <afCXPVNuvpf5ECGZ@bdtpg>
 <CA+TgmobtzkG9HzGtDXbgb3KVE46qHuSvGcuKHgAGQ1aejCoLLw@mail.gmail.com>
 <agsCqlLTytZCudMv@bdtpg> <aebf583b-b904-4035-a1c1-a4c5ad064dec@iki.fi>
 <ahYAKgbx05IQdjwR@bdtpg>
Content-Language: en-US
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: <ahYAKgbx05IQdjwR@bdtpg>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/3963e573-4852-4d25-b741-7c5c7e1f9140%40iki.fi>
Precedence: bulk

On 26/05/2026 23:18, Bertrand Drouvot wrote:
> On Tue, May 26, 2026 at 09:00:11PM +0300, Heikki Linnakangas wrote:
>> - Added a test for a dependency on a role too, to cover the existing
>> shdepLockAndCheckObject() function. It's currently disabled because it
>> prints the OID, though.
> 
> Nit: It also adds:
> 
> +# function - role
> +permutation "s1_begin" "s1_alter_function_owner" "s2_drop_role" "s1_commit"
> 
> That is not disabled and would already pass without the changes added in 0001.
> 
> So I wonder if we could just start by adding this new test (and the XXX one) in
> a dedicated patch and then add 0001 that would focus on orphaned stuff only.

True, we could do that. It doesn't really seem to me any better or worse 
than just including it in the same commit though.

> A few comments:
> 
> 1/
> 
> +#include "catalog/index.h"
> 
> That doesn't look needed.
> 
> 2/
> 
> It looks like pgindent "complains" on pg_depend.c for dependencyLockAndCheckObject().

Fixed these, and did some more copy-editing on the comments. With that, 
committed and backpatched. Version 14 needed a little more work to 
backport because it didn't have IsPinnedObject(). And I noticed that 
recordDependencyFor() was being called with InvalidOid on ALTER TABLE 
commands on tables with dropped columns. That was bogus but harmless 
before this patch; I fixed that too.

Thanks!

- Heikki