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-6p3rOocjo7JbTogcxA2hxg@mail.gmail.com
That feels like a lot of extra facility for cases that should never
happen.
--
Michael