Thread: Package version in PG_VERSION and version()

Package version in PG_VERSION and version()

From
Christoph Berg
Date:
To be able to identify more easily which package a connected server is
coming from, I would like to embed the (Debian) package version in the
version() output which is coming from PG_VERSION. It is fairly easy to
do that, but it requires patching configure(.in):

$ ./configure VENDOR_VERSION="Debian 10.1-2"

# select version();
                                                      version
══════════════════════════════════════════════════════════════════════════════════════════════════════════════════
 PostgreSQL 10.1 on x86_64-pc-linux-gnu (Debian 10.1-2), compiled by gcc (Debian 7.2.0-17) 7.2.1 20171205, 64-bit

PoC patch against HEAD attached - if the approach is deemed
acceptable, I'll also update the relevant documentation bits.

Thanks,
Christoph

Attachment

Re: Package version in PG_VERSION and version()

From
Michael Paquier
Date:
On Fri, Dec 15, 2017 at 7:46 PM, Christoph Berg
<christoph.berg@credativ.de> wrote:
> To be able to identify more easily which package a connected server is
> coming from, I would like to embed the (Debian) package version in the
> version() output which is coming from PG_VERSION. It is fairly easy to
> do that, but it requires patching configure(.in):

Why reinventing the wheel when there is already --with-extra-version
that you can use for the same purpose? And I think that I can see a
bug here on HEAD, src/tools/msvc/Solution.pm correctly uses
--with-extra-version in PG_VERSION_STR but ./configure is forgetting
it. If you want to push for this proposal anyway, I think that you
should update the msvc scripts as well.
-- 
Michael


Re: Package version in PG_VERSION and version()

From
Christoph Berg
Date:
Re: Michael Paquier 2017-12-15 <CAB7nPqTra6ZkPr0xTmHY0J4gmKwbStbMmaKMa9Kswb2bZxe=yw@mail.gmail.com>
> On Fri, Dec 15, 2017 at 7:46 PM, Christoph Berg
> <christoph.berg@credativ.de> wrote:
> > To be able to identify more easily which package a connected server is
> > coming from, I would like to embed the (Debian) package version in the
> > version() output which is coming from PG_VERSION. It is fairly easy to
> > do that, but it requires patching configure(.in):
> 
> Why reinventing the wheel when there is already --with-extra-version
> that you can use for the same purpose?

That modifies the PG version number as such, as what psql is showing
on connect. I'd think that is too intrusive.

And it doesn't work anyway: $ ./configure --with-extra-version ' (Debian 10.1-2)'
configure: WARNING: you should use --build, --host, --target
configure: WARNING: invalid host type:  (Debian 10.1-2)
configure: error: argument required for --with-extra-version option

> And I think that I can see a
> bug here on HEAD, src/tools/msvc/Solution.pm correctly uses
> --with-extra-version in PG_VERSION_STR but ./configure is forgetting
> it. If you want to push for this proposal anyway, I think that you
> should update the msvc scripts as well.

configure.in looks right, it includes the extra version right in
PG_VERSION:

PGAC_ARG_REQ(with, extra-version, [STRING], [append STRING to version],
             [PG_VERSION="$PACKAGE_VERSION$withval"],
             [PG_VERSION="$PACKAGE_VERSION"])
AC_DEFINE_UNQUOTED(PG_VERSION, "$PG_VERSION", [PostgreSQL version as a string])

AC_DEFINE_UNQUOTED(PG_VERSION_STR,
                   ["PostgreSQL $PG_VERSION on $host, compiled by $cc_string, `expr $ac_cv_sizeof_void_p \* 8`-bit"],
                   [A string containing the version number, platform, and C compiler])

I'll update the msvc scripts once we figure out where my idea of
"vendor package version" is best placed.

Mit freundlichen Grüßen,
Christoph Berg
-- 
Senior Berater, Tel.: +49 2166 9901 187
credativ GmbH, HRB Mönchengladbach 12080, USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer
pgp fingerprint: 5C48 FE61 57F4 9179 5970  87C6 4C5A 6BAB 12D2 A7AE


Re: Package version in PG_VERSION and version()

From
Tom Lane
Date:
Christoph Berg <christoph.berg@credativ.de> writes:
> Re: Michael Paquier 2017-12-15 <CAB7nPqTra6ZkPr0xTmHY0J4gmKwbStbMmaKMa9Kswb2bZxe=yw@mail.gmail.com>
>> Why reinventing the wheel when there is already --with-extra-version
>> that you can use for the same purpose?

> That modifies the PG version number as such, as what psql is showing
> on connect. I'd think that is too intrusive.

I'm really pretty much -1 on having two different ways to do very nearly
the same thing, with the differences determined only by somebody's
arbitrary choices of where they think the modified version should be
exposed.  IMO, either you think the Debian package version is important
enough to show, or you don't.  (I'd incline to the "don't" side anyway.)

            regards, tom lane


Re: Package version in PG_VERSION and version()

From
Robert Haas
Date:
On Fri, Dec 15, 2017 at 10:23 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Christoph Berg <christoph.berg@credativ.de> writes:
>> Re: Michael Paquier 2017-12-15 <CAB7nPqTra6ZkPr0xTmHY0J4gmKwbStbMmaKMa9Kswb2bZxe=yw@mail.gmail.com>
>>> Why reinventing the wheel when there is already --with-extra-version
>>> that you can use for the same purpose?
>
>> That modifies the PG version number as such, as what psql is showing
>> on connect. I'd think that is too intrusive.
>
> I'm really pretty much -1 on having two different ways to do very nearly
> the same thing, with the differences determined only by somebody's
> arbitrary choices of where they think the modified version should be
> exposed.  IMO, either you think the Debian package version is important
> enough to show, or you don't.  (I'd incline to the "don't" side anyway.)

Unfortunately, actually modifying the main version number breaks large
numbers of tools and drivers that think they know what a PostgreSQL
version number looks like, as many people who work for my employer can
testify to from personal experience with a piece of software that
displays a non-default version number.  I think --with-extra-version
is therefore badly-designed and probably mostly useless in its current
form, and as Christoph's example shows, it's not really adapted for
the kind of string he wants to add.  I don't really care whether we
leave --with-extra-version as-is and add something else for the kind
of thing Christoph wants to do, or whether we add a different thing
that does what he wants to do, but I think it's a very good idea to
provide something along the lines of what he wants.

In short, "the version number is important enough to show" != "the
version number is important enough to break compatibility with large
numbers of tools and drivers".

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: Package version in PG_VERSION and version()

From
Christoph Berg
Date:
Re: Robert Haas 2017-12-17 <CA+Tgmoan5jER6OpGWOZZBvOgzQ5SJMpP7e2vvzhgFB4pxC33gw@mail.gmail.com>
> Unfortunately, actually modifying the main version number breaks large
> numbers of tools and drivers that think they know what a PostgreSQL
> version number looks like, as many people who work for my employer can
> testify to from personal experience with a piece of software that
> displays a non-default version number.  I think --with-extra-version
> is therefore badly-designed and probably mostly useless in its current
> form, and as Christoph's example shows, it's not really adapted for
> the kind of string he wants to add.  I don't really care whether we
> leave --with-extra-version as-is and add something else for the kind
> of thing Christoph wants to do, or whether we add a different thing
> that does what he wants to do, but I think it's a very good idea to
> provide something along the lines of what he wants.

My idea is to put the information in a place where it is accessible,
but not obtrusive. version() seemed to be the only place for that
(server_version is just the short version), and it already has the
compiler version, so it fits, I think.

> In short, "the version number is important enough to show" != "the
> version number is important enough to break compatibility with large
> numbers of tools and drivers".

Exactly.

Added as https://commitfest.postgresql.org/16/1418/ now.

If people think it's worth it (windows?), I'll add a configure
--switch, otherwise with just reading from $VENDOR_VERSION, it's a
one-liner.

Christoph
-- 
Senior Berater, Tel.: +49 2166 9901 187
credativ GmbH, HRB Mönchengladbach 12080, USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer
pgp fingerprint: 5C48 FE61 57F4 9179 5970  87C6 4C5A 6BAB 12D2 A7AE


Re: Package version in PG_VERSION and version()

From
Peter Eisentraut
Date:
On 12/15/17 06:53, Christoph Berg wrote:
>> Why reinventing the wheel when there is already --with-extra-version
>> that you can use for the same purpose?
> That modifies the PG version number as such, as what psql is showing
> on connect. I'd think that is too intrusive.
> 
> And it doesn't work anyway: $ ./configure --with-extra-version ' (Debian 10.1-2)'
> configure: WARNING: you should use --build, --host, --target
> configure: WARNING: invalid host type:  (Debian 10.1-2)
> configure: error: argument required for --with-extra-version option

I think --with-extra-version would do exactly the right thing for you:

./configure --with-extra-version=' (FooNix 1.2.3)'
make
make install

$ psql --version
psql (PostgreSQL) 11devel (FooNix 1.2.3)

$ psql
psql (11devel (FooNix 1.2.3))
Type "help" for help.

=# select version();
PostgreSQL 11devel (FooNix 1.2.3) on ..., compiled by ...

=# show server_version;
11devel (FooNix 1.2.3)

=# show server_version_num;
110000

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: Package version in PG_VERSION and version()

From
Craig Ringer
Date:
On 3 January 2018 at 00:53, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:
On 12/15/17 06:53, Christoph Berg wrote:
>> Why reinventing the wheel when there is already --with-extra-version
>> that you can use for the same purpose?
> That modifies the PG version number as such, as what psql is showing
> on connect. I'd think that is too intrusive.
>
> And it doesn't work anyway: $ ./configure --with-extra-version ' (Debian 10.1-2)'
> configure: WARNING: you should use --build, --host, --target
> configure: WARNING: invalid host type:  (Debian 10.1-2)
> configure: error: argument required for --with-extra-version option

I think --with-extra-version would do exactly the right thing for you:

./configure --with-extra-version=' (FooNix 1.2.3)'
make
make install

$ psql --version
psql (PostgreSQL) 11devel (FooNix 1.2.3)

$ psql
psql (11devel (FooNix 1.2.3))
Type "help" for help.

=# select version();
PostgreSQL 11devel (FooNix 1.2.3) on ..., compiled by ...

=# show server_version;
11devel (FooNix 1.2.3)

=# show server_version_num;
110000

Last time I tried to actually deploy packages that used --with-extra-version a variety of tools that talk to postgres broke because they choked when parsing the version. Including widely used ones like check_postgres.

These issues are why I've pushed repeatedly to make server_version_num GUC_REPORT, and expose PG_VERSION_NUM in pg_config, without success. I still think it needs doing.

--
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Re: Package version in PG_VERSION and version()

From
Robert Haas
Date:
On Tue, Jan 2, 2018 at 9:20 PM, Craig Ringer <craig@2ndquadrant.com> wrote:
> Last time I tried to actually deploy packages that used --with-extra-version
> a variety of tools that talk to postgres broke because they choked when
> parsing the version. Including widely used ones like check_postgres.

I think there's likely a big difference between --with-extra-version='
(blah)' and --with-extra-version='blah', though.  It does this:

        if ($row->{version} !~ /((\d+\.?\d+)(\w+|\.\d+))/) {
            add_unknown msg('invalid-query', $row->{version});
            next;
        }

        my ($full,$version,$revision) = ($1,$2,$3||'?');
        $revision =~ s/^\.//;

That being said, I'm starting to agree with you that exposing
server_version_num might be a good idea.  Anybody who creates a
modified version of PostgreSQL -- and we have a license that allows
that -- wants some way to indicate their modified version.  If you
change "PostgreSQL" in the version to anything else, or add additional
components to the version number, clients blow up and die.  I'm
guessing that adding "(Aurora 4.5.2)" or "(Advanced Server 11.1)" or
whatever after the main version number, separated by a space, might
work better, but it's still vulnerable to misinterpretation by a
client that is sufficiently-picky about the contents of the version
string.  Reporting a bare integer would fix that problem in a better
way, but of course only if client code is changed to use it, which we
have no way to enforce.

Well, we could enforce it if we broke SELECT version() outright, but
that approach would probably not attract many votes.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Re: Package version in PG_VERSION and version()

From
Peter Eisentraut
Date:
On 1/3/18 10:25, Robert Haas wrote:
> On Tue, Jan 2, 2018 at 9:20 PM, Craig Ringer <craig@2ndquadrant.com> wrote:
>> Last time I tried to actually deploy packages that used --with-extra-version
>> a variety of tools that talk to postgres broke because they choked when
>> parsing the version. Including widely used ones like check_postgres.
> 
> I think there's likely a big difference between --with-extra-version='
> (blah)' and --with-extra-version='blah', though.  It does this:

So what is the next action this thread?  I think --with-extra-version is
the right solution for packagers, so I'm tempted to close this commit
fest item.  There is some speculation that using it could break
third-party tools, but (a) we would need more concrete evidence, (b) we
should fix *that* then, and (c) it's likely unavoidable in general.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: Package version in PG_VERSION and version()

From
Christoph Berg
Date:
Re: Peter Eisentraut 2018-01-17 <f18403d3-278c-a4fa-e1f5-6b9a90ca077c@2ndquadrant.com>
> So what is the next action this thread?  I think --with-extra-version is
> the right solution for packagers, so I'm tempted to close this commit
> fest item.  There is some speculation that using it could break
> third-party tools, but (a) we would need more concrete evidence, (b) we
> should fix *that* then, and (c) it's likely unavoidable in general.

If you think I should use that for the packages in Debian and on
apt.postgresql.org, I can do that. I just fear it will explode
in all sorts of ways...

Christoph
-- 
Senior Berater, Tel.: +49 2166 9901 187
credativ GmbH, HRB Mönchengladbach 12080, USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer
pgp fingerprint: 5C48 FE61 57F4 9179 5970  87C6 4C5A 6BAB 12D2 A7AE


Re: Package version in PG_VERSION and version()

From
Tom Lane
Date:
Christoph Berg <christoph.berg@credativ.de> writes:
> Re: Peter Eisentraut 2018-01-17 <f18403d3-278c-a4fa-e1f5-6b9a90ca077c@2ndquadrant.com>
>> So what is the next action this thread?  I think --with-extra-version is
>> the right solution for packagers, so I'm tempted to close this commit
>> fest item.  There is some speculation that using it could break
>> third-party tools, but (a) we would need more concrete evidence, (b) we
>> should fix *that* then, and (c) it's likely unavoidable in general.

> If you think I should use that for the packages in Debian and on
> apt.postgresql.org, I can do that. I just fear it will explode
> in all sorts of ways...

Well, do that and see ;-).

IMO there's not really any evidence that we need an additional mechanism
in this space.  I don't see any way to get that evidence unless some
packager tries to use the existing mechanism and hits insurmountable
problems.

            regards, tom lane


Re: Package version in PG_VERSION and version()

From
Christoph Berg
Date:
Re: Tom Lane 2018-01-17 <15537.1516200157@sss.pgh.pa.us>
> IMO there's not really any evidence that we need an additional mechanism
> in this space.  I don't see any way to get that evidence unless some
> packager tries to use the existing mechanism and hits insurmountable
> problems.

The problem is that the problems will likely not be in my/our/Debian's
realm, but in anything that uses our packages downstream. E.g. the
"official" Docker images are using our packages. There is no way to
test that external stuff without actually publishing the packages for
production consumption.

Christoph
-- 
Senior Berater, Tel.: +49 2166 9901 187
credativ GmbH, HRB Mönchengladbach 12080, USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer
pgp fingerprint: 5C48 FE61 57F4 9179 5970  87C6 4C5A 6BAB 12D2 A7AE


Re: Package version in PG_VERSION and version()

From
Tom Lane
Date:
Christoph Berg <christoph.berg@credativ.de> writes:
> Re: Tom Lane 2018-01-17 <15537.1516200157@sss.pgh.pa.us>
>> IMO there's not really any evidence that we need an additional mechanism
>> in this space.  I don't see any way to get that evidence unless some
>> packager tries to use the existing mechanism and hits insurmountable
>> problems.

> The problem is that the problems will likely not be in my/our/Debian's
> realm, but in anything that uses our packages downstream. E.g. the
> "official" Docker images are using our packages. There is no way to
> test that external stuff without actually publishing the packages for
> production consumption.

Yeah, but the same argument could be made against the variant
you're proposing.  In theory, people could have written arbitrarily
brittle checks of version numbers/strings.  I'm not exactly convinced
that it's your (or our) problem if they did.

            regards, tom lane


Re: Package version in PG_VERSION and version()

From
Tom Lane
Date:
I wrote:
> Yeah, but the same argument could be made against the variant
> you're proposing.  In theory, people could have written arbitrarily
> brittle checks of version numbers/strings.  I'm not exactly convinced
> that it's your (or our) problem if they did.

BTW, as concrete evidence in this area, we could look to what happened
when we changed from three-part to two-part version numbers.  Which
was pretty much nothing.  I've been pleasantly surprised by how little
whining we've heard about that ;-).  I think if downstream users have
been able to survive the change from "x.y.z" to "x.y", they can probably
manage "x.y (Debian something)".  Maybe if you want to be careful, you
could make the addition only in PG 10 and up, guessing that anybody
who's really brittle in this area will be forced to improve their code
when they go to 10 anyway.

            regards, tom lane


Re: Package version in PG_VERSION and version()

From
Christoph Berg
Date:
Re: Tom Lane 2018-01-17 <16522.1516201388@sss.pgh.pa.us>
> I wrote:
> > Yeah, but the same argument could be made against the variant
> > you're proposing.  In theory, people could have written arbitrarily
> > brittle checks of version numbers/strings.  I'm not exactly convinced
> > that it's your (or our) problem if they did.

The difference is that when parsing version() (which is all my variant
is changing), people already have to deal with extra stuff at the end
(gcc version), while that would be new for "psql --version".

> BTW, as concrete evidence in this area, we could look to what happened
> when we changed from three-part to two-part version numbers.  Which
> was pretty much nothing.  I've been pleasantly surprised by how little
> whining we've heard about that ;-).  I think if downstream users have
> been able to survive the change from "x.y.z" to "x.y", they can probably
> manage "x.y (Debian something)".

I guess people just fixed it instead of whining. I'd want to avoid
both :)

> Maybe if you want to be careful, you
> could make the addition only in PG 10 and up, guessing that anybody
> who's really brittle in this area will be forced to improve their code
> when they go to 10 anyway.

That is an excellent idea.

Christoph
-- 
Senior Berater, Tel.: +49 2166 9901 187
credativ GmbH, HRB Mönchengladbach 12080, USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer
pgp fingerprint: 5C48 FE61 57F4 9179 5970  87C6 4C5A 6BAB 12D2 A7AE


Re: Package version in PG_VERSION and version()

From
Peter Eisentraut
Date:
On 1/17/18 10:14, Christoph Berg wrote:
> The difference is that when parsing version() (which is all my variant
> is changing), people already have to deal with extra stuff at the end
> (gcc version), while that would be new for "psql --version".

For me, having the package identifier in "psql -version" specifically
would actually be the biggest win in this, because that's where a lot of
issues arise (e.g., wrong libedit/readline library, different
compiled-in socket location).

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: Package version in PG_VERSION and version()

From
Christoph Berg
Date:
Re: Peter Eisentraut 2018-01-17 <aa78cb86-2b60-8348-0f36-f7e87241487c@2ndquadrant.com>
> On 1/17/18 10:14, Christoph Berg wrote:
> > The difference is that when parsing version() (which is all my variant
> > is changing), people already have to deal with extra stuff at the end
> > (gcc version), while that would be new for "psql --version".
> 
> For me, having the package identifier in "psql -version" specifically
> would actually be the biggest win in this, because that's where a lot of
> issues arise (e.g., wrong libedit/readline library, different
> compiled-in socket location).

I enabled it for 10 and 11 now. With the arguably very long 11devel
snapshot build version number, it now looks like this:

$ psql --version
psql (PostgreSQL) 11devel (Debian 11~~devel~20180119.1047-1~329.git4e54dd2.pgdg+1)

$ /usr/lib/postgresql/10/bin/psql --version
psql (PostgreSQL) 10.1 (Debian 10.1-3.pgdg+1)

$ psql
psql (11devel (Debian 11~~devel~20180119.1047-1~329.git4e54dd2.pgdg+1), Server 10.1 (Debian 10.1-3.pgdg+1))

I'll report back if I see anything exploding.

The CF item is closed, thanks!

Christoph
-- 
Senior Berater, Tel.: +49 2166 9901 187
credativ GmbH, HRB Mönchengladbach 12080, USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer
pgp fingerprint: 5C48 FE61 57F4 9179 5970  87C6 4C5A 6BAB 12D2 A7AE