Thread: Float output formatting options
Hi All, This relates to the recent discussion about floating point output format. The discussion was at a point where one parameter would be added to specify the number of extra digits used in fp output formatting. The parameter would have a 0 default value, a maximum of 2 and the minimum remained open for discussion. In a previous message I proposed that for double precision numbers a minimum value of -13 would be usefful. For single precision numbers this corresponds to a value of -4. I downloaded the PG sources and added two parameters (as PGC_USERSET): int extra_float4_digits, default 0, min -4, max 2 int extra_float8_digits, defualt 0, min -13, max 2 Compiled and tested for these functionalities. It is ok. The afected files are: src/backend/utils/adt/float.c src/backend/utils/misc/guc.c src/bin/psql/tab-complete.c src/backend/utils/misc/postgresql.conf.sample I used sources from Debian source package, postgresql_7.2.1-2woody2. Diff's produced with diff -u are enclosed as attachments. Can you comment on this (particularly the min values) ? Also, if we concluded that there is a need of 2 more digits, should'nt this be the default ? Best regards, Pedro M. Ferreira -- ---------------------------------------------------------------------- Pedro Miguel Frazao Fernandes Ferreira Universidade do Algarve Faculdade de Ciencias e Tecnologia Campus de Gambelas 8000-117 Faro Portugal Tel./Fax: (+351) 289 800950 / 289 819403 http://w3.ualg.pt/~pfrazao --- old/postgresql-7.2.1/src/backend/utils/adt/float.c Tue Dec 11 02:02:12 2001 +++ postgresql-7.2.1/src/backend/utils/adt/float.c Mon Nov 4 10:32:33 2002 @@ -65,6 +65,12 @@ #include "utils/array.h" #include "utils/builtins.h" +/* + * Configuration options for float4 and float8 extra digits in output format + */ + +int extra_float4_digits; +int extra_float8_digits; #if !(NeXT && NX_CURRENT_COMPILER_RELEASE > NX_COMPILER_RELEASE_3_2) /* NS3.3 has conflicting declarations of these in <math.h> */ @@ -237,7 +243,7 @@ if (infflag < 0) PG_RETURN_CSTRING(strcpy(ascii, "-Infinity")); - sprintf(ascii, "%.*g", FLT_DIG, num); + sprintf(ascii, "%.*g", FLT_DIG+extra_float4_digits, num); PG_RETURN_CSTRING(ascii); } @@ -299,7 +305,7 @@ if (infflag < 0) PG_RETURN_CSTRING(strcpy(ascii, "-Infinity")); - sprintf(ascii, "%.*g", DBL_DIG, num); + sprintf(ascii, "%.*g", DBL_DIG+extra_float8_digits, num); PG_RETURN_CSTRING(ascii); } --- old/postgresql-7.2.1/src/backend/utils/misc/guc.c Tue Oct 30 05:38:56 2001 +++ postgresql-7.2.1/src/backend/utils/misc/guc.c Mon Nov 4 10:53:15 2002 @@ -49,6 +49,11 @@ extern int CommitSiblings; extern bool FixBTree; +/* For float4 and float8 extra digits in output format */ + +extern int extra_float4_digits; +extern int extra_float8_digits; + #ifdef ENABLE_SYSLOG extern char *Syslog_facility; extern char *Syslog_ident; @@ -502,6 +507,19 @@ "commit_siblings", PGC_USERSET, &CommitSiblings, 5, 1, 1000, NULL, NULL }, + + /* + * For float4 and float8 extra digits in output format + */ + { + "float4_extra_digits", PGC_USERSET, &extra_float4_digits, + 0, -4, 2, NULL, NULL + }, + + { + "float8_extra_digits", PGC_USERSET, &extra_float8_digits, + 0, -13, 2, NULL, NULL + }, { NULL, 0, NULL, 0, 0, 0, NULL, NULL --- old/postgresql-7.2.1/src/bin/psql/tab-complete.c Mon Nov 5 17:46:31 2001 +++ postgresql-7.2.1/src/bin/psql/tab-complete.c Mon Nov 4 11:16:01 2002 @@ -266,6 +266,9 @@ "default_transaction_isolation", + "float4_extra_digits", + "float8_extra_digits", + NULL }; --- old/postgresql-7.2.1/src/backend/utils/misc/postgresql.conf.sample Fri Jan 4 05:50:25 2002 +++ postgresql-7.2.1/src/backend/utils/misc/postgresql.conf.sample Mon Nov 4 11:02:04 2002 @@ -182,3 +182,9 @@ #password_encryption = false #sql_inheritance = true #transform_null_equals = false + +# +# floating point extra digits in output formatting +# +#extra_float4_digits = 0 #min -4, max 2 +#extra_float8_digits = 0 #min -13, max 2
"Pedro M. Ferreira" <pfrazao@ualg.pt> writes:
> int extra_float4_digits, default 0, min -4, max 2
> int extra_float8_digits, defualt 0, min -13, max 2
I think a single setting extra_float_digits would be sufficient.
> Also, if we concluded that there is a need of 2 more digits, should'nt
> this be the default ?
No. pg_dump would want to bump it up on-the-fly.
regards, tom lane
Tom Lane wrote: > "Pedro M. Ferreira" <pfrazao@ualg.pt> writes: > >>int extra_float4_digits, default 0, min -4, max 2 >>int extra_float8_digits, defualt 0, min -13, max 2 > > > I think a single setting extra_float_digits would be sufficient. Ok. Assuming, int extra_float_digits, default 0, min -13, max 2 If extra_float_digits==-13 and we are outputing a float4 this results in a negative value for FLT_DIG+extra_float_digits. I dont know if sprintf's behaviour is the same across different libraries for this situation. Should I include the following to handle this case ? if(extra_float_digits<-4) sprintf(ascii, "%.*g", FLT_DIG-4, num); else sprintf(ascii, "%.*g", FLT_DIG+extra_float_digits, num); > > >>Also, if we concluded that there is a need of 2 more digits, should'nt >>this be the default ? > > > No. pg_dump would want to bump it up on-the-fly. > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 4: Don't 'kill -9' the postmaster > > -- ---------------------------------------------------------------------- Pedro Miguel Frazao Fernandes Ferreira Universidade do Algarve Faculdade de Ciencias e Tecnologia Campus de Gambelas 8000-117 Faro Portugal Tel./Fax: (+351) 289 800950 / 289 819403 http://w3.ualg.pt/~pfrazao
"Pedro M. Ferreira" <pfrazao@ualg.pt> writes:
> Tom Lane wrote:
>> I think a single setting extra_float_digits would be sufficient.
> Ok. Assuming,
> int extra_float_digits, default 0, min -13, max 2
> If extra_float_digits==-13 and we are outputing a float4 this results in
> a negative value for FLT_DIG+extra_float_digits.
You would want to clamp the values passed to %g to not less than 1.
I'd favor code likeint ndig = FLT_DIG + extra_float_digits;if (ndig < 1) ndig = 1;sprintf(ascii, "%.*g", ndig,
num);
Probably best to do it this way with float8 too; otherwise we're
essentially wiring in the assumption that we know what DBL_DIG is.
Which is exactly what we're trying to avoid doing.
regards, tom lane
Tom Lane wrote: > "Pedro M. Ferreira" <pfrazao@ualg.pt> writes: >> >>If extra_float_digits==-13 and we are outputing a float4 this results in >>a negative value for FLT_DIG+extra_float_digits. > > You would want to clamp the values passed to %g to not less than 1. > I'd favor code like > int ndig = FLT_DIG + extra_float_digits; > if (ndig < 1) > ndig = 1; > sprintf(ascii, "%.*g", ndig, num); > > Probably best to do it this way with float8 too; otherwise we're > essentially wiring in the assumption that we know what DBL_DIG is. > Which is exactly what we're trying to avoid doing. Good. Corrected this, compiled and tested it. Works fine. I am attaching the diff's made with diff -u. Sources were from Debian source package Version 7.2.1-2woody2. Best regards, Pedro > > regards, tom lane > > -- ---------------------------------------------------------------------- Pedro Miguel Frazao Fernandes Ferreira Universidade do Algarve Faculdade de Ciencias e Tecnologia Campus de Gambelas 8000-117 Faro Portugal Tel./Fax: (+351) 289 800950 / 289 819403 http://w3.ualg.pt/~pfrazao --- old/postgresql-7.2.1/src/backend/utils/adt/float.c Tue Dec 11 02:02:12 2001 +++ postgresql-7.2.1/src/backend/utils/adt/float.c Mon Nov 4 15:50:19 2002 @@ -65,6 +65,11 @@ #include "utils/array.h" #include "utils/builtins.h" +/* + * Configuration options for float4 and float8 extra digits in output format + */ + +int extra_float_digits; #if !(NeXT && NX_CURRENT_COMPILER_RELEASE > NX_COMPILER_RELEASE_3_2) /* NS3.3 has conflicting declarations of these in <math.h> */ @@ -228,6 +233,7 @@ float4 num = PG_GETARG_FLOAT4(0); char *ascii = (char *) palloc(MAXFLOATWIDTH + 1); int infflag; + int ndig = FLT_DIG + extra_float_digits; if (isnan(num)) PG_RETURN_CSTRING(strcpy(ascii, "NaN")); @@ -237,7 +243,10 @@ if (infflag < 0) PG_RETURN_CSTRING(strcpy(ascii, "-Infinity")); - sprintf(ascii, "%.*g", FLT_DIG, num); + if (ndig < 1) + ndig = 1; + + sprintf(ascii, "%.*g", ndig, num); PG_RETURN_CSTRING(ascii); } @@ -290,6 +299,7 @@ float8 num = PG_GETARG_FLOAT8(0); char *ascii = (char *) palloc(MAXDOUBLEWIDTH + 1); int infflag; + int ndig = DBL_DIG + extra_float_digits; if (isnan(num)) PG_RETURN_CSTRING(strcpy(ascii, "NaN")); @@ -299,7 +309,10 @@ if (infflag < 0) PG_RETURN_CSTRING(strcpy(ascii, "-Infinity")); - sprintf(ascii, "%.*g", DBL_DIG, num); + if (ndig < 1) + ndig = 1; + + sprintf(ascii, "%.*g", ndig, num); PG_RETURN_CSTRING(ascii); } --- old/postgresql-7.2.1/src/backend/utils/misc/guc.c Tue Oct 30 05:38:56 2001 +++ postgresql-7.2.1/src/backend/utils/misc/guc.c Mon Nov 4 15:41:24 2002 @@ -49,6 +49,10 @@ extern int CommitSiblings; extern bool FixBTree; +/* For float4 and float8 extra digits in output format */ + +extern int extra_float_digits; + #ifdef ENABLE_SYSLOG extern char *Syslog_facility; extern char *Syslog_ident; @@ -502,6 +506,14 @@ "commit_siblings", PGC_USERSET, &CommitSiblings, 5, 1, 1000, NULL, NULL }, + + /* + * For float4 and float8 extra digits in output format + */ + { + "float_extra_digits", PGC_USERSET, &extra_float_digits, + 0, -13, 2, NULL, NULL + }, { NULL, 0, NULL, 0, 0, 0, NULL, NULL --- old/postgresql-7.2.1/src/bin/psql/tab-complete.c Mon Nov 5 17:46:31 2001 +++ postgresql-7.2.1/src/bin/psql/tab-complete.c Mon Nov 4 15:38:52 2002 @@ -266,6 +266,8 @@ "default_transaction_isolation", + "float_extra_digits", + NULL }; --- old/postgresql-7.2.1/src/backend/utils/misc/postgresql.conf.sample Fri Jan 4 05:50:25 2002 +++ postgresql-7.2.1/src/backend/utils/misc/postgresql.conf.sample Mon Nov 4 15:39:55 2002 @@ -182,3 +182,8 @@ #password_encryption = false #sql_inheritance = true #transform_null_equals = false + +# +# floating point extra digits in output formatting +# +#extra_float_digits = 0 #min -13, max 2
"Pedro M. Ferreira" <pfrazao@ualg.pt> writes:
> I am attaching the diff's made with diff -u. Sources were from Debian
> source package Version 7.2.1-2woody2.
Looks good, will keep to apply after we branch for 7.4 development.
BTW, did you check to see if this affects the geometric types or not?
I am not sure that they go through float8out; they may need similar
adjustments in their output routines.
regards, tom lane
Tom Lane wrote: > Looks good, will keep to apply after we branch for 7.4 development. > > BTW, did you check to see if this affects the geometric types or not? > I am not sure that they go through float8out; they may need similar > adjustments in their output routines. Only checked arrays. I will check this and get you posted about it ASAP. Best regards, Pedro > > regards, tom lane > > -- ---------------------------------------------------------------------- Pedro Miguel Frazao Fernandes Ferreira Universidade do Algarve Faculdade de Ciencias e Tecnologia Campus de Gambelas 8000-117 Faro Portugal Tel./Fax: (+351) 289 800950 / 289 819403 http://w3.ualg.pt/~pfrazao
Tom Lane wrote: > BTW, did you check to see if this affects the geometric types or not? > I am not sure that they go through float8out; they may need similar > adjustments in their output routines. In fact they need adjustments. The *_out routines (in src/backend/utils/adt/geo_ops.c) for the geometric types rely on two functions to output data: static int pair_encode(float8 x, float8 y, char *str); static int single_encode(float8 x, char *str); These functions produce output with (for pair_encode): sprintf(str, "%.*g,%.*g", digits8, x, digits8, y); digits8 is defined as , #define P_MAXDIG DBL_DIG static int digits8 = P_MAXDIG; I think it would be done the same way as for float4_out and float8_out: extern int extra_float_digits; int ndig = digits8 + extra_float_digits; if (ndig < 1)ndig = 1; sprintf(str, "%.*g,%.*g", ndig, x, ndig, y); There a bunch of other places where output is produced. They are all within #ifdef GEODEBUG / #enfif blocks. Should these be corrected the same way ? Regards, Pedro
"Pedro M. Ferreira" <pfrazao@ualg.pt> writes:
> These functions produce output with (for pair_encode):
> sprintf(str, "%.*g,%.*g", digits8, x, digits8, y);
> digits8 is defined as ,
> #define P_MAXDIG DBL_DIG
> static int digits8 = P_MAXDIG;
> I think it would be done the same way as for float4_out and float8_out:
Yeah. In fact I'd be inclined to remove the static variable and make
the code match float8out exactly (do "DBL_DIG + extra_float_digits").
> There a bunch of other places where output is produced. They are all
> within #ifdef GEODEBUG / #enfif blocks. Should these be corrected the
> same way ?
Up to you. Personally I'd just leave them alone...
regards, tom lane
Tom Lane wrote: > > Yeah. In fact I'd be inclined to remove the static variable and make > the code match float8out exactly (do "DBL_DIG + extra_float_digits"). P_MAXDIG is only used in the lines below: #define P_MAXDIG DBL_DIG #define P_MAXLEN (2*(P_MAXDIG+7)+1) static int digits8 = P_MAXDIG; Is it ok to remove #define P_MAXDIG DBL_DIG, change P_MAXLEN to 2*(DBL_DIG+7)+1) and remove the line 'static int digits8 = P_MAXDIG;' ? Would then change the two geo output functions and replace digits8 by DBL_DIG in the #ifdef GEODEBUG / #enfif output stuff. >>There a bunch of other places where output is produced. They are all >>within #ifdef GEODEBUG / #enfif blocks. Should these be corrected the >>same way ?
"Pedro M. Ferreira" <pfrazao@ualg.pt> writes:
> Is it ok to remove #define P_MAXDIG DBL_DIG,
> change P_MAXLEN to 2*(DBL_DIG+7)+1) and
> remove the line 'static int digits8 = P_MAXDIG;' ?
Perhaps P_MAXLEN now needs to be (2*(DBL_DIG+2+7)+1), considering
that we'll allow extra_float_digits to be up to 2. What's it used for?
regards, tom lane
Tom Lane wrote: > "Pedro M. Ferreira" <pfrazao@ualg.pt> writes: > >>Is it ok to remove #define P_MAXDIG DBL_DIG, >>change P_MAXLEN to 2*(DBL_DIG+7)+1) and >>remove the line 'static int digits8 = P_MAXDIG;' ? > > Perhaps P_MAXLEN now needs to be (2*(DBL_DIG+2+7)+1), considering > that we'll allow extra_float_digits to be up to 2. What's it used for? Yes. I guess so, because it is used in what I think is a memory allocation function. P_MAXLEN is only used twice: * 1st use in path_encode() (allmost all the geo *_out functions use path_encode): int size = npts * (P_MAXLEN + 3) + 2; /* Check for integer overflow */ if ((size - 2) / npts != (P_MAXLEN + 3)) elog(ERROR, "Too many points requested"); * 2nd use in circle_out(PG_FUNCTION_ARGS): result = palloc(3 * (P_MAXLEN + 1) + 3); I will do the changes tomorrow and send in the appropriate diff's. Regards, Pedro
> > Perhaps P_MAXLEN now needs to be (2*(DBL_DIG+2+7)+1), considering > > that we'll allow extra_float_digits to be up to 2. What's it used for? > > Yes. I guess so, because it is used in what I think is a memory > allocation function. P_MAXLEN is only used twice: Curse my slowness, but what's the actual problem being fixed here? Chris
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:
> Curse my slowness, but what's the actual problem being fixed here?
Two things:
* allow pg_dump to accurately dump and restore float quantities (setting
float_extra_digits to 2 during the dump will accomplish this, at least
on systems with reasonable float I/O routines).
* allow us to get out from under the geometry regression test's platform
dependency problems (setting float_extra_digits to -2 or so during the
test should make most or all of the variations go away).
This proposal is the first one I've seen that solves both these problems
without introducing any compatibility issues of its own.
regards, tom lane
Pedro M. Ferreira wrote: > Tom Lane wrote: >> Perhaps P_MAXLEN now needs to be (2*(DBL_DIG+2+7)+1), considering >> that we'll allow extra_float_digits to be up to 2. What's it used for? > > Yes. I guess so, because it is used in what I think is a memory > allocation function. P_MAXLEN is only used twice: <...> > > I will do the changes tomorrow and send in the appropriate diff's. Ok. Its done now. Only one file changed: src/backend/utils/adt/geo_ops.c All the geometric types should now account for float_extra_digits on output. A diff -u is attached. Best reagards, Pedro > Regards, > Pedro > > > ---------------------------(end of broadcast)--------------------------- > TIP 2: you can get off all lists at once with the unregister command > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) > > -- ---------------------------------------------------------------------- Pedro Miguel Frazao Fernandes Ferreira Universidade do Algarve Faculdade de Ciencias e Tecnologia Campus de Gambelas 8000-117 Faro Portugal Tel./Fax: (+351) 289 800950 / 289 819403 http://w3.ualg.pt/~pfrazao --- old/postgresql-7.2.1/src/backend/utils/adt/geo_ops.c Mon Nov 4 12:01:39 2002 +++ postgresql-7.2.1/src/backend/utils/adt/geo_ops.c Tue Nov 5 10:47:56 2002 @@ -80,11 +80,11 @@ #define RDELIM_C '>' /* Maximum number of output digits printed */ -#define P_MAXDIG DBL_DIG -#define P_MAXLEN (2*(P_MAXDIG+7)+1) - -static int digits8 = P_MAXDIG; +/* ...+2+7 : 2 accounts for extra_float_digits max value */ +#define P_MAXLEN (2*(DBL_DIG+2+7)+1) +/* Extra digits in float output formatting (in float.c) */ +extern int extra_float_digits; /* * Geometric data types are composed of points. @@ -139,7 +139,12 @@ static int single_encode(float8 x, char *str) { - sprintf(str, "%.*g", digits8, x); + int ndig = DBL_DIG + extra_float_digits; + + if (ndig < 1) + ndig=1; + + sprintf(str, "%.*g", ndig, x); return TRUE; } /* single_encode() */ @@ -190,7 +195,12 @@ static int pair_encode(float8 x, float8 y, char *str) { - sprintf(str, "%.*g,%.*g", digits8, x, digits8, y); + int ndig = DBL_DIG + extra_float_digits; + + if (ndig < 1) + ndig=1; + + sprintf(str, "%.*g,%.*g", ndig, x, ndig, y); return TRUE; } @@ -974,7 +984,7 @@ #endif #ifdef GEODEBUG printf("line_construct_pts- line is neither vertical nor horizontal (diffs x=%.*g, y=%.*g\n", - digits8, (pt2->x - pt1->x), digits8, (pt2->y - pt1->y)); + DBL_DIG, (pt2->x - pt1->x), DBL_DIG, (pt2->y - pt1->y)); #endif } } @@ -1180,8 +1190,8 @@ #ifdef GEODEBUG printf("line_interpt- lines are A=%.*g, B=%.*g, C=%.*g, A=%.*g, B=%.*g, C=%.*g\n", - digits8, l1->A, digits8, l1->B, digits8, l1->C, digits8, l2->A, digits8, l2->B, digits8, l2->C); - printf("line_interpt- lines intersect at (%.*g,%.*g)\n", digits8, x, digits8, y); + DBL_DIG, l1->A, DBL_DIG, l1->B, DBL_DIG, l1->C, DBL_DIG, l2->A, DBL_DIG, l2->B, DBL_DIG, l2->C); + printf("line_interpt- lines intersect at (%.*g,%.*g)\n", DBL_DIG, x, DBL_DIG, y); #endif return result; @@ -2390,14 +2400,14 @@ p = line_interpt_internal(&tmp, line); #ifdef GEODEBUG printf("interpt_sl- segment is (%.*g %.*g) (%.*g %.*g)\n", - digits8, lseg->p[0].x, digits8, lseg->p[0].y, digits8, lseg->p[1].x, digits8, lseg->p[1].y); + DBL_DIG, lseg->p[0].x, DBL_DIG, lseg->p[0].y, DBL_DIG, lseg->p[1].x, DBL_DIG, lseg->p[1].y); printf("interpt_sl- segment becomes line A=%.*g B=%.*g C=%.*g\n", - digits8, tmp.A, digits8, tmp.B, digits8, tmp.C); + DBL_DIG, tmp.A, DBL_DIG, tmp.B, DBL_DIG, tmp.C); #endif if (PointerIsValid(p)) { #ifdef GEODEBUG - printf("interpt_sl- intersection point is (%.*g %.*g)\n", digits8, p->x, digits8, p->y); + printf("interpt_sl- intersection point is (%.*g %.*g)\n", DBL_DIG, p->x, DBL_DIG, p->y); #endif if (on_ps_internal(p, lseg)) {
I am confused about this patch. I don't see extra_float_digits defined anywhere. Am I missing a patch? --------------------------------------------------------------------------- Pedro M. Ferreira wrote: > Pedro M. Ferreira wrote: > > Tom Lane wrote: > >> Perhaps P_MAXLEN now needs to be (2*(DBL_DIG+2+7)+1), considering > >> that we'll allow extra_float_digits to be up to 2. What's it used for? > > > > Yes. I guess so, because it is used in what I think is a memory > > allocation function. P_MAXLEN is only used twice: > <...> > > > > I will do the changes tomorrow and send in the appropriate diff's. > > Ok. Its done now. > Only one file changed: src/backend/utils/adt/geo_ops.c > > All the geometric types should now account for float_extra_digits on output. > > A diff -u is attached. > > Best reagards, > Pedro > > > Regards, > > Pedro > > > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 2: you can get off all lists at once with the unregister command > > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) > > > > > > > -- > ---------------------------------------------------------------------- > Pedro Miguel Frazao Fernandes Ferreira > Universidade do Algarve > Faculdade de Ciencias e Tecnologia > Campus de Gambelas > 8000-117 Faro > Portugal > Tel./Fax: (+351) 289 800950 / 289 819403 > http://w3.ualg.pt/~pfrazao > --- old/postgresql-7.2.1/src/backend/utils/adt/geo_ops.c Mon Nov 4 12:01:39 2002 > +++ postgresql-7.2.1/src/backend/utils/adt/geo_ops.c Tue Nov 5 10:47:56 2002 > @@ -80,11 +80,11 @@ > #define RDELIM_C '>' > > /* Maximum number of output digits printed */ > -#define P_MAXDIG DBL_DIG > -#define P_MAXLEN (2*(P_MAXDIG+7)+1) > - > -static int digits8 = P_MAXDIG; > +/* ...+2+7 : 2 accounts for extra_float_digits max value */ > +#define P_MAXLEN (2*(DBL_DIG+2+7)+1) > > +/* Extra digits in float output formatting (in float.c) */ > +extern int extra_float_digits; > > /* > * Geometric data types are composed of points. > @@ -139,7 +139,12 @@ > static int > single_encode(float8 x, char *str) > { > - sprintf(str, "%.*g", digits8, x); > + int ndig = DBL_DIG + extra_float_digits; > + > + if (ndig < 1) > + ndig=1; > + > + sprintf(str, "%.*g", ndig, x); > return TRUE; > } /* single_encode() */ > > @@ -190,7 +195,12 @@ > static int > pair_encode(float8 x, float8 y, char *str) > { > - sprintf(str, "%.*g,%.*g", digits8, x, digits8, y); > + int ndig = DBL_DIG + extra_float_digits; > + > + if (ndig < 1) > + ndig=1; > + > + sprintf(str, "%.*g,%.*g", ndig, x, ndig, y); > return TRUE; > } > > @@ -974,7 +984,7 @@ > #endif > #ifdef GEODEBUG > printf("line_construct_pts- line is neither vertical nor horizontal (diffs x=%.*g, y=%.*g\n", > - digits8, (pt2->x - pt1->x), digits8, (pt2->y - pt1->y)); > + DBL_DIG, (pt2->x - pt1->x), DBL_DIG, (pt2->y - pt1->y)); > #endif > } > } > @@ -1180,8 +1190,8 @@ > > #ifdef GEODEBUG > printf("line_interpt- lines are A=%.*g, B=%.*g, C=%.*g, A=%.*g, B=%.*g, C=%.*g\n", > - digits8, l1->A, digits8, l1->B, digits8, l1->C, digits8, l2->A, digits8, l2->B, digits8, l2->C); > - printf("line_interpt- lines intersect at (%.*g,%.*g)\n", digits8, x, digits8, y); > + DBL_DIG, l1->A, DBL_DIG, l1->B, DBL_DIG, l1->C, DBL_DIG, l2->A, DBL_DIG, l2->B, DBL_DIG, l2->C); > + printf("line_interpt- lines intersect at (%.*g,%.*g)\n", DBL_DIG, x, DBL_DIG, y); > #endif > > return result; > @@ -2390,14 +2400,14 @@ > p = line_interpt_internal(&tmp, line); > #ifdef GEODEBUG > printf("interpt_sl- segment is (%.*g %.*g) (%.*g %.*g)\n", > - digits8, lseg->p[0].x, digits8, lseg->p[0].y, digits8, lseg->p[1].x, digits8, lseg->p[1].y); > + DBL_DIG, lseg->p[0].x, DBL_DIG, lseg->p[0].y, DBL_DIG, lseg->p[1].x, DBL_DIG, lseg->p[1].y); > printf("interpt_sl- segment becomes line A=%.*g B=%.*g C=%.*g\n", > - digits8, tmp.A, digits8, tmp.B, digits8, tmp.C); > + DBL_DIG, tmp.A, DBL_DIG, tmp.B, DBL_DIG, tmp.C); > #endif > if (PointerIsValid(p)) > { > #ifdef GEODEBUG > - printf("interpt_sl- intersection point is (%.*g %.*g)\n", digits8, p->x, digits8, p->y); > + printf("interpt_sl- intersection point is (%.*g %.*g)\n", DBL_DIG, p->x, DBL_DIG, p->y); > #endif > if (on_ps_internal(p, lseg)) > { > > ---------------------------(end of broadcast)--------------------------- > TIP 6: Have you searched our list archives? > > http://archives.postgresql.org -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> I am confused about this patch. I don't see extra_float_digits defined
> anywhere. Am I missing a patch?
Evidently. I have the patch and was planning to apply it myself as soon
as Pedro does something with the geometry types...
regards, tom lane
HI All, Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > >>I am confused about this patch. I don't see extra_float_digits defined >>anywhere. Am I missing a patch ? Probably! :) The definition of extra_float_digits was made in float.c and the diff was sent to the list and Tom before. I think its the one Tom refers below. There were diff's to four files: src/backend/utils/adt/float.c src/backend/utils/misc/guc.c src/bin/psql/tab-complete.c src/backend/utils/misc/postgresql.conf.sample > > > Evidently. I have the patch and was planning to apply it myself as soon > as Pedro does something with the geometry types... Lots of mail in the list! ;) Its done. The diff file to src/backend/utils/adt/geo_ops.c (to handle the gemetry types) was sent some days ago. Its the one in the message where Bruce says that extra_float_digits is not defined anywhere. If you want I can send in the diff's again. Best regards, Pedro > > regards, tom lane -- ---------------------------------------------------------------------- Pedro Miguel Frazao Fernandes Ferreira Universidade do Algarve Faculdade de Ciencias e Tecnologia Campus de Gambelas 8000-117 Faro Portugal Tel./Fax: (+351) 289 800950 / 289 819403 http://w3.ualg.pt/~pfrazao
"Pedro M. Frazao F. Ferreira" <pfrazao@ualg.pt> writes:
> Tom Lane wrote:
>> Evidently. I have the patch and was planning to apply it myself as soon
>> as Pedro does something with the geometry types...
> Its done.
So it is, dunno what I was thinking. Will work on getting it applied.
regards, tom lane
"Pedro M. Ferreira" <pfrazao@ualg.pt> writes:
> [ patch for extra_float_digits ]
I've applied this patch along with followup changes to pg_dump (it sets
extra_float_digits to 2 to allow accurate dump/reload) and the geometry
regression test (it sets extra_float_digits to -3).
I find that two geometry 'expected' files are now sufficient to cover
all the platforms I have available to test. (We'd only need one, if
everyone displayed minus zero as '-0', but some platforms print '0'.)
I tested on HPUX 10.20 (HPPA), Red Hat Linux 8.0 (Intel), Mac OS X 10.2.1
and LinuxPPC (PPC).
I'd be interested to hear results of testing CVS tip (now 7.4devel)
on other platforms. Does geometry pass cleanly for you?
regards, tom lane
> I'd be interested to hear results of testing CVS tip (now 7.4devel) > on other platforms. Does geometry pass cleanly for you? Yes for NetBSD-1.5.1/i386, where it previously didn't due to processor specific math libraries on this platform. Giles
Re: Geometry regression tests (was Re: Float output formatting
From
"Pedro M. Frazao F. Ferreira"
Date:
Tom Lane wrote: > "Pedro M. Ferreira" <pfrazao@ualg.pt> writes: > >>[ patch for extra_float_digits ] > > > I've applied this patch along with followup changes to pg_dump (it sets > extra_float_digits to 2 to allow accurate dump/reload) and the geometry > regression test (it sets extra_float_digits to -3). > > I find that two geometry 'expected' files are now sufficient to cover > all the platforms I have available to test. (We'd only need one, if > everyone displayed minus zero as '-0', but some platforms print '0'.) > I tested on HPUX 10.20 (HPPA), Red Hat Linux 8.0 (Intel), Mac OS X 10.2.1 > and LinuxPPC (PPC). > > I'd be interested to hear results of testing CVS tip (now 7.4devel) > on other platforms. Does geometry pass cleanly for you? Yes! :) All tests passed on a dual AMD Athlon MP with Debian GNU/Linux 3.0 (Woody), kernel 2.4.18-5. Tested with a snapshot downloaded yesterday. Best regards, Pedro M. Ferreira > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 4: Don't 'kill -9' the postmaster
Tom Lane writes: > I find that two geometry 'expected' files are now sufficient to cover > all the platforms I have available to test. (We'd only need one, if > everyone displayed minus zero as '-0', but some platforms print '0'.) Judging from the platforms affected by this, I would suspect that this is a (mis-)feature of the snprintf() implementation rather than compiler or processor. Would it make sense to provide a fixed version of snprintf() and get rid of these differences? -- Peter Eisentraut peter_e@gmx.net
Peter Eisentraut <peter_e@gmx.net> writes:
> Tom Lane writes:
>> I find that two geometry 'expected' files are now sufficient to cover
>> all the platforms I have available to test. (We'd only need one, if
>> everyone displayed minus zero as '-0', but some platforms print '0'.)
> Judging from the platforms affected by this, I would suspect that this is
> a (mis-)feature of the snprintf() implementation rather than compiler or
> processor. Would it make sense to provide a fixed version of snprintf()
> and get rid of these differences?
Certainly it's a library issue on most of these platforms --- AFAIK,
all these machines have IEEE-compliant float hardware, so it must be
sprintf's fault and not a matter of not getting the minus zero in the
first place.
I wouldn't want to write a float converter from scratch, but maybe we
could add a few lines in src/port/snprintf.c to patch up a wrong result?
regards, tom lane