Re: Ryu floating point output patch

On 2/14/19 10:14 AM, Andrew Dunstan wrote:
> On 2/13/19 12:09 PM, Andrew Gierth wrote:
>>>>>>> "Andrew" == Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk> writes:
>> Andrew> 2. How far do we need to go to support existing uses of
>> Andrew> extra_float_digits? For example, do we need a way for clients to
>> Andrew> request that they get the exact same output as previously for
>> Andrew> extra_float_digits=2 or 3, rather than just assuming that any
>> Andrew> round-trip-exact value will do?
>>
>> Andrew> (this would likely mean adding a new GUC, rather than basing
>> Andrew> everything off extra_float_digits as the patch does now)
>>
>> So it turns out, for reasons that should have been obvious in advance,
>> that _of course_ we need this, because otherwise there's no way to do
>> the cross-version upgrade regression check.
>>
>> So we need something like exact_float_output='on' (default) that can be
>> selectively set to 'off' to restore the previous behavior of
>> extra_float_digits.
>>
>> Thoughts?
>
>
> I applied this:
>
>
> diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
> index 9edc7b9a02..a6b804ad2c 100644
> --- a/src/bin/pg_dump/pg_dump.c
> +++ b/src/bin/pg_dump/pg_dump.c
> @@ -1062,7 +1062,15 @@ setup_connection(Archive *AH, const char
> *dumpencoding,
>      * Set extra_float_digits so that we can dump float data exactly (given
>      * correctly implemented float I/O code, anyway)
>      */
> -   if (AH->remoteVersion >= 90000)
> +   if (getenv("PGDUMP_EXTRA_FLOAT_DIGITS"))
> +   {
> +       PQExpBuffer q = createPQExpBuffer();
> +       appendPQExpBuffer(q, "SET extra_float_digits TO %s",
> +                         getenv("PGDUMP_EXTRA_FLOAT_DIGITS"));
> +       ExecuteSqlStatement(AH, q->data);
> +       destroyPQExpBuffer(q);
> +   }
> +   else if (AH->remoteVersion >= 90000)
>         ExecuteSqlStatement(AH, "SET extra_float_digits TO 3");
>     else
>         ExecuteSqlStatement(AH, "SET extra_float_digits TO 2");
>
>
> Then I got the buildfarm cross-version-upgrade module to set the
> environment value to 0 in the appropriate cases. All the tests passed,
> as far back as 9.2, no new GUC needed.
>
> We might not want to use that in more real-world cases of pg_dump use,
> but I think for this purpose it should be fine.
>
>

I haven't seen a response to this. Cross version upgrade testing is
still broken. I don't think we need a GUC to fix it, but we do need this
or a new switch to tell pg_dump what to set extra_float_digits to,
unless someone has a better idea.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services