On Mon, Feb 17, 2025 at 11:46:43AM -0800, Jeff Davis wrote:
> Attached a version that rebases both patches. In my patch, I added a
> report_status().
I briefly looked at v2-0002, and the UpgradeTask usage looks correct to me.
Did you find it easy enough to use?
+ /*
+ * 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.
+ report_status(PG_WARNING, "warning");
+ pg_log(PG_WARNING, "Your installation contains relations that may be affected by a new version of Unicode.\n"
+ "A list of potentially-affected relations is in the file:\n"
+ " %s", report.path);
This may have been discussed upthread, but is a warning enough? That seems
like something that could very easily be missed.
--
nathan
