Обсуждение: [PATCH] Move user options to the end of the command in pg_upgrade

Hi,

when the environment variable POSTGRESQL_UPGRADE_PGUPGRADE_OPTIONS is
used to specify options for pg_upgrade, options related to
unix_socket_directory/ies are being overridden by hardcoded options,
making it difficult to upgrade in some usecases.

The attached patch changes the order of those options so that the
hardcoded ones are eventually overridden by the user specified
options.

As I can see that in PostgreSQL 12 this issue has been solved by
implementing the -socketdir argument, my questions would be as
follows:

1) Could such change break something that I might have missed?
2) Would you be willing to accept this patch for versions prior to 12?

Thanks in advance.

--
Patrik Novotný

Associate Software Engineer

Red Hat

panovotn@redhat.com

Patrik Novotny <panovotn@redhat.com> writes:
> when the environment variable POSTGRESQL_UPGRADE_PGUPGRADE_OPTIONS is
> used to specify options for pg_upgrade, options related to
> unix_socket_directory/ies are being overridden by hardcoded options,
> making it difficult to upgrade in some usecases.

> The attached patch changes the order of those options so that the
> hardcoded ones are eventually overridden by the user specified
> options.

Hi Patrik,

It looks like you forgot to attach the patch?  But in any case,
I see no references to POSTGRESQL_UPGRADE_PGUPGRADE_OPTIONS in
any community Postgres code, so I'm wondering if this is just
changing some script that Red Hat supplies as part of packaging.
That would make it not our concern, really.

            regards, tom lane

Hi Tom,

thanks for you reply. You're right, and I apologise for the confusion.
Options I was talking about are specified via the `--old-options`
parameter of the pg_upgrade (ex.: --old-options '-c
unix_socket_directories=/run')
Mentioning of the environment variable came only from my own
confusion. I also attached the mentioned patch.



Regards,

On Tue, Sep 10, 2019 at 4:13 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Patrik Novotny <panovotn@redhat.com> writes:
> > when the environment variable POSTGRESQL_UPGRADE_PGUPGRADE_OPTIONS is
> > used to specify options for pg_upgrade, options related to
> > unix_socket_directory/ies are being overridden by hardcoded options,
> > making it difficult to upgrade in some usecases.
>
> > The attached patch changes the order of those options so that the
> > hardcoded ones are eventually overridden by the user specified
> > options.
>
> Hi Patrik,
>
> It looks like you forgot to attach the patch?  But in any case,
> I see no references to POSTGRESQL_UPGRADE_PGUPGRADE_OPTIONS in
> any community Postgres code, so I'm wondering if this is just
> changing some script that Red Hat supplies as part of packaging.
> That would make it not our concern, really.
>
>                         regards, tom lane



--

Patrik Novotný

Associate Software Engineer

Red Hat

panovotn@redhat.com

Patrik Novotny <panovotn@redhat.com> writes:
> thanks for you reply. You're right, and I apologise for the confusion.
> Options I was talking about are specified via the `--old-options`
> parameter of the pg_upgrade (ex.: --old-options '-c
> unix_socket_directories=/run')
> Mentioning of the environment variable came only from my own
> confusion. I also attached the mentioned patch.

Ah, now I see what you're on about.  I agree that this is a good
change ... and we probably should add a comment reminding people
to keep the user options at the end, because somebody[1] broke this
through add-at-the-end syndrome.

            regards, tom lane

[1] ... me, in fact

I wrote:
> Patrik Novotny <panovotn@redhat.com> writes:
>> thanks for you reply. You're right, and I apologise for the confusion.
>> Options I was talking about are specified via the `--old-options`
>> parameter of the pg_upgrade (ex.: --old-options '-c
>> unix_socket_directories=/run')
>> Mentioning of the environment variable came only from my own
>> confusion. I also attached the mentioned patch.

> Ah, now I see what you're on about.  I agree that this is a good
> change ... and we probably should add a comment reminding people
> to keep the user options at the end, because somebody[1] broke this
> through add-at-the-end syndrome.

Actually ... now that I look more carefully, I'm not sure this change
would improve matters.  You can't just reach in and select a different
socket directory behind pg_upgrade's back; if you try, the connection
attempts later are going to fail, because pg_upgrade will be telling
pg_dump to use what it thinks the socket directory is.

You might be better off back-patching the addition of the --socketdir
option (commit 2d34ad84303181111c6f0747186857ff50106267).

            regards, tom lane