Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sphere output precision limited to 15 significant digits #118

Closed
toniorte opened this issue Jan 3, 2024 · 10 comments
Closed

Sphere output precision limited to 15 significant digits #118

toniorte opened this issue Jan 3, 2024 · 10 comments
Assignees

Comments

@toniorte
Copy link

toniorte commented Jan 3, 2024

Dear All,

I was wondering if there is any reason to keep the maximum precision of the printed outputs to DBL_DIG macro (15) instead of DBL_DECIMAL_DIG (17), being that just 15 digits don't allow round-trip conversion.

In my case it causes me trouble because I cannot restore dumps of databases done with pg_dump, since the dumps contain spolys which lost precision during the dump, and pg_sphere does not pass the check of certain polygons which were inserted with 17 digits (from caom2db software) but now have 15.

Regards.

@esabol
Copy link
Contributor

esabol commented Jan 3, 2024

[...] 15 digits don't allow round-trip conversion.

Oh, that's a good point.

@toniorte
Copy link
Author

toniorte commented Jan 10, 2024

Dear All,

I have tried to create a binary backup using the pg_dumpbinary software, hoping that it would keep the original data with the needed precision, but the following error appears:

ERROR: no binary output function available for type public.spoly

Thus, going back to the quick solution for the text output, I guess that in fact the method set_sphere_output_precision(n) (which is limited to n<=DBL_DIG, i.e. 15) tries to imitate the PostgreSQL behavior of "SET extra_float_digits".
Of course and by default, the PostgreSQL "extra_float_digits" its set to return 17 digits (since PG12 as @df7cb pointed out). And in fact, PostgreSQL documentation says that the maximum value is "useful for dumping float data that needs to be restored exactly."

Regards.

@df7cb
Copy link
Contributor

df7cb commented Jan 10, 2024

The value changed from 15 to 17 in PG12.

@esabol
Copy link
Contributor

esabol commented Jan 16, 2024

@df7cb @vitcpp : So should it be changed to 17 on PG 12+?

@vitcpp
Copy link
Contributor

vitcpp commented Jan 16, 2024

Dear All,

Obviously, there should be a way to set 17 digits for output. But I'm not sure about DBL_DECIMAL_DIG. It seems to appear in C11. May be to utilize extra_float_digits? Let me, please, dig into this topic. I will come with some proposal later.

@df7cb
Copy link
Contributor

df7cb commented Jan 16, 2024

Some other datatypes (like my postgresql-unit) use extra_float_digits for their custom float rendering on output. pgsphere should do the same.

Perhaps it is just a matter of using float8out_internal for printing the numbers, so something along that line.

@vitcpp
Copy link
Contributor

vitcpp commented Jan 19, 2024

I agree, we should unify this behaviour with postgresql. What I'm concerned is the backward compatibility.

I propose:

  • Keep function set_sphere_output_precision for some time. Probably, issue some warning that this function is obsolete.
  • By default, use postgresql settings (extra_float_digits).
  • If set_sphere_output_precision is called, use the specified precision and the old behavuour.

If user decides to use postgresql extra_float_digits, set_sphere_output_precision shouldn't be called or called with -1, for example. It shouldn't affect old scripts with set_sphere_output_precision.

What do you think?

P.S. set_sphere_output_precision seems to be absent in the doc. Some clarification should be written in the doc.

@esabol
Copy link
Contributor

esabol commented Jan 19, 2024

That all makes sense to me, @vitcpp. Thanks!

@vitcpp vitcpp self-assigned this Mar 10, 2024
vitcpp added a commit to vitcpp/pgsphere that referenced this issue Mar 14, 2024
pgSphere used its own setting (set_sphere_output_precision function)
to limit the output precision of double values, that could not be
greater than 15 significant digits (DBL_DIG). It introduced some
problems with dump/restore. PostgreSQL uses its own precision setting:
extra_float_digits. The PostgreSQL setting allows to use more significant
digits.

This patch changes the default pgSphere output behaviour to utilize
PostgreSQL extra_float_digits. Now, extra_float_digits is used by default,
until set_sphere_output_precision is called.

The old behaviour is kept for compatibility purposes. Once,
set_sphere_output_precision is called, pgSphere starts to use the old
behaviour (read, please, issue postgrespro#118 discussion).

The patch introduces a new function - reset_sphere_output_precision.
It is used to reset to the PostgreSQL behaviour after using
set_sphere_output_precision.
vitcpp added a commit to vitcpp/pgsphere that referenced this issue Mar 14, 2024
pgSphere used its own setting (set_sphere_output_precision function)
to limit the output precision of double values, that could not be
greater than 15 significant digits (DBL_DIG). It introduced some
problems with dump/restore. PostgreSQL uses its own precision setting:
extra_float_digits. The PostgreSQL setting allows to use more significant
digits.

This patch changes the default pgSphere output behaviour to utilize
PostgreSQL extra_float_digits. Now, extra_float_digits is used by default,
until set_sphere_output_precision is called.

The old behaviour is kept for compatibility purposes. Once,
set_sphere_output_precision is called, pgSphere starts to use the old
behaviour (read, please, issue postgrespro#118 discussion).

The patch introduces a new function - reset_sphere_output_precision.
It is used to reset to the PostgreSQL behaviour after using
set_sphere_output_precision.
vitcpp added a commit that referenced this issue Mar 26, 2024
Fix output precision limit for double values (issue #118)

pgSphere used its own setting (set_sphere_output_precision function)
to limit the output precision of double values, that could not be
greater than 15 significant digits (DBL_DIG). It introduced some
problems with dump/restore. PostgreSQL uses its own precision setting:
extra_float_digits. The PostgreSQL setting allows to use more significant
digits.

This patch changes the default pgSphere output behaviour to utilize
PostgreSQL extra_float_digits. Now, extra_float_digits is used by default,
until set_sphere_output_precision is called.

The old behaviour is kept for compatibility purposes. Once,
set_sphere_output_precision is called, pgSphere starts to use the old
behaviour (read, please, issue #118 discussion).

The patch introduces a new function - reset_sphere_output_precision.
It is used to reset to the PostgreSQL behaviour after using
set_sphere_output_precision.

* Update upgrade script (reset_sphere_output_precision function)

* Add test for pgSphere output precision with different settings

expected/output_precision.out   - PG 10-11
expected/output_precision_1.out - PG 12+

* Add extra_float_digits = 2 for epochprop and bounding_box_gist tests
@vitcpp
Copy link
Contributor

vitcpp commented Mar 26, 2024

The PR was merged. I will close the issue if no objections.

@vitcpp
Copy link
Contributor

vitcpp commented May 1, 2024

Closed as completed.

@vitcpp vitcpp closed this as completed May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants