Обсуждение: Remove redundant AssertVariableIsOfType uses in pg_upgrade

pg_upgrade code contains a number of lines like

     AssertVariableIsOfType(&process_rel_infos, UpgradeTaskProcessCB);

This is presumably to ensure that the function signature is fitting for 
being used with upgrade_task_add_step().  But the signature of 
upgrade_task_add_step() already checks that itself, so these additional 
assertions are redundant, and I find them confusing.  So I propose to 
remove them.

In the original thread 
<https://www.postgresql.org/message-id/flat/20240516211638.GA1688936%40nathanxps13> 
I found that this pattern was introduced between patch versions v9 and 
v10, but there was no further explanation.

Hi,

On Tue, Jan 20, 2026 at 11:07:47AM +0100, Peter Eisentraut wrote:
> pg_upgrade code contains a number of lines like
> 
>     AssertVariableIsOfType(&process_rel_infos, UpgradeTaskProcessCB);
> 
> This is presumably to ensure that the function signature is fitting for
> being used with upgrade_task_add_step().  But the signature of
> upgrade_task_add_step() already checks that itself, so these additional
> assertions are redundant, and I find them confusing.  So I propose to remove
> them.

The changes loook good to me. All the functions where the Assert is removed
are used as arguments of upgrade_task_add_step() calls. Plus there is also
process_unicode_update() but there is no Assert to remove, so LGTM.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On Tue, Jan 20, 2026 at 11:07:47AM +0100, Peter Eisentraut wrote:
> pg_upgrade code contains a number of lines like
> 
>     AssertVariableIsOfType(&process_rel_infos, UpgradeTaskProcessCB);
> 
> This is presumably to ensure that the function signature is fitting for
> being used with upgrade_task_add_step().  But the signature of
> upgrade_task_add_step() already checks that itself, so these additional
> assertions are redundant, and I find them confusing.  So I propose to remove
> them.

I think this was borrowed from logical decoding output plugins, but
apparently I wrote the patch to remove these assertions from those, too
(commit 30b789eafe).  So, I'm not sure what I was thinking...  If they are
indeed redundant, I have no objection to their removal.

-- 
nathan

On 20.01.26 23:19, Nathan Bossart wrote:
> On Tue, Jan 20, 2026 at 11:07:47AM +0100, Peter Eisentraut wrote:
>> pg_upgrade code contains a number of lines like
>>
>>      AssertVariableIsOfType(&process_rel_infos, UpgradeTaskProcessCB);
>>
>> This is presumably to ensure that the function signature is fitting for
>> being used with upgrade_task_add_step().  But the signature of
>> upgrade_task_add_step() already checks that itself, so these additional
>> assertions are redundant, and I find them confusing.  So I propose to remove
>> them.
> 
> I think this was borrowed from logical decoding output plugins, but
> apparently I wrote the patch to remove these assertions from those, too
> (commit 30b789eafe).  So, I'm not sure what I was thinking...  If they are
> indeed redundant, I have no objection to their removal.

Ah, that is interesting context.  Anyway, change committed.