pg_wrapper doesn't work when -p is used

[D J Gardner]

Release: 16.04

pg_wrapper currently relies upon either run-time-supplied data or fixed files which can become out of date to select the correct version of clients.
Alternatively it chooses (potentially incompatible) versions of installed programs based on which cluster is using port 5432, even when a different port is in use, despite code intended to default to the newest version.

The problem is that the call to user_cluster_map() returns the default port, and thus the
condition just below this comment is not executed:

# if we only have a port, but no version here, use the latest version
# TODO: this could be improved by better argument parsing and mapping back the
# port to a cluster version/name
if (!$version and $port_specified) {
    $version = get_newest_version;
}

E.g. (here shown for an old system which has 9.1 on port 5432, and 9.3 on port 5433, similar results have been obtained on more up to date systems):
pg_dump -p 5433
pg_dump: server version: 9.3.24; pg_dump version: 9.1.24
pg_dump: aborting because of server version mismatch

Observed behaviour, ignore port value and newest version and instead choose version from port 5432.
Expected behaviour: (as per code below comment) choose version 9.3

The supplied patch implements the TODO: above.
If a port (and no host other than localhost) has been specified, it interrogates the configuration files to identify which cluster and version is listening on the specified port (via -p, --port or PGPORT environment variable) and selects those in the case that a port has been specified.

NB. the patch will probably not apply cleanly to the current version in debian/sid, as some reworking of that has happened. Looking at that code, however, the problem addressed here has not gone away: user_cluster_map() still returns a port while the code in pg_wrapper expects it not to.

[Ubuntu Foundations Team Bug Bot (crichton)]

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.]

[Robie Basak (racb)]

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.

[D J Gardner (djgardner)]

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...

[Robie Basak (racb)]

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.

[Robie Basak (racb)]

> 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)

[Martin Pitt (pitti)]

@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

[Robie Basak (racb)]

Thanks Martin!

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

[D J Gardner (djgardner)]

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

[Athos Ribeiro (athos-ribeiro)]

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

[Sergio Durigan Junior (sergiodj)]

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.

[Utkarsh Gupta (utkarsh)]

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