On Wed, Mar 05, 2025 at 02:33:42PM -0600, Nathan Bossart wrote:
> + /*
> + * The builtin provider did not exist prior to version 17. While there
> are
> + * still problems that could potentially be caught from earlier
> versions,
> + * such as an index on NORMALIZE(), we don't check for that here.
> + */
> + if (GET_MAJOR_VERSION(cluster->major_version) < 1700)
> + return;
>
> nitpick: In most cases, I think this check is done in
> check_and_dump_old_cluster() before actually calling the checking function.
> I don't think there's any big problem here, except you might strand the
> memory allocated for the task.
>
> + if (!unicode_version_changed(cluster))
> + {
> + check_ok();
> + return;
> + }
>
> Same nitpick here about stranding the task memory.
Coverity is unhappy about these. I think we should at least do something
like the following. I'll commit this when I have an opportunity.
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 8f946c4e3d6..18c2d652bb6 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -1825,7 +1825,7 @@ static void
check_for_unicode_update(ClusterInfo *cluster)
{
UpgradeTaskReport report;
- UpgradeTask *task = upgrade_task_create();
+ UpgradeTask *task;
const char *query;
/*
@@ -1920,6 +1920,7 @@ check_for_unicode_update(ClusterInfo *cluster)
" d.datname = current_database() AND "
" d.encoding = pg_char_to_encoding('UTF8');";
+ task = upgrade_task_create();
upgrade_task_add_step(task, query,
process_unicode_update,
true, &report);
--
nathan
