guc

Liam Stewart writes:

I've removed
GetConfigOption() and replaced it with ShowConfigOption() which does an
elog(NOTICE) instead of returning a string.

I certainly don't like that. I want to be able to get at the
configuration setting without any notice going off.

When setting REAL_FORMAT and DOUBLE_PRECISION_FORMAT, the printf-style
format string should be checked for certain things:

I wouldn't use a printf format string at all. It's not user-friendly --
not everyone is a C programmer, in fact most people aren't. It leaves
open too many ways to shoot yourself in the foot. And if you plan to
close all those ways you end up with a crippled system that is still
complex to understand for many.

What I would like to get out of the configurability of floating-point
numbers is:

1. The ability to dump them in binary or hex format for lossless
dump/reload. (printf("%a") does that.) That could be a boolean
setting.

2. To have (at least as an option) the same output format on all platforms.
Not sure how to approach that, perhaps it has nothing to do with
configurability.

Some people will also suggest

3. Be able to set the number of significant digits that are shown.

to allow simplifying the regression tests, but I do not think that that is
a good idea, because

a. If platforms behave differently, the test suite should not paint over
that; it might be important information.

b. If we actually break floating-point operations one day, we might miss
it.

c. Types represent data, data is altered by functions (round? truncate?).
Global side-effects are evil.

d. Input and output would not be inverses anymore.

--
Peter Eisentraut peter_e@gmx.net

Peter Eisentraut <peter_e@gmx.net> writes:

Liam Stewart writes:

I've removed
GetConfigOption() and replaced it with ShowConfigOption() which does an
elog(NOTICE) instead of returning a string.

I certainly don't like that. I want to be able to get at the
configuration setting without any notice going off.

I agree with Peter --- better to separate the getting of the string
from displaying it. What's the reason for collapsing them together?

When setting REAL_FORMAT and DOUBLE_PRECISION_FORMAT, the printf-style
format string should be checked for certain things:

I wouldn't use a printf format string at all.

Good point. If we have to set up a checking mechanism then we should
ask ourselves why we're bothering to use printf representation.

What I would like to get out of the configurability of floating-point
numbers is:
1. The ability to dump them in binary or hex format for lossless
dump/reload. (printf("%a") does that.)

On some platforms... I'd be happier with this if it were more portable...

Some people will also suggest
3. Be able to set the number of significant digits that are shown.

to allow simplifying the regression tests, but I do not think that that is
a good idea,

[ raises eyebrow ] To my mind that was one of the principal reasons
for working on such a thing at all. If you don't want to allow this,
then what alternative solution do you have for our geometry regression
test mess? (Defining it as not a mess won't fly.)

regards, tom lane

Some people will also suggest
3. Be able to set the number of significant digits that are shown.

to allow simplifying the regression tests, but I do not think that that is
a good idea,

[ raises eyebrow ] To my mind that was one of the principal reasons
for working on such a thing at all. If you don't want to allow this,
then what alternative solution do you have for our geometry regression
test mess? (Defining it as not a mess won't fly.)

Yes, I doubt the current regression are tests are input/output
reversable because of the rounding anyway.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Tom Lane writes:

[ raises eyebrow ] To my mind that was one of the principal reasons
for working on such a thing at all. If you don't want to allow this,
then what alternative solution do you have for our geometry regression
test mess? (Defining it as not a mess won't fly.)

I did some investigation and noticed that the different geometry expected
files differ in the same 5 groups of places (or less), and in 4 of those
places there are only two possible variations. So that at least gives me
the idea that perhaps the geometry test could be split up in two or three
tests, and that would probably reduce the relative number of "nonstandard"
expected files we'd need.

One difference case is a system failing to implement negative zeros.
Setting the output precision won't fix that.

Actually, I just took a peak into include/utils/geo_decls.h, and the
definiton of EPSILON at the top leaves me to think that printing out
anything more than 6 significant digits is nonsense anyway. Numerical
analysts would probably shudder.

--
Peter Eisentraut peter_e@gmx.net

Peter Eisentraut <peter_e@gmx.net> writes:

Actually, I just took a peak into include/utils/geo_decls.h, and the
definiton of EPSILON at the top leaves me to think that printing out
anything more than 6 significant digits is nonsense anyway. Numerical
analysts would probably shudder.

Indeed. Does that mean you'd be happy with restricting the number of
digits printed for geometrical types only?

regards, tom lane

Tom Lane writes:

Indeed. Does that mean you'd be happy with restricting the number of
digits printed for geometrical types only?

Not really. I'd much rather see the EPSILON removed/revised. I don't
claim to understand numerical analysis, but that thing is completely
bogus. I can see how the error would be controllable when you just add
numbers, but once you start multiplying or run trigonometric functions, a
fixed epsilon just doesn't cut it.

If you want to limit the number of digits, why not just reimplement the
geometric types as single precision?

And if we think that an epsilon-based float comparison is important, why
don't we do it everywhere?

--
Peter Eisentraut peter_e@gmx.net

Peter Eisentraut <peter_e@gmx.net> writes:

Not really. I'd much rather see the EPSILON removed/revised. I don't
claim to understand numerical analysis, but that thing is completely
bogus.

Yeah, and there's even the handy comment:

* XXX These routines were not written by a numerical analyst.

to remind you that this stuff was written by someone who was studying
databases not numerical analysis.

In my eyes, all of our geometric datatypes are firmly in the "academic
toy prototype" category. They could use a thorough overhaul, but in
view of the existence of the PostGIS project I doubt they'll ever get
one. Anyone who might have both the ability and the motivation to
improve these datatypes will probably go use/work on PostGIS instead.

I could make an argument that we should just yank these types from the
distribution and leave the field clear for PostGIS. I don't really want
to take that line; the types do have usefulness for simple applications,
and what's probably more important is they help keep us honest on
datatype extensibility concerns. But I have a hard time justifying
spending any core development time on them.

Basically what I want is some fairly simple answer that will let us stop
wasting quite so much maintenance effort on the geometry regression
test. Because, frankly, that code is nowhere near good enough to
justify our expending much time on it.

In that context, trimming the number of displayed decimal places seems
like a great solution. Whether it's the "right thing" from a purist's
viewpoint doesn't concern me a whole lot.

regards, tom lane

On Thu, Jan 17, 2002 at 05:57:29PM -0500, Tom Lane wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

Liam Stewart writes:

I've removed
GetConfigOption() and replaced it with ShowConfigOption() which does an
elog(NOTICE) instead of returning a string.

I certainly don't like that. I want to be able to get at the
configuration setting without any notice going off.

I agree with Peter --- better to separate the getting of the string
from displaying it. What's the reason for collapsing them together?

I ended up thinking that GetConfigOption was a bit useless right now
since, with the redone GetPGVariable, there's currently nothing that
would use it. I can see some use for it since not all guc options have
an associated variable but still have a value (e.g.: seed,
server_encoding). How about I add GetConfigOption back in and change
ShowConfigOption to use it. display_proc hooks would return a char *;
GetConfigOption would use a variable's display_proc hook if it is
non-null instead of doing its own thing. ShowConfigOption would no
longer call a variable's display_proc hook.

I wouldn't use a printf format string at all.

Good point. If we have to set up a checking mechanism then we should
ask ourselves why we're bothering to use printf representation.

I was going on the fact that printf format string was on the todo and
that Tom suggested using them last February. When thinking about parsing
printf strings, I realized how not nice it would be, so I'm alright
with a new representation.

What I would like to get out of the configurability of floating-point
numbers is:
1. The ability to dump them in binary or hex format for lossless
dump/reload. (printf("%a") does that.)

On some platforms... I'd be happier with this if it were more portable...

Roll our own? %a and %A are C99 so are much less portable than most
other printf conversion specifiers.

Liam

--
Liam Stewart :: Red Hat Canada, Ltd. :: liams@redhat.com

Liam Stewart <liams@redhat.com> writes:

What I would like to get out of the configurability of floating-point
numbers is:
1. The ability to dump them in binary or hex format for lossless
dump/reload. (printf("%a") does that.)

On some platforms... I'd be happier with this if it were more portable...

Roll our own? %a and %A are C99 so are much less portable than most
other printf conversion specifiers.

Hmm ... rolling our own might actually not be too unreasonable ...
and the lack of lossless dump/reload for floats has certainly been
a sore point for a long time, so expending some effort here is
justified.

regards, tom lane

Thread has been saved for the 7.3 release:

http://candle.pha.pa.us/cgi-bin/pgpatches2

---------------------------------------------------------------------------

Liam Stewart wrote:

I was playing around with adding a couple new SET commands (REAL_FORMAT
and DOUBLE_PRECISION_FORMAT as per todo list item) and I ended up
playing with GUC a bit.

I've cleaned it up a bit - most of the changes should be fairly
self-explanitory from the patch. Some of the configuration options
handled in variable.c (so far, server_encoding and seed) have been moved
into guc.c and are handled by GUC. Moving them in required adding a
couple of fields to config_generic and friends - a read_only field and a
display_proc field (hook for custom SHOW messages). I've removed
GetConfigOption() and replaced it with ShowConfigOption() which does an
elog(NOTICE) instead of returning a string.

When setting REAL_FORMAT and DOUBLE_PRECISION_FORMAT, the printf-style
format string should be checked for certain things:

- field width of "*" is not allowed
- precision of ".*" is not allowed
- all conversion specifiers except "s" and "n" are allowed
- only one conversion specification other than %% and variations is
allowed

Does this seem reasonable to people? I haven't written a function to
check those yet.. will do soon.

This patch isn't complete yet so don't append it to the pending patches
list, Bruce.

Liam

--
Liam Stewart :: Red Hat Canada, Ltd. :: liams@redhat.com

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

This patch was rejected. Please continue discussion on the hackers
list.

---------------------------------------------------------------------------

Liam Stewart wrote:

I was playing around with adding a couple new SET commands (REAL_FORMAT
and DOUBLE_PRECISION_FORMAT as per todo list item) and I ended up
playing with GUC a bit.

I've cleaned it up a bit - most of the changes should be fairly
self-explanitory from the patch. Some of the configuration options
handled in variable.c (so far, server_encoding and seed) have been moved
into guc.c and are handled by GUC. Moving them in required adding a
couple of fields to config_generic and friends - a read_only field and a
display_proc field (hook for custom SHOW messages). I've removed
GetConfigOption() and replaced it with ShowConfigOption() which does an
elog(NOTICE) instead of returning a string.

When setting REAL_FORMAT and DOUBLE_PRECISION_FORMAT, the printf-style
format string should be checked for certain things:

- field width of "*" is not allowed
- precision of ".*" is not allowed
- all conversion specifiers except "s" and "n" are allowed
- only one conversion specification other than %% and variations is
allowed

Does this seem reasonable to people? I haven't written a function to
check those yet.. will do soon.

This patch isn't complete yet so don't append it to the pending patches
list, Bruce.

Liam

--
Liam Stewart :: Red Hat Canada, Ltd. :: liams@redhat.com

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026