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
</messages/by-id/20240516211638.GA1688936@nathanxps13>
I found that this pattern was introduced between patch versions v9 and
v10, but there was no further explanation.
Attachments:
0001-Remove-redundant-AssertVariableIsOfType-uses.patchtext/plain; charset=UTF-8; name=0001-Remove-redundant-AssertVariableIsOfType-uses.patchDownload+0-33
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.