[PATCH] Replacement for OSSP-UUID for Linux and BSD

Matteo Beccati <php@beccati.com> writes:

here's the latest version of my uuid changes patch, according to
proposal (2) from Tom in the thread about OSSP-UUID[1].

Hmm ... this is not actually what I had in mind. Unless I'm misreading
the patch, this nukes the "uuid-ossp" extension entirely in favor of a
new extension "uuid" (providing the same SQL functions with a different
underlying implementation). I don't think this works from the standpoint
of providing compatibility for users of the existing extension.
In particular, it'd break pg_upgrade (because of the change of the .so
library name) as well as straight pg_dump upgrades (which would expect
CREATE EXTENSION "uuid-ossp" to work). Not to mention application code
that might expect CREATE EXTENSION "uuid-ossp" to still work.

Another objection is that for people for whom OSSP uuid still works fine,
this is forcing them to adopt a new implementation whose compatibility is
as yet unproven.

What I'd rather do is preserve contrib/uuid-ossp with the same extension
and .so name, but with two configure options that select different
underlying implementations.

In the long run it'd be nice to migrate away from the "uuid-ossp"
extension name, mostly because of the poorly-chosen use of a dash in the
name. But I'm not sure how we do that without breaking backwards
compatibility, and anyway it's an entirely cosmetic thing that we can
worry about later.

Anyhow, doing it like that seems like it ought to be a pretty
straightforward refactoring of your patch. I could pursue that,
or you can.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi Tom,

thanks for the feedback.

On 25/05/2014 21:10, Tom Lane wrote:

Matteo Beccati <php@beccati.com> writes:

here's the latest version of my uuid changes patch, according to
proposal (2) from Tom in the thread about OSSP-UUID[1].

Hmm ... this is not actually what I had in mind. Unless I'm misreading
the patch, this nukes the "uuid-ossp" extension entirely in favor of a
new extension "uuid" (providing the same SQL functions with a different
underlying implementation). I don't think this works from the standpoint
of providing compatibility for users of the existing extension.
In particular, it'd break pg_upgrade (because of the change of the .so
library name) as well as straight pg_dump upgrades (which would expect
CREATE EXTENSION "uuid-ossp" to work). Not to mention application code
that might expect CREATE EXTENSION "uuid-ossp" to still work.

Another objection is that for people for whom OSSP uuid still works fine,
this is forcing them to adopt a new implementation whose compatibility is
as yet unproven.

What I'd rather do is preserve contrib/uuid-ossp with the same extension
and .so name, but with two configure options that select different
underlying implementations.

Sure, that makes sense. I wasn't actually sure it was ok to keep the
"OSSP" brand even though the extensions didn't use the oosp library,
hence the renaming. But I do agree upgrades wouldn't be very easy if we
don't.

In the long run it'd be nice to migrate away from the "uuid-ossp"
extension name, mostly because of the poorly-chosen use of a dash in the
name. But I'm not sure how we do that without breaking backwards
compatibility, and anyway it's an entirely cosmetic thing that we can
worry about later.

Anyhow, doing it like that seems like it ought to be a pretty
straightforward refactoring of your patch. I could pursue that,
or you can.

I do have a system with the ossp library installed: I'd be happy to give
it a try tomorrow morning my time, depending on my workload. I'll keep
you posted!

Cheers
--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi Tom,

On 25/05/2014 21:10, Tom Lane wrote:

Anyhow, doing it like that seems like it ought to be a pretty
straightforward refactoring of your patch. I could pursue that,
or you can.

I'm attaching v2 of the patch. Here's a list of changes from v1:

* Restored --with-ossp-uuid. Configure tries ossp support first, then
falls back to Linux and BSD variants
* md5.o and sha1.o are linked only when not using the ossp library
* fixed a bug in the v1mc generation carried over from uuid-freebsd:
only the mac part is randomized now
* updated regression tests, which are now passing on ubuntu w/ and w/o
ossp and netbsd.

This was the best I could achieve in the limited timeframe I had
available. I believe it could be a good starting point for your final
refactoring.

Cheers
--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

Matteo Beccati <php@beccati.com> writes:

I'm attaching v2 of the patch. Here's a list of changes from v1:

* Restored --with-ossp-uuid. Configure tries ossp support first, then
falls back to Linux and BSD variants
* md5.o and sha1.o are linked only when not using the ossp library
* fixed a bug in the v1mc generation carried over from uuid-freebsd:
only the mac part is randomized now
* updated regression tests, which are now passing on ubuntu w/ and w/o
ossp and netbsd.

This was the best I could achieve in the limited timeframe I had
available. I believe it could be a good starting point for your final
refactoring.

I've not reviewed this in any detail yet, but I believe we're in agreement
on the general ideas. I do think that we need two separate configure
switches: one asking to use the OSSP library and one asking to use the
other library. That's an easy change though.

Assuming this works as advertised, does anyone have an objection to
pushing it into 9.4?

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2014-05-26 13:25:49 -0400, Tom Lane wrote:

Matteo Beccati <php@beccati.com> writes:

I'm attaching v2 of the patch. Here's a list of changes from v1:

* Restored --with-ossp-uuid. Configure tries ossp support first, then
falls back to Linux and BSD variants

Imo should be the other way round.

* md5.o and sha1.o are linked only when not using the ossp library

The backend already has a md5 implementation, no?

Assuming this works as advertised, does anyone have an objection to
pushing it into 9.4?

I think it's a pretty bad idea. But the alternatives seem worse. We
should better get it right though if we do it.

Greetings,

Andres Freund

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 26/05/2014 19:31, Andres Freund wrote:

On 2014-05-26 13:25:49 -0400, Tom Lane wrote:

Matteo Beccati <php@beccati.com> writes:

I'm attaching v2 of the patch. Here's a list of changes from v1:

* Restored --with-ossp-uuid. Configure tries ossp support first, then
falls back to Linux and BSD variants

Imo should be the other way round.

My goal was to avoid changing the underlying implementation if someone
has the ossp library installed. Tom is suggesting to split the configure
switch in two, so I guess that would allow the most flexibility.

* md5.o and sha1.o are linked only when not using the ossp library

The backend already has a md5 implementation, no?

Yes, but as far as I could see it can only convert a string to hash.
Making it fit the uuid use case would require concatenating the
namespace and source string before passing it to the md5 function. The
BSD system and pgcrypto implementations do not:

https://github.com/mbeccati/postgres/blob/ossp/contrib/uuid-ossp/uuid-ossp.c#L237

We could even use the system md5 and sha1 on BSD, but I've spotted at
least a difference between FreeBSD and NetBSD (sha.h vs sha1.h and
different function names) and I didn't think it was worth to pursue
that, especially if beta2 is the target.

Cheers
--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Matteo Beccati <php@beccati.com> writes:

On 26/05/2014 19:31, Andres Freund wrote:

On 2014-05-26 13:25:49 -0400, Tom Lane wrote:

Matteo Beccati <php@beccati.com> writes:

* Restored --with-ossp-uuid. Configure tries ossp support first, then
falls back to Linux and BSD variants

Imo should be the other way round.

My goal was to avoid changing the underlying implementation if someone
has the ossp library installed. Tom is suggesting to split the configure
switch in two, so I guess that would allow the most flexibility.

Yeah: if we have two configure switches then we can avoid having to decide
which library takes precedence. But the main reason for that is we have
a general policy that what you get from a given configure switch should be
predictable, rather than depending on what is or is not installed. (If we
were sure these libraries were 100% interchangeable then maybe one switch
would be OK, but I'm not too sure about that, yet.)

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: Tom Lane 2014-05-25 <12508.1401045013@sss.pgh.pa.us>

Matteo Beccati <php@beccati.com> writes:

here's the latest version of my uuid changes patch, according to
proposal (2) from Tom in the thread about OSSP-UUID[1].

Hmm ... this is not actually what I had in mind. Unless I'm misreading
the patch, this nukes the "uuid-ossp" extension entirely in favor of a
new extension "uuid" (providing the same SQL functions with a different
underlying implementation). I don't think this works from the standpoint
of providing compatibility for users of the existing extension.
In particular, it'd break pg_upgrade (because of the change of the .so
library name) as well as straight pg_dump upgrades (which would expect
CREATE EXTENSION "uuid-ossp" to work). Not to mention application code
that might expect CREATE EXTENSION "uuid-ossp" to still work.

Another objection is that for people for whom OSSP uuid still works fine,
this is forcing them to adopt a new implementation whose compatibility is
as yet unproven.

What I'd rather do is preserve contrib/uuid-ossp with the same extension
and .so name, but with two configure options that select different
underlying implementations.

What I'd propose is to have a "pure" uuid extension using native OS
functions, so people starting a new database could go for that without
any OSSP legacy. The ossp-uuid extension would then have a dependency
(like earthdistance has requires = 'cube'), plus maybe compatibility
functions to call the proper functions from uuid.

This would make pg_dump upgrades work. No idea about pg_upgrade, but I
wanted to post that idea here, as it didn't seem to have been
discussed yet.

In the long run it'd be nice to migrate away from the "uuid-ossp"
extension name, mostly because of the poorly-chosen use of a dash in the
name. But I'm not sure how we do that without breaking backwards
compatibility, and anyway it's an entirely cosmetic thing that we can
worry about later.

Anyhow, doing it like that seems like it ought to be a pretty
straightforward refactoring of your patch. I could pursue that,
or you can.

Christoph
--
cb@df7cb.de | http://www.df7cb.de/

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Christoph Berg <cb@df7cb.de> writes:

Re: Tom Lane 2014-05-25 <12508.1401045013@sss.pgh.pa.us>

Hmm ... this is not actually what I had in mind. Unless I'm misreading
the patch, this nukes the "uuid-ossp" extension entirely in favor of a
new extension "uuid" (providing the same SQL functions with a different
underlying implementation). I don't think this works from the standpoint
of providing compatibility for users of the existing extension.
In particular, it'd break pg_upgrade (because of the change of the .so
library name) as well as straight pg_dump upgrades (which would expect
CREATE EXTENSION "uuid-ossp" to work). Not to mention application code
that might expect CREATE EXTENSION "uuid-ossp" to still work.

Another objection is that for people for whom OSSP uuid still works fine,
this is forcing them to adopt a new implementation whose compatibility is
as yet unproven.

What I'd rather do is preserve contrib/uuid-ossp with the same extension
and .so name, but with two configure options that select different
underlying implementations.

What I'd propose is to have a "pure" uuid extension using native OS
functions, so people starting a new database could go for that without
any OSSP legacy. The ossp-uuid extension would then have a dependency
(like earthdistance has requires = 'cube'), plus maybe compatibility
functions to call the proper functions from uuid.

The only thing that seems particularly "legacy" about this design is that
the extension is still named uuid-ossp. While that's certainly annoying
when the implementation isn't using OSSP, I'm not really seeing the value
of providing two near-duplicate extensions. And no, what you suggest
doesn't work well with pg_upgrade. I'm not even sure it works well
ignoring that issue: the new extension would have to choose all-new
function names so that it could be installed alongside uuid-ossp.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

I wrote:

Matteo Beccati <php@beccati.com> writes:

On 26/05/2014 19:31, Andres Freund wrote:

On 2014-05-26 13:25:49 -0400, Tom Lane wrote:

Matteo Beccati <php@beccati.com> writes:

* Restored --with-ossp-uuid. Configure tries ossp support first, then
falls back to Linux and BSD variants

Imo should be the other way round.

My goal was to avoid changing the underlying implementation if someone
has the ossp library installed. Tom is suggesting to split the configure
switch in two, so I guess that would allow the most flexibility.

Yeah: if we have two configure switches then we can avoid having to decide
which library takes precedence. But the main reason for that is we have
a general policy that what you get from a given configure switch should be
predictable, rather than depending on what is or is not installed. (If we
were sure these libraries were 100% interchangeable then maybe one switch
would be OK, but I'm not too sure about that, yet.)

Some research uncovered the fact that the e2fsprogs UUID library (ie the
"Linux" one) is available for FreeBSD too:
http://www.freshports.org/misc/e2fsprogs-libuuid/

This means that if we want to give users control over which implementation
gets selected, we actually need *three* configure switches. In the
attached revision of Matteo's patch, I called them --with-ossp-uuid
(the existing switch name), --with-linux-uuid, and --with-bsd-uuid.
I'm not necessarily wedded to the latter two names; in particular it seems
unfortunate that the right one to use on OS X is --with-linux-uuid.
But I think --with-e2fsprogs-uuid is right out, so it's not clear what
would be better.

I've verified functionality of this patch on these scenarios:

(1) --with-ossp-uuid on RHEL6, using uuid-1.6.1-10.el6.x86_64
(2) --with-linux-uuid on RHEL6, using libuuid-2.17.2-12.14.el6_5.x86_64
(3) --with-linux-uuid on OS X 10.9.3, Intel
(4) --with-linux-uuid on OS X 10.4.11, PPC (hence, bigendian)

I do not have a machine on which to try --with-bsd-uuid, so it's
possible I broke that portion of Matteo's patch. Would someone try
that case on a FreeBSD box?

Other than retesting that case, I think this is ready to go except that
it lacks documentation updates.

regards, tom lane

On May 26, 2014, at 6:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

This means that if we want to give users control over which implementation
gets selected, we actually need *three* configure switches. In the
attached revision of Matteo's patch, I called them --with-ossp-uuid
(the existing switch name), --with-linux-uuid, and --with-bsd-uuid.
I'm not necessarily wedded to the latter two names; in particular it seems
unfortunate that the right one to use on OS X is --with-linux-uuid.
But I think --with-e2fsprogs-uuid is right out, so it's not clear what
would be better.

How about --with-unix-uuid? Or --with-ext2-uuid?

Which one is the default -- or is there one? Should we use some sort of mapping to select the right switch by platform, or if ossp-uuid appears to be installed?

Best,

David

"David E. Wheeler" <david@justatheory.com> writes:

On May 26, 2014, at 6:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

This means that if we want to give users control over which implementation
gets selected, we actually need *three* configure switches. In the
attached revision of Matteo's patch, I called them --with-ossp-uuid
(the existing switch name), --with-linux-uuid, and --with-bsd-uuid.
I'm not necessarily wedded to the latter two names; in particular it seems
unfortunate that the right one to use on OS X is --with-linux-uuid.
But I think --with-e2fsprogs-uuid is right out, so it's not clear what
would be better.

How about --with-unix-uuid? Or --with-ext2-uuid?

Meh. "Unix" certainly subsumes BSD, so that doesn't seem like a very
useful distinction. I guess we could use "ext2" but that would just
confuse most people.

Which one is the default -- or is there one?

The point here is that we won't make a default choice.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On May 26, 2014, at 9:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

How about --with-unix-uuid? Or --with-ext2-uuid?

Meh. "Unix" certainly subsumes BSD, so that doesn't seem like a very
useful distinction. I guess we could use "ext2" but that would just
confuse most people.

--with-uuid?

Which one is the default -- or is there one?

The point here is that we won't make a default choice.

So no UUID functions by default, which I guess has been the case all along?

Always seemed weird to me that there was a core configure option specific to a contrib module.

D

"David E. Wheeler" <david@justatheory.com> writes:

On May 26, 2014, at 9:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

How about --with-unix-uuid? Or --with-ext2-uuid?

Meh. "Unix" certainly subsumes BSD, so that doesn't seem like a very
useful distinction. I guess we could use "ext2" but that would just
confuse most people.

--with-uuid?

I thought about that but figured we'd regret it ...

So no UUID functions by default, which I guess has been the case all along?

Yup.

Always seemed weird to me that there was a core configure option specific to a contrib module.

There's other cases, eg --with-libxslt for contrib/xml2. I think
contrib/sepgsql has bespoke configure support too.

Fundamentally the reason uuid-ossp isn't in core is that we feared it
wasn't portable enough, a fear subsequently thoroughly validated.
Perhaps this patch will move us a notch towards having functionality
we could expect to be available everywhere ... though we're certainly
not all the way there yet.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi Tom,

On 27/05/2014 03:07, Tom Lane wrote:

I've verified functionality of this patch on these scenarios:

(1) --with-ossp-uuid on RHEL6, using uuid-1.6.1-10.el6.x86_64
(2) --with-linux-uuid on RHEL6, using libuuid-2.17.2-12.14.el6_5.x86_64
(3) --with-linux-uuid on OS X 10.9.3, Intel
(4) --with-linux-uuid on OS X 10.4.11, PPC (hence, bigendian)

I do not have a machine on which to try --with-bsd-uuid, so it's
possible I broke that portion of Matteo's patch. Would someone try
that case on a FreeBSD box?

I've tested on NetBSD i386 and --with-bsd-uuid worked out of the box. I
could fire up some virtual machines with FreeBSD and other BSD flavours,
but maybe some buildfarm animals could be used for that.

I'm attaching a little patch to be applied on top of yours.

I didn't notice that "buf ? 13 : 0" was raising a warning about the
condition being always true on BSD. I guess it's safe to hardcode 13 as
the argument is ignored anyway with ossp, so I've fixed that.

I've also fixed v1mc generation on "linux" to match the OSSP and BSD
variants and added a regression test for it.

Cheers
--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

Matteo Beccati <php@beccati.com> writes:

On 27/05/2014 03:07, Tom Lane wrote:

I do not have a machine on which to try --with-bsd-uuid, so it's
possible I broke that portion of Matteo's patch. Would someone try
that case on a FreeBSD box?

I've tested on NetBSD i386 and --with-bsd-uuid worked out of the box.

Ah, cool. I had documented this option as only working for FreeBSD,
but that's obviously too conservative. Anyone know about whether it
works on OpenBSD?

I'm attaching a little patch to be applied on top of yours.

Thanks, will incorporate this.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi Tom,

On 27/05/2014 15:52, Tom Lane wrote:

Matteo Beccati <php@beccati.com> writes:

On 27/05/2014 03:07, Tom Lane wrote:

I do not have a machine on which to try --with-bsd-uuid, so it's
possible I broke that portion of Matteo's patch. Would someone try
that case on a FreeBSD box?

I've tested on NetBSD i386 and --with-bsd-uuid worked out of the box.

Ah, cool. I had documented this option as only working for FreeBSD,
but that's obviously too conservative. Anyone know about whether it
works on OpenBSD?

I've tried to google "man uuid openbsd" and I got the e2fs package
(which contains uuid/uuid.h and libuuid) instead of a man page, so I
believe that could be another use case for --with-linux-uuid.

If it's confirmed to be working, that makes two BSD-derived systems
requiring "linux-uuid", so --with-e2fs-uuid or similar would be more
appropriate.

Cheers
--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane wrote:

"David E. Wheeler" <david@justatheory.com> writes:

On May 26, 2014, at 9:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

How about --with-unix-uuid? Or --with-ext2-uuid?

Meh. "Unix" certainly subsumes BSD, so that doesn't seem like a very
useful distinction. I guess we could use "ext2" but that would just
confuse most people.

--with-uuid?

I thought about that but figured we'd regret it ...

Why not --with-uuid-implementation=<impl>, and have <impl> be one of
e2utils, bsd, ossp, with the latter being default? We could also have
offer the value "list" or "help" which would list the available options.
That way, if we come up with a new implementation in the future, this is
easily extensible.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Matteo Beccati <php@beccati.com> writes:

On 27/05/2014 15:52, Tom Lane wrote:

Ah, cool. I had documented this option as only working for FreeBSD,
but that's obviously too conservative. Anyone know about whether it
works on OpenBSD?

I've tried to google "man uuid openbsd" and I got the e2fs package
(which contains uuid/uuid.h and libuuid) instead of a man page, so I
believe that could be another use case for --with-linux-uuid.

Yeah, googling led me to the same conclusion: e2fsprogs libuuid is
definitely available for OpenBSD, but I found no evidence that it
has the built-in UUID functions that FreeBSD/NetBSD have.

If it's confirmed to be working, that makes two BSD-derived systems
requiring "linux-uuid", so --with-e2fs-uuid or similar would be more
appropriate.

Still don't like that name much, but maybe that's the best we can do.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers