Thread: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
Hi, I am working on this TODO item: * Consider allowing 64-bit integers and floats to be passed by value on 64-bit platforms Also change 32-bit floats (float4) to be passed by value at the same time. For genbki.sh, to correctly determine whether HAVE_LONG_INT_64 is defined, the attached bugfix is needed in the configure.in machinery. This way the pg_config.h actually conforms to the comments for HAVE_LONG_INT_64 and HAVE_LONG_LONG_INT_64. Best regards, Zoltán Böszörményi -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH http://www.postgresql.at/ diff -dcrpN pgsql.orig/config/c-compiler.m4 pgsql/config/c-compiler.m4 *** pgsql.orig/config/c-compiler.m4 2008-02-18 13:49:56.000000000 +0100 --- pgsql/config/c-compiler.m4 2008-03-11 11:00:30.000000000 +0100 *************** AC_COMPILE_IFELSE([AC_LANG_BOOL_COMPILE_ *** 62,68 **** Ac_define=$Ac_cachevar if test x"$Ac_cachevar" = xyes ; then ! AC_DEFINE(Ac_define,, [Define to 1 if `]$1[' works and is 64 bits.]) fi undefine([Ac_define])dnl undefine([Ac_cachevar])dnl --- 62,68 ---- Ac_define=$Ac_cachevar if test x"$Ac_cachevar" = xyes ; then ! AC_DEFINE(Ac_define, 1, [Define to 1 if `]$1[' works and is 64 bits.]) fi undefine([Ac_define])dnl undefine([Ac_cachevar])dnl diff -dcrpN pgsql.orig/configure pgsql/configure *** pgsql.orig/configure 2008-03-02 13:44:42.000000000 +0100 --- pgsql/configure 2008-03-11 11:01:34.000000000 +0100 *************** HAVE_LONG_INT_64=$pgac_cv_type_long_int_ *** 19608,19614 **** if test x"$pgac_cv_type_long_int_64" = xyes ; then cat >>confdefs.h <<\_ACEOF ! #define HAVE_LONG_INT_64 _ACEOF fi --- 19608,19614 ---- if test x"$pgac_cv_type_long_int_64" = xyes ; then cat >>confdefs.h <<\_ACEOF ! #define HAVE_LONG_INT_64 1 _ACEOF fi *************** HAVE_LONG_LONG_INT_64=$pgac_cv_type_long *** 19741,19747 **** if test x"$pgac_cv_type_long_long_int_64" = xyes ; then cat >>confdefs.h <<\_ACEOF ! #define HAVE_LONG_LONG_INT_64 _ACEOF fi --- 19741,19747 ---- if test x"$pgac_cv_type_long_long_int_64" = xyes ; then cat >>confdefs.h <<\_ACEOF ! #define HAVE_LONG_LONG_INT_64 1 _ACEOF fi
int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
From
Zoltan Boszormenyi
Date:
Hi, I got around to (almost) finalize the patch. What it does: - fixes the configure define described in my previous mail - when the value of HAVE_LONG_INT_64 == 1, the following types are passed by value: int8, float8, time, timestamp, timestamptz The time[stamp[tz]] types caused segfaults in the regression test if their attbyval setting was different from int8/float8, so it's really needed. I am not sure there's another type that needs a similat switch, the type regression tests are running fine. - In the HAVE_LONG_INT_64 == 1 case, Int64GetDatum() becomes a casting macro instead of a function, some others become functions. The implementation of DatumGetFloat4()/Float4GetDatum()/etc functions may change into an inline or change internals. - float4 is now unconditionally passed by value - the int8inc(), int2_sum() and int4_sum() used pointers directly from the Datums for performance, that code path is now commented out, the other code path is correct for the AggState and !AggState runs and correct every time and now because of the passbyval nature of int8, the !AggState version is not slower than using the pointer directly. - removed deprecated DatumGetFloat32/Float32GetDatum/etc macros, they aren't used anywhere in the core and contrib source. There is only one regression, in the tsearch tests. Some selects in tsearch now return no records but they don't segfault. I couldn't hunt the bug down yet, not sure I will have time in the near future for that. Comments welcome. Best regards, Zoltán Böszörményi Zoltan Boszormenyi írta: > Hi, > > I am working on this TODO item: > > * Consider allowing 64-bit integers and floats to be passed by value on > 64-bit platforms > > Also change 32-bit floats (float4) to be passed by value at the same > time. > > For genbki.sh, to correctly determine whether HAVE_LONG_INT_64 > is defined, the attached bugfix is needed in the configure.in machinery. > This way the pg_config.h actually conforms to the comments for > HAVE_LONG_INT_64 and HAVE_LONG_LONG_INT_64. > > Best regards, > Zoltán Böszörményi > > ------------------------------------------------------------------------ > > -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH http://www.postgresql.at/
Attachment
Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
From
Gregory Stark
Date:
"Zoltan Boszormenyi" <zb@cybertec.at> writes: > - the int8inc(), int2_sum() and int4_sum() used pointers directly from the > Datums > for performance, that code path is now commented out, the other code path > is correct for the AggState and !AggState runs and correct every time and now > because of the passbyval nature of int8, the !AggState version is not slower > than using the pointer directly. Does this mean count() and sum() are slower on a 32-bit machine? -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training!
Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
From
Zoltan Boszormenyi
Date:
Hi, Gregory Stark írta: > "Zoltan Boszormenyi" <zb@cybertec.at> writes: > > >> - the int8inc(), int2_sum() and int4_sum() used pointers directly from the >> Datums >> for performance, that code path is now commented out, the other code path >> is correct for the AggState and !AggState runs and correct every time and now >> because of the passbyval nature of int8, the !AggState version is not slower >> than using the pointer directly. >> > > Does this mean count() and sum() are slower on a 32-bit machine? > If you mean "slower than on a 64-bit machine" then yes. If you mean "slower than before", then no. I didn't express myself correctly. The original code path is not commented out, it is just conditionally compiled. BTW I found the tsearch bug, it was a missing conversion of float4 in gistproc.c, it was an unfortunate detail that this didn't cause a segfault, it woul have been easier to find. Now there are no failing regression tests. -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH http://www.postgresql.at/
Attachment
Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
From
Gregory Stark
Date:
Ok, ignore my previous message. I've read the patch now and that's not an issue. The old code path is not commented out, it's #ifdef'd conditionally on HAVE_LONG_INT_64 is right (well it seems right, it's a bit hard to tell in patch form). A few comments: 1) Please don't include configure in your patch. I don't know why it's checked into CVS but it is so that means manually removing it from any patch. It's usually a huge portion of the diff so it's worth removing. 2) The genbki.sh change could be a bit tricky for multi-platform builds (ie OSX). I don't really see an alternative so it's just something to note for the folks setting that up (Hi Dave). Actually there is an alternative but I prefer the approach you've taken. The alternative would be to have a special value in the catalogs for 8-bit maybe-pass-by-value data types and handle the check at run-time. Another alternative would be to have initdb fix up these values in C code instead of fixing them directly in the bki scripts. That seems like more hassle than it's worth though and a bigger break with the rest. 3) You could get rid of a bunch of #ifndef HAVE_LONG_INT_64 snippets by having a #define like INT64PASSBYVALUE which is defined to be either "true" or "false". It might start getting confusing having three different defines for the same thing though. But personally I hate having more #ifdefs than necessary, it makes it hard to read the code. 4) Your problems with tsearch and timestamp etc raise an interesting problem. We don't need to mark this in pg_control because it's a purely a run-time issue and doesn't affect on-disk storage. However it does affect ABI compatibility with modules. Perhaps it should be added to PG_MODULE_MAGIC_DATA. Actually, why isn't sizeof(Datum) in there already? Do we have any protection against loading 64-bit compiled modules in a 32-bit server or vice versa? But generally this is something I've been wanting to do for a while and basically the same approach I would have taken. It seems sound to me. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services!
Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
From
Zoltan Boszormenyi
Date:
Gregory Stark írta: > Ok, ignore my previous message. I've read the patch now and that's not an > issue. The old code path is not commented out, it's #ifdef'd conditionally on > HAVE_LONG_INT_64 is right (well it seems right, it's a bit hard to tell in > patch form). > > A few comments: > > 1) Please don't include configure in your patch. I don't know why it's checked > into CVS but it is so that means manually removing it from any patch. It's > usually a huge portion of the diff so it's worth removing. > Noted. > 2) The genbki.sh change could be a bit tricky for multi-platform builds (ie > OSX). I don't really see an alternative so it's just something to note for > the folks setting that up (Hi Dave). > > Actually there is an alternative but I prefer the approach you've taken. > The alternative would be to have a special value in the catalogs for 8-bit > maybe-pass-by-value data types and handle the check at run-time. > > Another alternative would be to have initdb fix up these values in C code > instead of fixing them directly in the bki scripts. That seems like more > hassle than it's worth though and a bigger break with the rest. > > 3) You could get rid of a bunch of #ifndef HAVE_LONG_INT_64 snippets by having > a #define like INT64PASSBYVALUE which is defined to be either "true" or > "false". It might start getting confusing having three different defines > for the same thing though. But personally I hate having more #ifdefs than > necessary, it makes it hard to read the code. > OK, this would also make the patch smaller. Is pg_config_manual.h good for this setting? Or which header would you suggest? > 4) Your problems with tsearch and timestamp etc raise an interesting problem. > We don't need to mark this in pg_control because it's a purely a run-time > issue and doesn't affect on-disk storage. However it does affect ABI > compatibility with modules. Perhaps it should be added to > PG_MODULE_MAGIC_DATA. > I am looking into it. > Actually, why isn't sizeof(Datum) in there already? Do we have any > protection against loading 64-bit compiled modules in a 32-bit server or > vice versa? > You can't mix 32-bit executables with 64-bit shared libraries, well, without tricks. I don't see any problem here. > But generally this is something I've been wanting to do for a while and > basically the same approach I would have taken. It seems sound to me. > Thanks for commenting and encouragement. -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH http://www.postgresql.at/
Re: Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
From
Zoltan Boszormenyi
Date:
Zoltan Boszormenyi írta: > Gregory Stark írta: >> 4) Your problems with tsearch and timestamp etc raise an interesting >> problem. >> We don't need to mark this in pg_control because it's a purely a >> run-time >> issue and doesn't affect on-disk storage. However it does affect ABI >> compatibility with modules. Perhaps it should be added to >> PG_MODULE_MAGIC_DATA. >> > > I am looking into it. Do you think it's actually needed? PG_MODULE_MAGIC_DATA contains the server version the external module was compiled for. This patch won't go to older versions, so it's already protected from the unconditional float4 change. And because you can't mix server and libraries with different bitsize, it's protected from the conditional int64, float8, etc. changes. Best regards, Zoltán Böszörményi -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH http://www.postgresql.at/
Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
From
Zoltan Boszormenyi
Date:
Zoltan Boszormenyi írta: > > BTW I found the tsearch bug, it was a missing conversion of float4 > in gistproc.c, it was an unfortunate detail that this didn't cause a > segfault, > it woul have been easier to find. Now there are no failing regression > tests. Disregards this patch, the bugfix for tsearch is not real. -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH http://www.postgresql.at/
Re: Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
From
Gregory Stark
Date:
"Zoltan Boszormenyi" <zb@cybertec.at> writes: > Zoltan Boszormenyi írta: >> Gregory Stark írta: >>> 4) Your problems with tsearch and timestamp etc raise an interesting >>> problem. >>> We don't need to mark this in pg_control because it's a purely a run-time >>> issue and doesn't affect on-disk storage. However it does affect ABI >>> compatibility with modules. Perhaps it should be added to >>> PG_MODULE_MAGIC_DATA. >> >> I am looking into it. > > Do you think it's actually needed? > PG_MODULE_MAGIC_DATA contains the server version > the external module was compiled for. This patch won't go to > older versions, so it's already protected from the unconditional > float4 change. And because you can't mix server and libraries > with different bitsize, it's protected from the conditional int64, > float8, etc. changes. Right, it does seem like we're conservative about adding things to PG_MODULE_MAGIC. As long as this is hard coded based on HAVE_LONG_INT_64 then we don't strictly need it. And I don't see much reason to make this something the user can override. If there are modules which use the wrong macros and assume certain other data types are pass-by-reference when they're not then they're going to fail regardless of what version of postgres they're compiled against anyways. So I would say in response to your other query to _not_ use pg_config_manual.h which is intended for variables which the user can override. If you put anything there then we would have to worry about incompatibilities. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's Slony Replication support!
Zoltan Boszormenyi wrote: > For genbki.sh, to correctly determine whether HAVE_LONG_INT_64 > is defined, the attached bugfix is needed in the configure.in machinery. > This way the pg_config.h actually conforms to the comments for > HAVE_LONG_INT_64 and HAVE_LONG_LONG_INT_64. I think this part of the patch can be committed right away. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Re: Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
From
Zoltan Boszormenyi
Date:
Gregory Stark írta: > "Zoltan Boszormenyi" <zb@cybertec.at> writes: > > >> Zoltan Boszormenyi írta: >> >>> Gregory Stark írta: >>> >>>> 4) Your problems with tsearch and timestamp etc raise an interesting >>>> problem. >>>> We don't need to mark this in pg_control because it's a purely a run-time >>>> issue and doesn't affect on-disk storage. However it does affect ABI >>>> compatibility with modules. Perhaps it should be added to >>>> PG_MODULE_MAGIC_DATA. >>>> >>> I am looking into it. >>> >> Do you think it's actually needed? >> PG_MODULE_MAGIC_DATA contains the server version >> the external module was compiled for. This patch won't go to >> older versions, so it's already protected from the unconditional >> float4 change. And because you can't mix server and libraries >> with different bitsize, it's protected from the conditional int64, >> float8, etc. changes. >> > > > Right, it does seem like we're conservative about adding things to > PG_MODULE_MAGIC. As long as this is hard coded based on HAVE_LONG_INT_64 then > we don't strictly need it. And I don't see much reason to make this something > the user can override. > > If there are modules which use the wrong macros and assume certain other data > types are pass-by-reference when they're not then they're going to fail > regardless of what version of postgres they're compiled against anyways. > > So I would say in response to your other query to _not_ use pg_config_manual.h > which is intended for variables which the user can override. If you put > anything there then we would have to worry about incompatibilities OK, third version, the #define INT64PASSBYVAL is used now for less #ifdef'd code, I used postgres.h for the defines for now. As for the tsearch problem, it seems that bth tsearch and gist in general uses the "internal" type for passing pointers to different datatypes around for modifying them in-place, float4 among them. So, somewhere tsearch or gist uses hardcoded passed-by-ref where it really a float4, not "internal". Someone with more knowledge about tsearch could look into this... -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH http://www.postgresql.at/
Attachment
Re: Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
From
Tom Lane
Date:
Zoltan Boszormenyi <zb@cybertec.at> writes:
> Gregory Stark �rta:
>> 1) Please don't include configure in your patch. I don't know why it's checked
>> into CVS but it is so that means manually removing it from any patch. It's
>> usually a huge portion of the diff so it's worth removing.
> Noted.
Just for the record: the reason configure is in CVS is to avoid
requiring users of CVS checkouts to have autoconf installed.
Perhaps we should rethink that, but in any case there's no point
in submitting manual diffs to configure (or any other generated
file). Best practice is to just remind the committer that the
generated file needs to be regenerated.
>> 3) You could get rid of a bunch of #ifndef HAVE_LONG_INT_64 snippets by having
>> a #define like INT64PASSBYVALUE which is defined to be either "true" or
>> "false".
> OK, this would also make the patch smaller.
> Is pg_config_manual.h good for this setting?
I'd go for having a #define like that, but what is the reason to set it
in pg_config_manual.h? Seems like the configure script should set it.
regards, tom lane
Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
From
Magnus Hagander
Date:
I haven't had time to look through the patch, but reading Gregs comments I noted: > 2) The genbki.sh change could be a bit tricky for multi-platform builds (ie > OSX). I don't really see an alternative so it's just something to note for > the folks setting that up (Hi Dave). Changes to genbki.sh also have to be "mirrored" in the msvc build scripts (Genbki.pm) in most cases... //Magnus
Re: Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
From
Zoltan Boszormenyi
Date:
Tom Lane wrote: > Zoltan Boszormenyi <zb@cybertec.at> writes: > >> Gregory Stark írta: >> >>> 1) Please don't include configure in your patch. I don't know why it's checked >>> into CVS but it is so that means manually removing it from any patch. It's >>> usually a huge portion of the diff so it's worth removing. >>> > > >> Noted. >> > > Just for the record: the reason configure is in CVS is to avoid > requiring users of CVS checkouts to have autoconf installed. > > Perhaps we should rethink that, but in any case there's no point > in submitting manual diffs to configure (or any other generated > file). Best practice is to just remind the committer that the > generated file needs to be regenerated. > > >>> 3) You could get rid of a bunch of #ifndef HAVE_LONG_INT_64 snippets by having >>> a #define like INT64PASSBYVALUE which is defined to be either "true" or >>> "false". >>> > > >> OK, this would also make the patch smaller. >> Is pg_config_manual.h good for this setting? >> > > I'd go for having a #define like that, but what is the reason to set it > in pg_config_manual.h? Seems like the configure script should set it. > Obviously. :-) Thanks. And: Magnus Hagander wrote: > > Changes to genbki.sh also have to be "mirrored" in the msvc build > scripts (Genbki.pm) in most cases... > > > //Magnus Thanks for the info, I modified this file as well. Please review the change, I am not a Perl expert and I don't have a Windows build environment. New patch is attached. -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH http://www.postgresql.at/
Attachment
Re: Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
From
Zoltan Boszormenyi
Date:
Tom Lane wrote: > Zoltan Boszormenyi <zb@cybertec.at> writes: > >> Gregory Stark írta: >> >>> 1) Please don't include configure in your patch. I don't know why it's checked >>> into CVS but it is so that means manually removing it from any patch. It's >>> usually a huge portion of the diff so it's worth removing. >>> > > >> Noted. >> > > Just for the record: the reason configure is in CVS is to avoid > requiring users of CVS checkouts to have autoconf installed. > > Perhaps we should rethink that, but in any case there's no point > in submitting manual diffs to configure (or any other generated > file). Best practice is to just remind the committer that the > generated file needs to be regenerated. > > >>> 3) You could get rid of a bunch of #ifndef HAVE_LONG_INT_64 snippets by having >>> a #define like INT64PASSBYVALUE which is defined to be either "true" or >>> "false". >>> > > >> OK, this would also make the patch smaller. >> Is pg_config_manual.h good for this setting? >> > > I'd go for having a #define like that, but what is the reason to set it > in pg_config_manual.h? Seems like the configure script should set it. > Obviously. :-) Thanks. And: Magnus Hagander wrote: > > Changes to genbki.sh also have to be "mirrored" in the msvc build > scripts (Genbki.pm) in most cases... > > > //Magnus Thanks for the info, I modified this file as well. Please review the change, I am not a Perl expert and I don't have a Windows build environment. New patch is attached. -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH http://www.postgresql.at/
Attachment
Re: Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
From
Alvaro Herrera
Date:
I don't think
my $int64passbyval = "(?($real64 = 1)t|f)";
works. Perhaps
my $int64passbyval = $real64 ? 't' : 'f';
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Attachment
Re: Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
From
Zoltan Boszormenyi
Date:
Alvaro Herrera írta: > I don't think > my $int64passbyval = "(?($real64 = 1)t|f)"; > > works. Perhaps > > my $int64passbyval = $real64 ? 't' : 'f'; > Thanks. Modified patch attached. Stupid question follows. Now that float4 is passed by value unconditionally, is it worth modifying the *penalty() functions in GIST/TSearch to just use PG_RETURN_FLOAT4()? Or the implicit "modify the float4 value at the caller site and return the same pointer I got as 3rd parameter" is an internal API set in stone? Modifying them to have only 2 parameters (the 3rd one was an implicit OUT parameter anyway) and omitting the pointer dereference might give a small speedup. -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH http://www.postgresql.at/
Attachment
Hi,
I tried to split the previous patch up to see where the tsearch regression
comes from. So, it turned out that:
- float4 conversion is risk free (patch #1)
- float8 conversion is okay, too, if coupled with time[stamp[tz]] conversion
(patch #2) but with int64 timestamps enabled, the next one is also
needed:
- int64 conversion (patch #3) is mostly okay but it is the one that's
causing
the tsearch regression
I looked at the tsearch code and found only one thing that can be
suspicious, namely:
typedef uint64 TSQuerySign;
TSQuerySign is handled almost everywhere as an allocated,
passed-by-reference value. I converted it with the attached patch (#4)
so it uses Int64GetDatum() and DatumGetInt64() functions internally
and the regression went away. Please, consider applying all four patches.
Best regards,
Zoltán Böszörményi
--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
http://www.postgresql.at/
Attachment
Zoltan Boszormenyi wrote: > - float4 conversion is risk free (patch #1) I applied this #1 patch. It needed some further adjustments; in particular contrib/btree_gist regression check was crashing, and utils/fmgr/README needed updating. With contrib/seg also adjusted to use float4 instead of float32, and thus the last usage of float32 gone, I am now wondering if it would be a good idea to remove the float32 and float32data definitions in c.h. Thanks to Zoltan for the patch. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera wrote:
> With contrib/seg also adjusted to use float4 instead of float32, and
> thus the last usage of float32 gone, I am now wondering if it would be a
> good idea to remove the float32 and float32data definitions in c.h.
Ok, the buildfarm is going yellow over this change. On "cardinal" I see
this:
*** ./expected/seg.out Fri Apr 18 20:45:38 2008
--- ./results/seg.out Fri Apr 18 20:59:40 2008
***************
*** 1069,1218 ****
SELECT seg_lower(s), seg_center(s), seg_upper(s)
FROM test_seg WHERE s @> '11.2..11.3' OR s IS NULL ORDER BY s;
seg_lower | seg_center | seg_upper
! -----------+------------+-----------
! -Infinity | -Infinity | 40
! -Infinity | -Infinity | 82
! -Infinity | -Infinity | 90
! 1 | 7 | 13
! 1.3 | 6.65 | 12
! 2 | 6.75 | 11.5
! 2.1 | 6.95 | 11.8
! 2.3 | Infinity | Infinity
! 2.3 | Infinity | Infinity
! 2.4 | 6.85 | 11.3
! 2.5 | 7 | 11.5
! 2.5 | 7.15 | 11.8
! 2.6 | Infinity | Infinity
! 2.7 | 7.35 | 12
! 3 | Infinity | Infinity
! 3 | 30.5 | 58
! 3.1 | 7.3 | 11.5
! 3.5 | 7.5 | 11.5
! 3.5 | 7.85 | 12.2
! 4 | 8 | 12
! 4 | Infinity | Infinity
! 4 | 8 | 12
! 4 | 7.85 | 11.7
! 4 | 8.25 | 12.5
! 4 | 8.5 | 13
! 4 | 32 | 60
! 4 | Infinity | Infinity
! 4.2 | 7.85 | 11.5
! 4.2 | 7.95 | 11.7
! 4.5 | 8.25 | 12
! 4.5 | 8 | 11.5
! 4.5 | 8.25 | 12
! 4.5 | 8.25 | 12
! 4.5 | 8.5 | 12.5
! 4.5 | 59.75 | 115
! 4.7 | 8.25 | 11.8
! 4.8 | 8.15 | 11.5
! 4.8 | 8.2 | 11.6
! 4.8 | 8.65 | 12.5
! 4.8 | Infinity | Infinity
! 4.9 | 8.45 | 12
! 4.9 | Infinity | Infinity
! 5 | 8.25 | 11.5
! 5 | 8.5 | 12
! 5 | 17.5 | 30
! 5 | 8.2 | 11.4
! 5 | 8.25 | 11.5
! 5 | 8.3 | 11.6
! 5 | 8.35 | 11.7
! 5 | 8.5 | 12
! 5 | 8.5 | 12
! 5 | 8.5 | 12
! 5.2 | 8.35 | 11.5
! 5.2 | 8.6 | 12
! 5.25 | 8.625 | 12
! 5.3 | 8.4 | 11.5
! 5.3 | 9.15 | 13
! 5.3 | 47.65 | 90
! 5.3 | Infinity | Infinity
! 5.4 | Infinity | Infinity
! 5.5 | 8.5 | 11.5
! 5.5 | 8.6 | 11.7
! 5.5 | 8.75 | 12
! 5.5 | 8.75 | 12
! 5.5 | 9 | 12.5
! 5.5 | 9.5 | 13.5
! 5.5 | Infinity | Infinity
! 5.5 | Infinity | Infinity
! 5.7 | Infinity | Infinity
! 5.9 | Infinity | Infinity
! 6 | 8.75 | 11.5
! 6 | 9 | 12
! 6 | 8.75 | 11.5
! 6 | 9.5 | 13
! 6 | 8.75 | 11.5
! 6.1 | 9.05 | 12
! 6.1 | Infinity | Infinity
! 6.2 | 8.85 | 11.5
! 6.3 | Infinity | Infinity
! 6.5 | 9 | 11.5
! 6.5 | 9.25 | 12
! 6.5 | 9.25 | 12
! 6.5 | Infinity | Infinity
! 6.6 | Infinity | Infinity
! 6.7 | 9.1 | 11.5
! 6.7 | Infinity | Infinity
! 6.75 | Infinity | Infinity
! 6.8 | Infinity | Infinity
! 6.9 | 9.55 | 12.2
! 6.9 | 48.45 | 90
! 6.9 | Infinity | Infinity
! 7 | 9.25 | 11.5
! 7 | 9.25 | 11.5
! 7 | 9.25 | 11.5
! 7 | Infinity | Infinity
! 7.15 | Infinity | Infinity
! 7.2 | 10.35 | 13.5
! 7.3 | 48.65 | 90
! 7.3 | Infinity | Infinity
! 7.3 | Infinity | Infinity
! 7.4 | 9.75 | 12.1
! 7.4 | Infinity | Infinity
! 7.5 | 9.5 | 11.5
! 7.5 | 9.75 | 12
! 7.5 | Infinity | Infinity
! 7.7 | 9.6 | 11.5
! 7.7 | Infinity | Infinity
! 7.75 | Infinity | Infinity
! 8 | 9.85 | 11.7
! 8 | 10 | 12
! 8 | 10.5 | 13
! 8.2 | Infinity | Infinity
! 8.3 | Infinity | Infinity
! 8.5 | 10 | 11.5
! 8.5 | 10.5 | 12.5
! 8.5 | Infinity | Infinity
! 8.6 | 53.8 | 99
! 8.7 | 10 | 11.3
! 8.7 | 10.2 | 11.7
! 8.9 | 10.2 | 11.5
! 9 | 10.5 | 12
! 9 | 10.15 | 11.3
! 9 | 10.25 | 11.5
! 9 | 10.5 | 12
! 9 | Infinity | Infinity
! 9.2 | 10.6 | 12
! 9.4 | 10.8 | 12.2
! 9.5 | 10.75 | 12
! 9.5 | 10.85 | 12.2
! 9.5 | Infinity | Infinity
! 9.6 | 10.55 | 11.5
! 9.7 | 10.6 | 11.5
! 9.7 | 10.85 | 12
! 9.8 | 11.15 | 12.5
! 10 | 10.8 | 11.6
! 10 | 10.75 | 11.5
! 10 | 11.25 | 12.5
! 10 | 11.25 | 12.5
! 10.2 | 11 | 11.8
! 10.5 | 11 | 11.5
! 10.5 | 11 | 11.5
! 10.5 | 12 | 13.5
! 10.7 | 11.5 | 12.3
| |
(144 rows)
--- 1069,1218 ----
SELECT seg_lower(s), seg_center(s), seg_upper(s)
FROM test_seg WHERE s @> '11.2..11.3' OR s IS NULL ORDER BY s;
seg_lower | seg_center | seg_upper
! --------------+--------------+--------------
! -3.40315e-06 | -3.40315e-06 | -3.40315e-06
! -3.40286e-06 | -3.40286e-06 | -3.40286e-06
! -3.40296e-06 | -3.40296e-06 | -3.40296e-06
! -3.40056e-06 | -3.40056e-06 | -3.40056e-06
! -3.39789e-06 | -3.39789e-06 | -3.39789e-06
! -3.39752e-06 | -3.39752e-06 | -3.39752e-06
! -3.39735e-06 | -3.39735e-06 | -3.39735e-06
! -3.40043e-06 | -3.40043e-06 | -3.40043e-06
! -3.39825e-06 | -3.39825e-06 | -3.39825e-06
! -3.39722e-06 | -3.39722e-06 | -3.39722e-06
! -3.39715e-06 | -3.39715e-06 | -3.39715e-06
! -3.39714e-06 | -3.39714e-06 | -3.39714e-06
! -3.397e-06 | -3.397e-06 | -3.397e-06
! -3.39695e-06 | -3.39695e-06 | -3.39695e-06
! -3.40275e-06 | -3.40275e-06 | -3.40275e-06
! -3.39669e-06 | -3.39669e-06 | -3.39669e-06
! -3.39641e-06 | -3.39641e-06 | -3.39641e-06
! -3.39605e-06 | -3.39605e-06 | -3.39605e-06
! -3.39604e-06 | -3.39604e-06 | -3.39604e-06
! -3.4025e-06 | -3.4025e-06 | -3.4025e-06
! -3.40257e-06 | -3.40257e-06 | -3.40257e-06
! -3.39517e-06 | -3.39517e-06 | -3.39517e-06
! -3.39504e-06 | -3.39504e-06 | -3.39504e-06
! -3.39503e-06 | -3.39503e-06 | -3.39503e-06
! -3.39503e-06 | -3.39503e-06 | -3.39503e-06
! -3.39489e-06 | -3.39489e-06 | -3.39489e-06
! -3.39509e-06 | -3.39509e-06 | -3.39509e-06
! -3.39464e-06 | -3.39464e-06 | -3.39464e-06
! -3.39463e-06 | -3.39463e-06 | -3.39463e-06
! -3.40236e-06 | -3.40236e-06 | -3.40236e-06
! -3.39408e-06 | -3.39408e-06 | -3.39408e-06
! -3.39361e-06 | -3.39361e-06 | -3.39361e-06
! -3.3936e-06 | -3.3936e-06 | -3.3936e-06
! -3.39387e-06 | -3.39387e-06 | -3.39387e-06
! -3.39408e-06 | -3.39408e-06 | -3.39408e-06
! -3.39333e-06 | -3.39333e-06 | -3.39333e-06
! -3.39309e-06 | -3.39309e-06 | -3.39309e-06
! -3.39308e-06 | -3.39308e-06 | -3.39308e-06
! -3.39307e-06 | -3.39307e-06 | -3.39307e-06
! -3.39311e-06 | -3.39311e-06 | -3.39311e-06
! -3.39279e-06 | -3.39279e-06 | -3.39279e-06
! -3.39287e-06 | -3.39287e-06 | -3.39287e-06
! -3.39274e-06 | -3.39274e-06 | -3.39274e-06
! -3.39273e-06 | -3.39273e-06 | -3.39273e-06
! -3.39272e-06 | -3.39272e-06 | -3.39272e-06
! -3.39258e-06 | -3.39258e-06 | -3.39258e-06
! -3.39257e-06 | -3.39257e-06 | -3.39257e-06
! -3.39257e-06 | -3.39257e-06 | -3.39257e-06
! -3.39256e-06 | -3.39256e-06 | -3.39256e-06
! -3.39255e-06 | -3.39255e-06 | -3.39255e-06
! -3.39227e-06 | -3.39227e-06 | -3.39227e-06
! -3.39228e-06 | -3.39228e-06 | -3.39228e-06
! -3.39186e-06 | -3.39186e-06 | -3.39186e-06
! -3.39161e-06 | -3.39161e-06 | -3.39161e-06
! -3.39155e-06 | -3.39155e-06 | -3.39155e-06
! -3.39148e-06 | -3.39148e-06 | -3.39148e-06
! -3.39147e-06 | -3.39147e-06 | -3.39147e-06
! -3.39128e-06 | -3.39128e-06 | -3.39128e-06
! -3.39153e-06 | -3.39153e-06 | -3.39153e-06
! -3.39126e-06 | -3.39126e-06 | -3.39126e-06
! -3.39106e-06 | -3.39106e-06 | -3.39106e-06
! -3.39105e-06 | -3.39105e-06 | -3.39105e-06
! -3.39105e-06 | -3.39105e-06 | -3.39105e-06
! -3.39074e-06 | -3.39074e-06 | -3.39074e-06
! -3.39104e-06 | -3.39104e-06 | -3.39104e-06
! -3.39103e-06 | -3.39103e-06 | -3.39103e-06
! -3.3911e-06 | -3.3911e-06 | -3.3911e-06
! -3.39841e-06 | -3.39841e-06 | -3.39841e-06
! -3.3904e-06 | -3.3904e-06 | -3.3904e-06
! -3.3896e-06 | -3.3896e-06 | -3.3896e-06
! -3.38943e-06 | -3.38943e-06 | -3.38943e-06
! -3.38936e-06 | -3.38936e-06 | -3.38936e-06
! -3.38925e-06 | -3.38925e-06 | -3.38925e-06
! -3.38924e-06 | -3.38924e-06 | -3.38924e-06
! -3.39837e-06 | -3.39837e-06 | -3.39837e-06
! -3.38867e-06 | -3.38867e-06 | -3.38867e-06
! -3.38885e-06 | -3.38885e-06 | -3.38885e-06
! -3.38815e-06 | -3.38815e-06 | -3.38815e-06
! -3.38805e-06 | -3.38805e-06 | -3.38805e-06
! -3.38735e-06 | -3.38735e-06 | -3.38735e-06
! -3.38735e-06 | -3.38735e-06 | -3.38735e-06
! -3.38705e-06 | -3.38705e-06 | -3.38705e-06
! -3.38742e-06 | -3.38742e-06 | -3.38742e-06
! -3.38694e-06 | -3.38694e-06 | -3.38694e-06
! -3.38672e-06 | -3.38672e-06 | -3.38672e-06
! -3.38673e-06 | -3.38673e-06 | -3.38673e-06
! -3.38624e-06 | -3.38624e-06 | -3.38624e-06
! -3.38618e-06 | -3.38618e-06 | -3.38618e-06
! -3.38581e-06 | -3.38581e-06 | -3.38581e-06
! -3.38561e-06 | -3.38561e-06 | -3.38561e-06
! -3.38583e-06 | -3.38583e-06 | -3.38583e-06
! -3.3988e-06 | -3.3988e-06 | -3.3988e-06
! -3.38541e-06 | -3.38541e-06 | -3.38541e-06
! -3.38512e-06 | -3.38512e-06 | -3.38512e-06
! -3.38548e-06 | -3.38548e-06 | -3.38548e-06
! -3.39831e-06 | -3.39831e-06 | -3.39831e-06
! -3.3848e-06 | -3.3848e-06 | -3.3848e-06
! -3.38396e-06 | -3.38396e-06 | -3.38396e-06
! -3.38423e-06 | -3.38423e-06 | -3.38423e-06
! -3.3981e-06 | -3.3981e-06 | -3.3981e-06
! -3.3839e-06 | -3.3839e-06 | -3.3839e-06
! -3.38392e-06 | -3.38392e-06 | -3.38392e-06
! -3.3836e-06 | -3.3836e-06 | -3.3836e-06
! -3.3836e-06 | -3.3836e-06 | -3.3836e-06
! -3.38369e-06 | -3.38369e-06 | -3.38369e-06
! -3.38304e-06 | -3.38304e-06 | -3.38304e-06
! -3.38309e-06 | -3.38309e-06 | -3.38309e-06
! -3.3827e-06 | -3.3827e-06 | -3.3827e-06
! -3.38223e-06 | -3.38223e-06 | -3.38223e-06
! -3.38222e-06 | -3.38222e-06 | -3.38222e-06
! -3.38207e-06 | -3.38207e-06 | -3.38207e-06
! -3.3819e-06 | -3.3819e-06 | -3.3819e-06
! -3.38171e-06 | -3.38171e-06 | -3.38171e-06
! -3.38131e-06 | -3.38131e-06 | -3.38131e-06
! -3.38141e-06 | -3.38141e-06 | -3.38141e-06
! -3.38147e-06 | -3.38147e-06 | -3.38147e-06
! -3.38122e-06 | -3.38122e-06 | -3.38122e-06
! -3.38118e-06 | -3.38118e-06 | -3.38118e-06
! -3.38117e-06 | -3.38117e-06 | -3.38117e-06
! -3.38084e-06 | -3.38084e-06 | -3.38084e-06
! -3.38079e-06 | -3.38079e-06 | -3.38079e-06
! -3.3807e-06 | -3.3807e-06 | -3.3807e-06
! -3.38069e-06 | -3.38069e-06 | -3.38069e-06
! -3.38068e-06 | -3.38068e-06 | -3.38068e-06
! -3.38077e-06 | -3.38077e-06 | -3.38077e-06
! -3.38057e-06 | -3.38057e-06 | -3.38057e-06
! -3.3805e-06 | -3.3805e-06 | -3.3805e-06
! -3.39849e-06 | -3.39849e-06 | -3.39849e-06
! -3.39848e-06 | -3.39848e-06 | -3.39848e-06
! -3.38048e-06 | -3.38048e-06 | -3.38048e-06
! -3.38036e-06 | -3.38036e-06 | -3.38036e-06
! -3.38032e-06 | -3.38032e-06 | -3.38032e-06
! -3.3803e-06 | -3.3803e-06 | -3.3803e-06
! -3.38026e-06 | -3.38026e-06 | -3.38026e-06
! -3.40282e-06 | -3.40282e-06 | -3.40282e-06
! -3.38017e-06 | -3.38017e-06 | -3.38017e-06
! -3.38017e-06 | -3.38017e-06 | -3.38017e-06
! -3.38016e-06 | -3.38016e-06 | -3.38016e-06
! -3.38014e-06 | -3.38014e-06 | -3.38014e-06
! -3.40054e-06 | -3.40054e-06 | -3.40054e-06
! -3.38008e-06 | -3.38008e-06 | -3.38008e-06
! -3.38007e-06 | -3.38007e-06 | -3.38007e-06
! -3.38004e-06 | -3.38004e-06 | -3.38004e-06
| |
(144 rows)
So clearly the data is not being passed around as expected.
Please note that this is a completely new test -- prior to this commit,
these functions were not being called at all. So it is quite possible
that they have been failed for long.
I assume this is just some dumb portability mistake on my part ... or
perhaps the fact that the functions are still using v0 fmgr convention?
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@commandprompt.com> writes:
> I assume this is just some dumb portability mistake on my part ... or
> perhaps the fact that the functions are still using v0 fmgr convention?
Since they're v0, they'd have to explicitly know about the pass-by-ref
status of float4.
Did this patch include a compile-time choice of whether things could
remain pass-by-ref? I rather imagine that some people out there will
prefer to stay that way instead of fix their old v0 code.
In the meantime, converting contrib/seg to v1 might be the best
solution.
regards, tom lane
Tom Lane wrote: > Alvaro Herrera <alvherre@commandprompt.com> writes: > > I assume this is just some dumb portability mistake on my part ... or > > perhaps the fact that the functions are still using v0 fmgr convention? > > Since they're v0, they'd have to explicitly know about the pass-by-ref > status of float4. Well, the previous code was doing some pallocs, and the new code is not: http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/contrib/seg/seg.c.diff?r1=1.20;r2=1.21 > Did this patch include a compile-time choice of whether things could > remain pass-by-ref? I rather imagine that some people out there will > prefer to stay that way instead of fix their old v0 code. Hmm, nope. Do we really need that? I understand the backwards-compatibility argument, yet I wonder if it's worth the extra effort and code complexity. > In the meantime, converting contrib/seg to v1 might be the best > solution. Will do. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@commandprompt.com> writes: > Tom Lane wrote: >> Since they're v0, they'd have to explicitly know about the pass-by-ref >> status of float4. > Well, the previous code was doing some pallocs, and the new code is not: > http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/contrib/seg/seg.c.diff?r1=1.20;r2=1.21 [ shrug... ] So, you missed something. >> Did this patch include a compile-time choice of whether things could >> remain pass-by-ref? I rather imagine that some people out there will >> prefer to stay that way instead of fix their old v0 code. > Hmm, nope. Do we really need that? Given that we *have to* handle a compile-time choice for whether float8 is pass-by-ref, I should think that allowing a similar choice for float4 is perfectly sensible and not really more work (it'll just be a second instance of the same code pattern). I'm not at all sure it made sense to apply this portion of the patch separately. regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > Alvaro Herrera <alvherre@commandprompt.com> writes: >> Tom Lane wrote: >>> Since they're v0, they'd have to explicitly know about the pass-by-ref >>> status of float4. >> Well, the previous code was doing some pallocs, and the new code is not: >> http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/contrib/seg/seg.c.diff?r1=1.20;r2=1.21 > [ shrug... ] So, you missed something. Specifically, I think what you missed is that on some platforms C functions pass or return float values differently from similar-sized integer or pointer values (typically, the float values get passed in floating-point registers). On such platforms it is essentially impossible for a v0 function to work with pass-by-val float4 --- since fmgr_oldstyle thinks it's passing integers and getting back pointers, the values are being transferred in the wrong registers. The only way to make it work would be to mangle the function declarations and then play union tricks like those in Float4GetDatum/DatumGetFloat4, which is certainly not very practical. It'd be less ugly to convert to v1 calling convention. (This very likely explains why the Berkeley folk made float4 pass-by-ref in the first place, a decision that doesn't make a lot of sense if you don't know about this problem.) So I think we really do need to offer that compile-time option. Failing to do so will be tantamount to desupporting v0 functions altogether, which I don't think we're prepared to do. regards, tom lane
Tom Lane wrote: > Specifically, I think what you missed is that on some platforms C > functions pass or return float values differently from similar-sized > integer or pointer values (typically, the float values get passed in > floating-point registers). Argh ... I would have certainly missed that. > It'd be less ugly to convert to v1 calling convention. Okay -- I'll change contrib/seg to v1 to greenify back the buildfarm. > So I think we really do need to offer that compile-time option. > Failing to do so will be tantamount to desupporting v0 functions > altogether, which I don't think we're prepared to do. I have asked the Cybertec guys for a patch. Since it's basically a copy of the float8 change, it should be easy to do. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
"Alvaro Herrera" <alvherre@commandprompt.com> writes: > Tom Lane wrote: > >> Specifically, I think what you missed is that on some platforms C >> functions pass or return float values differently from similar-sized >> integer or pointer values (typically, the float values get passed in >> floating-point registers). > > Argh ... I would have certainly missed that. Hum. That's a valid concern for some platforms, Sparc I think? But I'm skeptical that it would hit such a wide swathe of the build farm. In particular AFAIK the standard ABI for i386 does no such thing. You can get behaviour like that from GCC using function attributes like regparam but it's not the default. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's Slony Replication support!
Gregory Stark <stark@enterprisedb.com> writes:
>> Tom Lane wrote:
>>> Specifically, I think what you missed is that on some platforms C
>>> functions pass or return float values differently from similar-sized
>>> integer or pointer values (typically, the float values get passed in
>>> floating-point registers).
> Hum. That's a valid concern for some platforms, Sparc I think?
> But I'm skeptical that it would hit such a wide swathe of the build
> farm.
I was wondering about that too, once it became obvious that (almost?)
everything was failing not just some platforms. However, this
afternoon's CVS HEAD *does* pass the seg regression test for me on HPPA,
and I presume it passed on whatever Alvaro is using (btw, what was
that?). So there's definitely a platform dependency involved and not
just you-missed-a-pointer-someplace.
regards, tom lane
Tom Lane wrote: > Gregory Stark <stark@enterprisedb.com> writes: > >> Tom Lane wrote: > >>> Specifically, I think what you missed is that on some platforms C > >>> functions pass or return float values differently from similar-sized > >>> integer or pointer values (typically, the float values get passed in > >>> floating-point registers). > > > Hum. That's a valid concern for some platforms, Sparc I think? > > But I'm skeptical that it would hit such a wide swathe of the build > > farm. > > I was wondering about that too, once it became obvious that (almost?) > everything was failing not just some platforms. However, this > afternoon's CVS HEAD *does* pass the seg regression test for me on HPPA, > and I presume it passed on whatever Alvaro is using (btw, what was > that?). So there's definitely a platform dependency involved and not > just you-missed-a-pointer-someplace. Yeah, it passed for me -- this is a dual-core AMD X2 (amd64), gcc 4.1.3. Also, it didn't fail in the same way everywhere: for example, fennec (x86-64, gcc 4.1) shows everything returned as zeros instead of random values that most other platforms show. Further fumbling says that fennec, chinchilla, heron, dungbeetle and platypus display zeroes. These are all the AMD64 machines (different operating systems: mostly Linux, one FreeBSD) that managed to run during the time that the bug was out there. Dugong failed differently, returning NaN. The others returned random pointers, all in the same area (they all display the same power of 10 values, presumably random but nearby stack or data addresses or something like that). http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=fennec&dt=2008-04-18%2021:10:01 *************** *** 1070,1218 **** FROM test_seg WHERE s @> '11.2..11.3' OR s IS NULL ORDER BY s; seg_lower | seg_center | seg_upper -----------+------------+----------- ! -Infinity | -Infinity | 40 ! -Infinity | -Infinity | 82 ! -Infinity | -Infinity | 90 ! 1 | 7 | 13 ! 1.3 | 6.65 | 12 ! 2 | 6.75 | 11.5 ! 2.1 | 6.95 | 11.8 ! 2.3 | Infinity | Infinity ! 2.3 | Infinity | Infinity ! 2.4 | 6.85 | 11.3 ! 2.5 | 7 | 11.5 ! 2.5 | 7.15 | 11.8 ! 2.6 | Infinity | Infinity ! 2.7 | 7.35 | 12 ! 3 | Infinity | Infinity ! 3 | 30.5 | 58 ! 3.1 | 7.3 | 11.5 ! 3.5 | 7.5 | 11.5 ! 3.5 | 7.85 | 12.2 ! 4 | 8 | 12 ! 4 | Infinity | Infinity ! 4 | 8 | 12 ! 4 | 7.85 | 11.7 ! 4 | 8.25 | 12.5 ! 4 | 8.5 | 13 ! 4 | 32 | 60 ! 4 | Infinity | Infinity ! 4.2 | 7.85 | 11.5 ! 4.2 | 7.95 | 11.7 ! 4.5 | 8.25 | 12 ! 4.5 | 8 | 11.5 ! 4.5 | 8.25 | 12 ! 4.5 | 8.25 | 12 ! 4.5 | 8.5 | 12.5 ! 4.5 | 59.75 | 115 ! 4.7 | 8.25 | 11.8 ! 4.8 | 8.15 | 11.5 ! 4.8 | 8.2 | 11.6 ! 4.8 | 8.65 | 12.5 ! 4.8 | Infinity | Infinity ! 4.9 | 8.45 | 12 ! 4.9 | Infinity | Infinity ! 5 | 8.25 | 11.5 ! 5 | 8.5 | 12 ! 5 | 17.5 | 30 ! 5 | 8.2 | 11.4 ! 5 | 8.25 | 11.5 ! 5 | 8.3 | 11.6 ! 5 | 8.35 | 11.7 ! 5 | 8.5 | 12 ! 5 | 8.5 | 12 ! 5 | 8.5 | 12 ! 5.2 | 8.35 | 11.5 ! 5.2 | 8.6 | 12 ! 5.25 | 8.625 | 12 ! 5.3 | 8.4 | 11.5 ! 5.3 | 9.15 | 13 ! 5.3 | 47.65 | 90 ! 5.3 | Infinity | Infinity ! 5.4 | Infinity | Infinity ! 5.5 | 8.5 | 11.5 ! 5.5 | 8.6 | 11.7 ! 5.5 | 8.75 | 12 ! 5.5 | 8.75 | 12 ! 5.5 | 9 | 12.5 ! 5.5 | 9.5 | 13.5 ! 5.5 | Infinity | Infinity ! 5.5 | Infinity | Infinity ! 5.7 | Infinity | Infinity ! 5.9 | Infinity | Infinity ! 6 | 8.75 | 11.5 ! 6 | 9 | 12 ! 6 | 8.75 | 11.5 ! 6 | 9.5 | 13 ! 6 | 8.75 | 11.5 ! 6.1 | 9.05 | 12 ! 6.1 | Infinity | Infinity ! 6.2 | 8.85 | 11.5 ! 6.3 | Infinity | Infinity ! 6.5 | 9 | 11.5 ! 6.5 | 9.25 | 12 ! 6.5 | 9.25 | 12 ! 6.5 | Infinity | Infinity ! 6.6 | Infinity | Infinity ! 6.7 | 9.1 | 11.5 ! 6.7 | Infinity | Infinity ! 6.75 | Infinity | Infinity ! 6.8 | Infinity | Infinity ! 6.9 | 9.55 | 12.2 ! 6.9 | 48.45 | 90 ! 6.9 | Infinity | Infinity ! 7 | 9.25 | 11.5 ! 7 | 9.25 | 11.5 ! 7 | 9.25 | 11.5 ! 7 | Infinity | Infinity ! 7.15 | Infinity | Infinity ! 7.2 | 10.35 | 13.5 ! 7.3 | 48.65 | 90 ! 7.3 | Infinity | Infinity ! 7.3 | Infinity | Infinity ! 7.4 | 9.75 | 12.1 ! 7.4 | Infinity | Infinity ! 7.5 | 9.5 | 11.5 ! 7.5 | 9.75 | 12 ! 7.5 | Infinity | Infinity ! 7.7 | 9.6 | 11.5 ! 7.7 | Infinity | Infinity ! 7.75 | Infinity | Infinity ! 8 | 9.85 | 11.7 ! 8 | 10 | 12 ! 8 | 10.5 | 13 ! 8.2 | Infinity | Infinity ! 8.3 | Infinity | Infinity ! 8.5 | 10 | 11.5 ! 8.5 | 10.5 | 12.5 ! 8.5 | Infinity | Infinity ! 8.6 | 53.8 | 99 ! 8.7 | 10 | 11.3 ! 8.7 | 10.2 | 11.7 ! 8.9 | 10.2 | 11.5 ! 9 | 10.5 | 12 ! 9 | 10.15 | 11.3 ! 9 | 10.25 | 11.5 ! 9 | 10.5 | 12 ! 9 | Infinity | Infinity ! 9.2 | 10.6 | 12 ! 9.4 | 10.8 | 12.2 ! 9.5 | 10.75 | 12 ! 9.5 | 10.85 | 12.2 ! 9.5 | Infinity | Infinity ! 9.6 | 10.55 | 11.5 ! 9.7 | 10.6 | 11.5 ! 9.7 | 10.85 | 12 ! 9.8 | 11.15 | 12.5 ! 10 | 10.8 | 11.6 ! 10 | 10.75 | 11.5 ! 10 | 11.25 | 12.5 ! 10 | 11.25 | 12.5 ! 10.2 | 11 | 11.8 ! 10.5 | 11 | 11.5 ! 10.5 | 11 | 11.5 ! 10.5 | 12 | 13.5 ! 10.7 | 11.5 | 12.3 | | (144 rows) --- 1070,1218 ---- FROM test_seg WHERE s @> '11.2..11.3' OR s IS NULL ORDER BY s; seg_lower | seg_center | seg_upper -----------+------------+----------- ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 ! 0 | 0 | 0 | | (144 rows) Mastodon, Windows 2003, MSVC http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=mastodon&dt=2008-04-18%2019:00:00 ================= pgsql.4236/contrib/seg/regression.diffs =================== *** ./expected/seg_1.out 2008-04-18 20:00:10.593750000 +0100 --- ./results/seg.out 2008-04-18 20:16:25.375000000 +0100 *************** *** 1069,1218 **** SELECT seg_lower(s), seg_center(s), seg_upper(s) FROM test_seg WHERE s @> '11.2..11.3' OR s IS NULL ORDER BY s; seg_lower | seg_center | seg_upper ! -----------+------------+----------- ! -Infinity | -Infinity | 40 ! -Infinity | -Infinity | 82 ! -Infinity | -Infinity | 90 ! 1 | 7 | 13 ! 1.3 | 6.65 | 12 ! 2 | 6.75 | 11.5 ! 2.1 | 6.95 | 11.8 ! 2.3 | Infinity | Infinity ! 2.3 | Infinity | Infinity ! 2.4 | 6.85 | 11.3 ! 2.5 | 7 | 11.5 ! 2.5 | 7.15 | 11.8 ! 2.6 | Infinity | Infinity ! 2.7 | 7.35 | 12 ! 3 | Infinity | Infinity ! 3 | 30.5 | 58 ! 3.1 | 7.3 | 11.5 ! 3.5 | 7.5 | 11.5 ! 3.5 | 7.85 | 12.2 ! 4 | 8 | 12 ! 4 | Infinity | Infinity ! 4 | 8 | 12 ! 4 | 7.85 | 11.7 ! 4 | 8.25 | 12.5 ! 4 | 8.5 | 13 ! 4 | 32 | 60 ! 4 | Infinity | Infinity ! 4.2 | 7.85 | 11.5 ! 4.2 | 7.95 | 11.7 ! 4.5 | 8.25 | 12 ! 4.5 | 8 | 11.5 ! 4.5 | 8.25 | 12 ! 4.5 | 8.25 | 12 ! 4.5 | 8.5 | 12.5 ! 4.5 | 59.75 | 115 ! 4.7 | 8.25 | 11.8 ! 4.8 | 8.15 | 11.5 ! 4.8 | 8.2 | 11.6 ! 4.8 | 8.65 | 12.5 ! 4.8 | Infinity | Infinity ! 4.9 | 8.45 | 12 ! 4.9 | Infinity | Infinity ! 5 | 8.25 | 11.5 ! 5 | 8.5 | 12 ! 5 | 17.5 | 30 ! 5 | 8.2 | 11.4 ! 5 | 8.25 | 11.5 ! 5 | 8.3 | 11.6 ! 5 | 8.35 | 11.7 ! 5 | 8.5 | 12 ! 5 | 8.5 | 12 ! 5 | 8.5 | 12 ! 5.2 | 8.35 | 11.5 ! 5.2 | 8.6 | 12 ! 5.25 | 8.625 | 12 ! 5.3 | 8.4 | 11.5 ! 5.3 | 9.15 | 13 ! 5.3 | 47.65 | 90 ! 5.3 | Infinity | Infinity ! 5.4 | Infinity | Infinity ! 5.5 | 8.5 | 11.5 ! 5.5 | 8.6 | 11.7 ! 5.5 | 8.75 | 12 ! 5.5 | 8.75 | 12 ! 5.5 | 9 | 12.5 ! 5.5 | 9.5 | 13.5 ! 5.5 | Infinity | Infinity ! 5.5 | Infinity | Infinity ! 5.7 | Infinity | Infinity ! 5.9 | Infinity | Infinity ! 6 | 8.75 | 11.5 ! 6 | 9 | 12 ! 6 | 8.75 | 11.5 ! 6 | 9.5 | 13 ! 6 | 8.75 | 11.5 ! 6.1 | 9.05 | 12 ! 6.1 | Infinity | Infinity ! 6.2 | 8.85 | 11.5 ! 6.3 | Infinity | Infinity ! 6.5 | 9 | 11.5 ! 6.5 | 9.25 | 12 ! 6.5 | 9.25 | 12 ! 6.5 | Infinity | Infinity ! 6.6 | Infinity | Infinity ! 6.7 | 9.1 | 11.5 ! 6.7 | Infinity | Infinity ! 6.75 | Infinity | Infinity ! 6.8 | Infinity | Infinity ! 6.9 | 9.55 | 12.2 ! 6.9 | 48.45 | 90 ! 6.9 | Infinity | Infinity ! 7 | 9.25 | 11.5 ! 7 | 9.25 | 11.5 ! 7 | 9.25 | 11.5 ! 7 | Infinity | Infinity ! 7.15 | Infinity | Infinity ! 7.2 | 10.35 | 13.5 ! 7.3 | 48.65 | 90 ! 7.3 | Infinity | Infinity ! 7.3 | Infinity | Infinity ! 7.4 | 9.75 | 12.1 ! 7.4 | Infinity | Infinity ! 7.5 | 9.5 | 11.5 ! 7.5 | 9.75 | 12 ! 7.5 | Infinity | Infinity ! 7.7 | 9.6 | 11.5 ! 7.7 | Infinity | Infinity ! 7.75 | Infinity | Infinity ! 8 | 9.85 | 11.7 ! 8 | 10 | 12 ! 8 | 10.5 | 13 ! 8.2 | Infinity | Infinity ! 8.3 | Infinity | Infinity ! 8.5 | 10 | 11.5 ! 8.5 | 10.5 | 12.5 ! 8.5 | Infinity | Infinity ! 8.6 | 53.8 | 99 ! 8.7 | 10 | 11.3 ! 8.7 | 10.2 | 11.7 ! 8.9 | 10.2 | 11.5 ! 9 | 10.5 | 12 ! 9 | 10.15 | 11.3 ! 9 | 10.25 | 11.5 ! 9 | 10.5 | 12 ! 9 | Infinity | Infinity ! 9.2 | 10.6 | 12 ! 9.4 | 10.8 | 12.2 ! 9.5 | 10.75 | 12 ! 9.5 | 10.85 | 12.2 ! 9.5 | Infinity | Infinity ! 9.6 | 10.55 | 11.5 ! 9.7 | 10.6 | 11.5 ! 9.7 | 10.85 | 12 ! 9.8 | 11.15 | 12.5 ! 10 | 10.8 | 11.6 ! 10 | 10.75 | 11.5 ! 10 | 11.25 | 12.5 ! 10 | 11.25 | 12.5 ! 10.2 | 11 | 11.8 ! 10.5 | 11 | 11.5 ! 10.5 | 11 | 11.5 ! 10.5 | 12 | 13.5 ! 10.7 | 11.5 | 12.3 | | (144 rows) --- 1069,1218 ---- SELECT seg_lower(s), seg_center(s), seg_upper(s) FROM test_seg WHERE s @> '11.2..11.3' OR s IS NULL ORDER BY s; seg_lower | seg_center | seg_upper ! --------------+--------------+-------------- ! 9.04487e-038 | 9.04487e-038 | 9.04487e-038 ! 9.04408e-038 | 9.04408e-038 | 9.04408e-038 ! 9.04435e-038 | 9.04435e-038 | 9.04435e-038 ! 9.03785e-038 | 9.03785e-038 | 9.03785e-038 ! 9.0302e-038 | 9.0302e-038 | 9.0302e-038 ! 9.0297e-038 | 9.0297e-038 | 9.0297e-038 ! 9.02923e-038 | 9.02923e-038 | 9.02923e-038 ! 9.03752e-038 | 9.03752e-038 | 9.03752e-038 ! 9.03118e-038 | 9.03118e-038 | 9.03118e-038 ! 9.02887e-038 | 9.02887e-038 | 9.02887e-038 ! 9.02869e-038 | 9.02869e-038 | 9.02869e-038 ! 9.02867e-038 | 9.02867e-038 | 9.02867e-038 ! 9.02829e-038 | 9.02829e-038 | 9.02829e-038 ! 9.02813e-038 | 9.02813e-038 | 9.02813e-038 ! 9.04377e-038 | 9.04377e-038 | 9.04377e-038 ! 9.02744e-038 | 9.02744e-038 | 9.02744e-038 ! 9.02623e-038 | 9.02623e-038 | 9.02623e-038 ! 9.02522e-038 | 9.02522e-038 | 9.02522e-038 ! 9.0252e-038 | 9.0252e-038 | 9.0252e-038 ! 9.0431e-038 | 9.0431e-038 | 9.0431e-038 ! 9.0433e-038 | 9.0433e-038 | 9.0433e-038 ! 9.02336e-038 | 9.02336e-038 | 9.02336e-038 ! 9.023e-038 | 9.023e-038 | 9.023e-038 ! 9.02298e-038 | 9.02298e-038 | 9.02298e-038 ! 9.02296e-038 | 9.02296e-038 | 9.02296e-038 ! 9.0226e-038 | 9.0226e-038 | 9.0226e-038 ! 9.02314e-038 | 9.02314e-038 | 9.02314e-038 ! 9.02146e-038 | 9.02146e-038 | 9.02146e-038 ! 9.02144e-038 | 9.02144e-038 | 9.02144e-038 ! 9.04272e-038 | 9.04272e-038 | 9.04272e-038 ! 9.01994e-038 | 9.01994e-038 | 9.01994e-038 ! 9.01917e-038 | 9.01917e-038 | 9.01917e-038 ! 9.01913e-038 | 9.01913e-038 | 9.01913e-038 ! 9.01989e-038 | 9.01989e-038 | 9.01989e-038 ! 9.01991e-038 | 9.01991e-038 | 9.01991e-038 ! 9.01839e-038 | 9.01839e-038 | 9.01839e-038 ! 9.0173e-038 | 9.0173e-038 | 9.0173e-038 ! 9.01727e-038 | 9.01727e-038 | 9.01727e-038 ! 9.01725e-038 | 9.01725e-038 | 9.01725e-038 ! 9.01734e-038 | 9.01734e-038 | 9.01734e-038 ! 9.01649e-038 | 9.01649e-038 | 9.01649e-038 ! 9.01669e-038 | 9.01669e-038 | 9.01669e-038 ! 9.01633e-038 | 9.01633e-038 | 9.01633e-038 ! 9.01631e-038 | 9.01631e-038 | 9.01631e-038 ! 9.01629e-038 | 9.01629e-038 | 9.01629e-038 ! 9.0159e-038 | 9.0159e-038 | 9.0159e-038 ! 9.01588e-038 | 9.01588e-038 | 9.01588e-038 ! 9.01586e-038 | 9.01586e-038 | 9.01586e-038 ! 9.01584e-038 | 9.01584e-038 | 9.01584e-038 ! 9.01582e-038 | 9.01582e-038 | 9.01582e-038 ! 9.01505e-038 | 9.01505e-038 | 9.01505e-038 ! 9.01508e-038 | 9.01508e-038 | 9.01508e-038 ! 9.01445e-038 | 9.01445e-038 | 9.01445e-038 ! 9.01378e-038 | 9.01378e-038 | 9.01378e-038 ! 9.0136e-038 | 9.0136e-038 | 9.0136e-038 ! 9.01342e-038 | 9.01342e-038 | 9.01342e-038 ! 9.01339e-038 | 9.01339e-038 | 9.01339e-038 ! 9.01243e-038 | 9.01243e-038 | 9.01243e-038 ! 9.01355e-038 | 9.01355e-038 | 9.01355e-038 ! 9.01237e-038 | 9.01237e-038 | 9.01237e-038 ! 9.01183e-038 | 9.01183e-038 | 9.01183e-038 ! 9.01181e-038 | 9.01181e-038 | 9.01181e-038 ! 9.01178e-038 | 9.01178e-038 | 9.01178e-038 ! 9.01093e-038 | 9.01093e-038 | 9.01093e-038 ! 9.01176e-038 | 9.01176e-038 | 9.01176e-038 ! 9.01174e-038 | 9.01174e-038 | 9.01174e-038 ! 9.01194e-038 | 9.01194e-038 | 9.01194e-038 ! 9.03205e-038 | 9.03205e-038 | 9.03205e-038 ! 9.01001e-038 | 9.01001e-038 | 9.01001e-038 ! 9.00791e-038 | 9.00791e-038 | 9.00791e-038 ! 9.00744e-038 | 9.00744e-038 | 9.00744e-038 ! 9.00724e-038 | 9.00724e-038 | 9.00724e-038 ! 9.00695e-038 | 9.00695e-038 | 9.00695e-038 ! 9.00692e-038 | 9.00692e-038 | 9.00692e-038 ! 9.03196e-038 | 9.03196e-038 | 9.03196e-038 ! 9.00535e-038 | 9.00535e-038 | 9.00535e-038 ! 9.00585e-038 | 9.00585e-038 | 9.00585e-038 ! 9.00446e-038 | 9.00446e-038 | 9.00446e-038 ! 9.00419e-038 | 9.00419e-038 | 9.00419e-038 ! 9.00184e-038 | 9.00184e-038 | 9.00184e-038 ! 9.00182e-038 | 9.00182e-038 | 9.00182e-038 ! 9.00101e-038 | 9.00101e-038 | 9.00101e-038 ! 9.00202e-038 | 9.00202e-038 | 9.00202e-038 ! 9.00072e-038 | 9.00072e-038 | 9.00072e-038 ! 9.00009e-038 | 9.00009e-038 | 9.00009e-038 ! 9.00013e-038 | 9.00013e-038 | 9.00013e-038 ! 8.99886e-038 | 8.99886e-038 | 8.99886e-038 ! 8.9987e-038 | 8.9987e-038 | 8.9987e-038 ! 8.99769e-038 | 8.99769e-038 | 8.99769e-038 ! 8.99713e-038 | 8.99713e-038 | 8.99713e-038 ! 8.99774e-038 | 8.99774e-038 | 8.99774e-038 ! 9.03313e-038 | 9.03313e-038 | 9.03313e-038 ! 8.9966e-038 | 8.9966e-038 | 8.9966e-038 ! 8.99581e-038 | 8.99581e-038 | 8.99581e-038 ! 8.9968e-038 | 8.9968e-038 | 8.9968e-038 ! 9.03178e-038 | 9.03178e-038 | 9.03178e-038 ! 8.99494e-038 | 8.99494e-038 | 8.99494e-038 ! 8.9927e-038 | 8.9927e-038 | 8.9927e-038 ! 8.99346e-038 | 8.99346e-038 | 8.99346e-038 ! 9.03078e-038 | 9.03078e-038 | 9.03078e-038 ! 8.99254e-038 | 8.99254e-038 | 8.99254e-038 ! 8.99261e-038 | 8.99261e-038 | 8.99261e-038 ! 8.99174e-038 | 8.99174e-038 | 8.99174e-038 ! 8.99171e-038 | 8.99171e-038 | 8.99171e-038 ! 8.99196e-038 | 8.99196e-038 | 8.99196e-038 ! 8.98974e-038 | 8.98974e-038 | 8.98974e-038 ! 8.98988e-038 | 8.98988e-038 | 8.98988e-038 ! 8.98934e-038 | 8.98934e-038 | 8.98934e-038 ! 8.98806e-038 | 8.98806e-038 | 8.98806e-038 ! 8.98804e-038 | 8.98804e-038 | 8.98804e-038 ! 8.98761e-038 | 8.98761e-038 | 8.98761e-038 ! 8.98714e-038 | 8.98714e-038 | 8.98714e-038 ! 8.98663e-038 | 8.98663e-038 | 8.98663e-038 ! 8.98509e-038 | 8.98509e-038 | 8.98509e-038 ! 8.9858e-038 | 8.9858e-038 | 8.9858e-038 ! 8.98598e-038 | 8.98598e-038 | 8.98598e-038 ! 8.98484e-038 | 8.98484e-038 | 8.98484e-038 ! 8.98473e-038 | 8.98473e-038 | 8.98473e-038 ! 8.9847e-038 | 8.9847e-038 | 8.9847e-038 ! 8.98432e-038 | 8.98432e-038 | 8.98432e-038 ! 8.98419e-038 | 8.98419e-038 | 8.98419e-038 ! 8.98394e-038 | 8.98394e-038 | 8.98394e-038 ! 8.98392e-038 | 8.98392e-038 | 8.98392e-038 ! 8.9839e-038 | 8.9839e-038 | 8.9839e-038 ! 8.98414e-038 | 8.98414e-038 | 8.98414e-038 ! 8.98361e-038 | 8.98361e-038 | 8.98361e-038 ! 8.9834e-038 | 8.9834e-038 | 8.9834e-038 ! 9.03228e-038 | 9.03228e-038 | 9.03228e-038 ! 9.03225e-038 | 9.03225e-038 | 9.03225e-038 ! 8.98334e-038 | 8.98334e-038 | 8.98334e-038 ! 8.98302e-038 | 8.98302e-038 | 8.98302e-038 ! 8.98291e-038 | 8.98291e-038 | 8.98291e-038 ! 8.98287e-038 | 8.98287e-038 | 8.98287e-038 ! 8.98273e-038 | 8.98273e-038 | 8.98273e-038 ! 9.04397e-038 | 9.04397e-038 | 9.04397e-038 ! 8.98251e-038 | 8.98251e-038 | 8.98251e-038 ! 8.98248e-038 | 8.98248e-038 | 8.98248e-038 ! 8.98246e-038 | 8.98246e-038 | 8.98246e-038 ! 8.98242e-038 | 8.98242e-038 | 8.98242e-038 ! 9.03781e-038 | 9.03781e-038 | 9.03781e-038 ! 8.98224e-038 | 8.98224e-038 | 8.98224e-038 ! 8.98222e-038 | 8.98222e-038 | 8.98222e-038 ! 8.98215e-038 | 8.98215e-038 | 8.98215e-038 | | (144 rows) -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Tom Lane wrote: > I was wondering about that too, once it became obvious that (almost?) > everything was failing not just some platforms. However, this > afternoon's CVS HEAD *does* pass the seg regression test for me on HPPA, > and I presume it passed on whatever Alvaro is using (btw, what was > that?). So there's definitely a platform dependency involved and not > just you-missed-a-pointer-someplace. Huh, this was with the code with v0 conventions, right? I committed the change to v1 conventions a while ago. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Tom Lane wrote:
>> I was wondering about that too, once it became obvious that (almost?)
>> everything was failing not just some platforms. However, this
>> afternoon's CVS HEAD *does* pass the seg regression test for me on HPPA,
>> and I presume it passed on whatever Alvaro is using (btw, what was
>> that?). So there's definitely a platform dependency involved and not
>> just you-missed-a-pointer-someplace.
> Huh, this was with the code with v0 conventions, right? I committed the
> change to v1 conventions a while ago.
Right, I had intentionally cvs updated to the broken version to test.
regards, tom lane
Alvaro Herrera írta: > Tom Lane wrote: > > >> Specifically, I think what you missed is that on some platforms C >> functions pass or return float values differently from similar-sized >> integer or pointer values (typically, the float values get passed in >> floating-point registers). >> > > Argh ... I would have certainly missed that. > > >> It'd be less ugly to convert to v1 calling convention. >> > > Okay -- I'll change contrib/seg to v1 to greenify back the buildfarm. > > >> So I think we really do need to offer that compile-time option. >> Failing to do so will be tantamount to desupporting v0 functions >> altogether, which I don't think we're prepared to do. >> > > I have asked the Cybertec guys for a patch. Since it's basically a copy > of the float8 change, it should be easy to do. > Here's the patch (against current CVS) with the required change. Please review, it passed make check with both --enable and --disable-float4-byval. -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH http://www.postgresql.at/
Attachment
Zoltan Boszormenyi wrote: >>> So I think we really do need to offer that compile-time option. >>> Failing to do so will be tantamount to desupporting v0 functions >>> altogether, which I don't think we're prepared to do. >>> >> >> I have asked the Cybertec guys for a patch. Since it's basically a copy >> of the float8 change, it should be easy to do. > > Here's the patch (against current CVS) with the required change. > Please review, it passed make check with both --enable and > --disable-float4-byval. Does it pass "make installcheck" in contrib? I'm worried about btree_gist in particular. Perhaps the change I introduced in the previous revision needs to be #ifdef'd out? -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera írta: > Zoltan Boszormenyi wrote: > > >>>> So I think we really do need to offer that compile-time option. >>>> Failing to do so will be tantamount to desupporting v0 functions >>>> altogether, which I don't think we're prepared to do. >>>> >>>> >>> I have asked the Cybertec guys for a patch. Since it's basically a copy >>> of the float8 change, it should be easy to do. >>> >> Here's the patch (against current CVS) with the required change. >> Please review, it passed make check with both --enable and >> --disable-float4-byval. >> > > Does it pass "make installcheck" in contrib? I'm worried about > It seems to pass, see below. > btree_gist in particular. Perhaps the change I introduced in the > previous revision needs to be #ifdef'd out? > Both --enable and --disable-float4-byval produced only this regression, but it seems to be a tuple order difference. ============= running regression test queries ============== test tsearch2 ... FAILED cat tsearch2/regression.diffs: *** ./expected/tsearch2.out Tue Nov 20 05:23:10 2007 --- ./results/tsearch2.out Sat Apr 19 20:48:16 2008 *************** *** 1490,1497 **** w0 | 29 | 29 y9 | 29 | 29 zm | 29 | 29 - zs | 29 | 29 zy | 29 | 29 ax | 28 | 28 cd | 28 | 28 dj | 28 | 28 --- 1490,1497 ---- w0 | 29 | 29 y9 | 29 | 29 zm | 29 | 29 zy | 29 | 29 + zs | 29 | 29 ax | 28 | 28 cd | 28 | 28 dj | 28 | 28 ====================================================================== -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH http://www.postgresql.at/
Zoltan Boszormenyi <zb@cybertec.at> writes:
> Both --enable and --disable-float4-byval produced only this regression,
> but it seems to be a tuple order difference.
That looks suspiciously locale-ish; what locale are you running PG in?
regards, tom lane
Gregory Stark <stark@enterprisedb.com> writes:
>> Tom Lane wrote:
>>> Specifically, I think what you missed is that on some platforms C
>>> functions pass or return float values differently from similar-sized
>>> integer or pointer values (typically, the float values get passed in
>>> floating-point registers).
> But I'm skeptical that it would hit such a wide swathe of the build farm. In
> particular AFAIK the standard ABI for i386 does no such thing.
I did some digging, and it seems you're mistaken. The standard gcc ABI
for both i386 and x86_64 returns floats in float registers (387
registers in the first case, and SSE registers in the second case).
This appears to have been the case for a very long time. I quote from
the manual for gcc 2.95:
`-mno-fp-ret-in-387'
Do not use the FPU registers for return values of functions.
The usual calling convention has functions return values of types
`float' and `double' in an FPU register, even if there is no FPU.
The idea is that the operating system should emulate an FPU.
The option `-mno-fp-ret-in-387' causes such values to be returned
in ordinary CPU registers instead.
It seems very odd that Alvaro's testing on an AMD64 platform didn't
show the problem.
regards, tom lane
Tom Lane írta: > Zoltan Boszormenyi <zb@cybertec.at> writes: > >> Both --enable and --disable-float4-byval produced only this regression, >> but it seems to be a tuple order difference. >> > > That looks suspiciously locale-ish; what locale are you running PG in? > > regards, tom lane > > hu_HU.UTF-8 -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH http://www.postgresql.at/
BTW, I trolled the contrib files for other v0 functions taking or
returning float4 or float8. I found seg_size (fixed it) and a whole
bunch of functions in earthdistance. Those use float8 not float4,
so they are not broken by this patch, but that module will have to
be v1-ified before we can consider applying the other part of the
patch.
regards, tom lane
Alvaro Herrera <alvherre@commandprompt.com> writes:
> With contrib/seg also adjusted to use float4 instead of float32, and
> thus the last usage of float32 gone, I am now wondering if it would be a
> good idea to remove the float32 and float32data definitions in c.h.
Well, there might still be some references to them in add-on code, but
considering that c.h has said this since 7.1
* XXX: these typedefs are now deprecated in favor of float4 and float8.
* They will eventually go away.
it seems hard to argue that we've not given adequate notice. I'm +1
for removing them, and float64/float64data too. It's not like they're
hard to avoid using, even if you don't want to update to v1 call
convention.
If there are no objections, I'll see about pushing in the remaining
parts of the patch (the original and the just-submitted fix to allow
float4byval to be selectable) over this weekend.
regards, tom lane
Zoltan Boszormenyi <zb@cybertec.at> writes:
>> That looks suspiciously locale-ish; what locale are you running PG in?
> hu_HU.UTF-8
Ah, and I'll bet zs sorts after zy in hu_HU.
The query is already doing an ORDER BY, so it's not at fault. I think
the only thing we could do about this is add a variant expected file
with the hu_HU sort ordering. I'd be happy to do that if it were
affecting the main regression tests, but not sure it's worth it for
contrib/tsearch2 ... thoughts?
regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes:
> BTW, I trolled the contrib files for other v0 functions taking or
> returning float4 or float8. I found seg_size (fixed it) and a whole
> bunch of functions in earthdistance.
Hmm, actually most of the "bunch" are SQL functions, there's only one
that needs fixing. Done.
regards, tom lane
Tom Lane írta: > Zoltan Boszormenyi <zb@cybertec.at> writes: > >>> That looks suspiciously locale-ish; what locale are you running PG in? >>> > > >> hu_HU.UTF-8 >> > > Ah, and I'll bet zs sorts after zy in hu_HU. > Yes, "zs" is a double letter that sorts after "z" in general un hu_HU. > The query is already doing an ORDER BY, so it's not at fault. I think > the only thing we could do about this is add a variant expected file > with the hu_HU sort ordering. I'd be happy to do that if it were > affecting the main regression tests, but not sure it's worth it for > contrib/tsearch2 ... thoughts? > > regards, tom lane > > -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH http://www.postgresql.at/
"Tom Lane" <tgl@sss.pgh.pa.us> writes: > Gregory Stark <stark@enterprisedb.com> writes: >>> Tom Lane wrote: >>>> Specifically, I think what you missed is that on some platforms C >>>> functions pass or return float values differently from similar-sized >>>> integer or pointer values (typically, the float values get passed in >>>> floating-point registers). > >> But I'm skeptical that it would hit such a wide swathe of the build farm. In >> particular AFAIK the standard ABI for i386 does no such thing. > > I did some digging, and it seems you're mistaken. The standard gcc ABI > for both i386 and x86_64 returns floats in float registers (387 > registers in the first case, and SSE registers in the second case). > This appears to have been the case for a very long time. I quote from > the manual for gcc 2.95: Ah, return values. I accidentally glossed over that point and was looking for how parameters were passed. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres support!
"Tom Lane" <tgl@sss.pgh.pa.us> writes: > BTW, I trolled the contrib files for other v0 functions taking or > returning float4 or float8. I found seg_size (fixed it) and a whole > bunch of functions in earthdistance. Those use float8 not float4, > so they are not broken by this patch, but that module will have to > be v1-ified before we can consider applying the other part of the > patch. So are you killing V0 for non-integral types? Because if not we should keep some sacrificial module to the regression tests to use to test for this problem. I don't see any way not to kill v0 for non-integral types if we want to make float4 and float8 pass-by-value. We could leave those pass-by-reference and just make bigint pass-by-value. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's On-Demand Production Tuning
Gregory Stark írta: > "Tom Lane" <tgl@sss.pgh.pa.us> writes: > > >> BTW, I trolled the contrib files for other v0 functions taking or >> returning float4 or float8. I found seg_size (fixed it) and a whole >> bunch of functions in earthdistance. Those use float8 not float4, >> so they are not broken by this patch, but that module will have to >> be v1-ified before we can consider applying the other part of the >> patch. >> > > So are you killing V0 for non-integral types? Because if not we should keep > some sacrificial module to the regression tests to use to test for this > problem. > > I don't see any way not to kill v0 for non-integral types if we want to make > float4 and float8 pass-by-value. We could leave those pass-by-reference and > just make bigint pass-by-value. > Just a note: time[stamp[tz]] also depend on either float8 or int64 and they have to be the same pass-by-value or pass-by-reference as their base storage types. There were crashes or regression failures if not. -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH http://www.postgresql.at/
Gregory Stark <stark@enterprisedb.com> writes:
> So are you killing V0 for non-integral types? Because if not we should keep
> some sacrificial module to the regression tests to use to test for this
> problem.
Well, we could potentially continue to have, say, the oldstyle
geo_distance function used when not FLOAT8PASSBYVAL. Not clear how
useful that really is though.
regards, tom lane
Zoltan Boszormenyi <zb@cybertec.at> writes:
> I tried to split the previous patch up to see where the tsearch regression
> comes from. So, it turned out that:
> - float4 conversion is risk free (patch #1)
> - float8 conversion is okay, too, if coupled with time[stamp[tz]] conversion
> (patch #2) but with int64 timestamps enabled, the next one is also
> needed:
> - int64 conversion (patch #3) is mostly okay but it is the one that's
> causing
> the tsearch regression
Applied with revisions --- mostly, supporting configure-time control
over whether pass-by-value is used.
regards, tom lane
Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
From
Tom Lane
Date:
Gregory Stark <stark@enterprisedb.com> writes:
> 4) Your problems with tsearch and timestamp etc raise an interesting problem.
> We don't need to mark this in pg_control because it's a purely a run-time
> issue and doesn't affect on-disk storage.
Just for the record, that's wrong. It's true that on-disk data isn't
affected, but the system catalog contents are, and they'd better match
what the postgres binary is going to do.
> However it does affect ABI
> compatibility with modules. Perhaps it should be added to
> PG_MODULE_MAGIC_DATA.
Very good point, especially now that it's configurable. Included
in committed patch.
regards, tom lane
On Mon, Apr 21, 2008 at 2:28 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Applied with revisions --- mostly, supporting configure-time control > over whether pass-by-value is used. Do we need buildfarm coverage of these options? -- Guillaume
"Guillaume Smet" <guillaume.smet@gmail.com> writes:
> On Mon, Apr 21, 2008 at 2:28 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Applied with revisions --- mostly, supporting configure-time control
>> over whether pass-by-value is used.
> Do we need buildfarm coverage of these options?
Wouldn't hurt, I guess, though it's not very urgent since the
non-default cases were default up till these patches.
Note that we are already exercising both ends of the
float8-byval option, so probably only --disable-float4-byval
is very interesting to test explicitly.
regards, tom lane
Tom Lane wrote: > Gregory Stark <stark@enterprisedb.com> writes: > > So are you killing V0 for non-integral types? Because if not we should keep > > some sacrificial module to the regression tests to use to test for this > > problem. > > Well, we could potentially continue to have, say, the oldstyle > geo_distance function used when not FLOAT8PASSBYVAL. Not clear how > useful that really is though. So is this whole float4/float8/int64 issue closed? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
On Mon, Apr 21, 2008 at 7:10 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Note that we are already exercising both ends of the > float8-byval option, so probably only --disable-float4-byval > is very interesting to test explicitly. I'll set up a new animal with --disable-float4-byval this week. -- Guillaume