Thread

Commits

  1. Remove redundant AssertVariableIsOfType uses

  2. Remove uses of AssertVariableIsOfType() obsoleted by f2b73c8

  1. Remove redundant AssertVariableIsOfType uses in pg_upgrade

    Peter Eisentraut <peter@eisentraut.org> — 2026-01-20T10:07:47Z

    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.
  2. Re: Remove redundant AssertVariableIsOfType uses in pg_upgrade

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-01-20T14:56:38Z

    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
    
    
    
    
  3. Re: Remove redundant AssertVariableIsOfType uses in pg_upgrade

    Nathan Bossart <nathandbossart@gmail.com> — 2026-01-20T22:19:22Z

    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
    
    
    
    
  4. Re: Remove redundant AssertVariableIsOfType uses in pg_upgrade

    Peter Eisentraut <peter@eisentraut.org> — 2026-01-22T11:21:28Z

    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.