On Thu, Nov 02, 2023 at 04:35:26PM +1100, Peter Smith wrote: > The chance of being able to do so should be small as pg_upgrade uses its > own port and unix domain directory (customizable as well with > --socketdir), but just preventing the launcher to start is safer at the > end, because we are then sure that no changes would ever be applied. > ~ > "safer at the end" (??)
Well, just safer. > 2a. > The comment is one big long sentence. IMO it will be better to break it up. > 2b. > Add a blank line between this comment note and the previous one. Yes, I found that equally confusing when looking at this patch, so I've edited the patch this way when I was looking at it today. This is enough to do the job, so I have applied it for now, before moving on with the second one of this thread. > 2c. > In a recent similar thread [1], they chose to implement a guc_hook to > prevent a user from overriding this via the command line option during > the upgrade. Shouldn't this patch do the same thing, for consistency? > 2d. > If you do implement such a guc_hook (per #2c above), then should the > patch also include a test case for getting an ERROR if the user tries > to override that GUC? Yeah, that may be something to do, but I am not sure that it is worth complicating the backend code for the remote case where one enforces an option while we are already setting a GUC in the upgrade path: https://www.postgresql.org/message-id/CAA4eK1Lh9J5VLypSQugkdD+H=_5-6p3roocjo7jbtogcxa2...@mail.gmail.com That feels like a lot of extra facility for cases that should never happen. -- Michael
signature.asc
Description: PGP signature