pg_wrapper doesn't work when -p is used

The attachment "port-based-pgversion.patch" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

Thank you for taking the time to report this bug and helping to make Ubuntu better.

Am I right in thinking that your problem is "If a system has a newer version concurrently installed on a non-default port, then pg_wrapper fails to automatically select the newer client when -p is specified"?

I don't see anything in the documentation of pg_wrapper that says it'll pay attention to -p specified manually. So is this a feature request (with patch)?

> NB. the patch will probably not apply cleanly to the current version in debian/sid

Please note that we won't update stable releases without first updating the development release, since otherwise users upgrading to the next release will subsequently find a regression. If your patch does pass review etc, it will still be blocked from landing pending a corresponding patch to the development release. So it's best to start with testing/fixing the development release, get that landed, and then backport what is required.

There is a mismatch in behaviour of a library function (returns cluster on
default port) and what the program expects (null value for port if no
cluster was selected)

In some circumstances, (-p specified and older version listening on default
port ) this leads to the incorrect binary being run, despite comments in
the (program) code saying it should be the most recent binary that is
selected.

The patch avoids that bug by implementing a TODO: note in the code.

My reference to sid was because I saw that some variable names had been
changed. I do not currently have a test machine to see which of the 2
versions I have seen is the one in testing.

David

Sent from my phone. Please excuse any errors

On Tue, 9 Oct 2018, 15:01 Robie Basak, <email address hidden> wrote:

> Thank you for taking the time to report this bug and helping to make
> Ubuntu better.
>
> Am I right in thinking that your problem is "If a system has a newer
> version concurrently installed on a non-default port, then pg_wrapper
> fails to automatically select the newer client when -p is specified"?
>
> I don't see anything in the documentation of pg_wrapper that says it'll
> pay attention to -p specified manually. So is this a feature request
> (with patch)?
>
> > NB. the patch will probably not apply cleanly to the current version
> in debian/sid
>
> Please note that we won't update stable releases without first updating
> the development release, since otherwise users upgrading to the next
> release will subsequently find a regression. If your patch does pass
> review etc, it will still be blocked from landing pending a
> corresponding patch to the development release. So it's best to start
> with testing/fixing the development release, get that landed, and then
> backport what is required.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1796407
>
> Title:
> patch: make pg_wrapper select correct version for local cluster, based
> on port
>
> To manage notifications about this bug go to:
>
> https://bugs.launchpad.net/ubuntu/+source/postgresql-common/+bug/1796407/+subscriptions
>

On 9 Oct 2018 3:01 pm, "Robie Basak" <email address hidden> wrote:

Thank you for taking the time to report this bug and helping to make
Ubuntu better.

Am I right in thinking that your problem is "If a system has a newer
version concurrently installed on a non-default port, then pg_wrapper
fails to automatically select the newer client when -p is specified"?

I don't see anything in the documentation of pg_wrapper that says it'll
pay attention to -p specified manually. So is this a feature request
(with patch)?

> NB. the patch will probably not apply cleanly to the current version
in debian/sid

Please note that we won't update stable releases without first updating
the development release, since otherwise users upgrading to the next
release will subsequently find a regression. If your patch does pass
review etc, it will still be blocked from landing pending a
corresponding patch to the development release. So it's best to start
with testing/fixing the development release, get that landed, and then
backp...

OK, thanks.

Since this isn't a bug in the documented behaviour of pg_wrapper from a user perspective, I'm going to treat this as a new feature ("detect -p in the command line and use it to work out which binary to run") for now, unless someone wants to explain why this interpretation is wrong.

From that perspective, I'm not sure this is suitable for SRU, so it will need to go to the development release first. Unfortunately since the patch also doesn't apply to the version currently in the development release, it is not actionable right now.

If you could update the patch to be applicable to Debian sid, then we could submit the patch there, and Ubuntu will in time receive the update (if you could test/submit against Debian directly, that would be great).

In the meantime I think this bug itself is "pg_wrapper doesn't work when -p is used" and so I'll rename the bug and triage it accordingly.

I'll also see if I can contact Martin, the original author of pg_wrapper (AFAICT) to see if he has any comment.

> From that perspective, I'm not sure this is suitable for SRU, so it will need to go to the development release first.

(actually it'd have to go in the development release first anyway, but I do think it changes views on priorities somewhat)

@Robie: It's been a while since I dabbled with this, but to me this is not really "wishlist", it's an actual bug. Surely pg_wrapper doesn't document that it looks at the explicitly specified port, but it certainly ought to. This case was just plain forgotten.

So the patch certainly needs some massaging, at least:

 - adjust it to current version [1], which e. g. has $explicit_port instead of $port_specified
 - Fix the "locahost" typo, and support IP addresses as well (127.0.0.* or ::1)
 - Add an integration test to https://salsa.debian.org/postgresql/postgresql-common/blob/master/t/090_multicluster.t

but the general heading of it seems fine to me.

[1] https://salsa.debian.org/postgresql/postgresql-common/blob/master/pg_wrapper

Thanks Martin!

@D J Gardner I hope that gives you a clear path forward?

I'll revise the patch when I have time... may have an hour /two this weekend.
But I have never done anything using the test thing though.. there'd
be quite a learning curve to do that, I think.
On Thu, 18 Oct 2018 at 12:55, Robie Basak <email address hidden> wrote:
>
> Thanks Martin!
>
> @D J Gardner I hope that gives you a clear path forward?
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1796407
>
> Title:
> pg_wrapper doesn't work when -p is used
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/ubuntu/+source/postgresql-common/+bug/1796407/+subscriptions

This is still valid for bionic and focal. It is possibly reproducible in jammy as well, but since we do not have an official newer version in to test in jammy, we cannot easily get a higher version to get a simple reproducer.

Steps to reproduce:

From a bionic/focal vm:

# apt update && apt install -y docker.io postgresql postgresql-common postgresql-client-common
# docker run -d --name postgres-13 -p 5433:5432 -e POSTGRES_PASSWORD=postgres ubuntu/postgres:13-21.10_beta
# pg_dump -h localhost --user postgres -p 5433
pg_dump: server version: 13.5 (Ubuntu 13.5-0ubuntu0.21.10.1); pg_dump version: 10.19 (Ubuntu 10.19-0ubuntu0.18.04.1)
pg_dump: aborting because of server version mismatch

Added tasks for B/F/J/K, and marked B/F as Triaged according to Athos' comment above. We should verify if the bug still applies to J/K (it probably does) and set the tasks accordingly.

Ubuntu 22.10 (Kinetic Kudu) has reached end of life, so this bug will not be fixed for that specific release.