Horton

Thread

Commits

  1. Doc: adjust examples of EXTRACT() output to match current reality.

  2. Change return type of EXTRACT to numeric

  1. BUG #17866: behavior does not match documentation

    The Post Office <noreply@postgresql.org> — 2023-03-23T15:10:08Z

    The following bug has been logged on the website:

Bug reference:      17866
Logged by:          Евгений Жужнев
Email address:      eugeny@zhuzhnev.com
PostgreSQL version: 15.2
Operating system:   Oracle Linux 9
Description:        

sudo -u postgres psql pgc
psql (15.2)
Type "help" for help.

pgc=# SELECT EXTRACT(EPOCH FROM INTERVAL '5 days 3 hours');
    extract    
---------------
 442800.000000
(1 row)
--
But in documentation we can see:
SELECT EXTRACT(EPOCH FROM INTERVAL '5 days 3 hours');
Result: 442800
https://www.postgresql.org/docs/current/functions-datetime.html

  2. Re: BUG #17866: behavior does not match documentation

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-03-23T18:31:35Z

    PG Bug reporting form <noreply@postgresql.org> writes:
> pgc=# SELECT EXTRACT(EPOCH FROM INTERVAL '5 days 3 hours');
>     extract    
> ---------------
>  442800.000000
> (1 row)

> But in documentation we can see:
> SELECT EXTRACT(EPOCH FROM INTERVAL '5 days 3 hours');
> Result: 442800

Hmm.  That's not wrong exactly, but it's fair to question whether it
satisfies the POLA.  This behavior change happened with commit a2da77cdb
(Change return type of EXTRACT to numeric), and it was intentional
according to the commit log:
    
    - Return values when extracting fields with possibly fractional
      values, such as second and epoch, now have the full scale that the
      value has internally (so, for example, '1.000000' instead of just
      '1').

But exactly nothing was mentioned of that in user-facing docs.
I wonder if we should rethink that and have these operations strip
insignificant trailing zeroes.  It looks like that could be done as
practically a one-liner change, since int64_div_fast_to_numeric isn't
yet used anywhere except in these datetime extraction functions.

(The test cases that change behavior are either reverting to their
pre-a2da77cdb output, or were newly added in that commit.)

Thoughts?

			regards, tom lane

  3. Re: BUG #17866: behavior does not match documentation

    Daniel Gustafsson <daniel@yesql.se> — 2023-03-23T21:25:49Z

    > On 23 Mar 2023, at 19:31, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> This behavior change happened with commit a2da77cdb
> (Change return type of EXTRACT to numeric), and it was intentional
> according to the commit log:
> 
>    - Return values when extracting fields with possibly fractional
>      values, such as second and epoch, now have the full scale that the
>      value has internally (so, for example, '1.000000' instead of just
>      '1').
> 
> But exactly nothing was mentioned of that in user-facing docs.

Skimming the thread it's also not really discussed much AFAICT.  It's first
brought up in [0] as:

	When extracting seconds or microseconds, I made it always produce 6 or
	3 decimal places, even if they are zero.  I don't know if we want that
	or what behavior we want.  That's what all the changes in the
	regression tests are about.  Everything else passes unchanged.

There are no follow-ups to that though.

> I wonder if we should rethink that and have these operations strip
> insignificant trailing zeroes.

Any app relying on insignificant trailing zeroes seems broken, but the inverse
can be argued as well.  It's however quite easy to argue for the stripped
output being more readable though.

--
Daniel Gustafsson

[0] https://www.postgresql.org/message-id/a3be61d9-f44b-7fce-3dc8-d700fdfb6f48%402ndquadrant.com

  4. Re: BUG #17866: behavior does not match documentation

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-03-23T21:54:50Z

    Daniel Gustafsson <daniel@yesql.se> writes:
> On 23 Mar 2023, at 19:31, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I wonder if we should rethink that and have these operations strip
>> insignificant trailing zeroes.

> Any app relying on insignificant trailing zeroes seems broken, but the inverse
> can be argued as well.  It's however quite easy to argue for the stripped
> output being more readable though.

It's more readable for sure, and it duplicates what you got in pre-v14
versions, at least textually.  I'm not sure I'd propose back-patching
this, but it feels like a good idea for HEAD.

The argument for the current behavior probably goes like "we're exposing
the actual precision of the value".  But I don't believe that we are;
we're exposing the maximum possible precision.  We have no way to know
how precise the original timestamp or interval input was.  So I don't
think there's much basis for claiming that the result is good to six
fractional digits.

			regards, tom lane

  5. Re: BUG #17866: behavior does not match documentation

    Daniel Gustafsson <daniel@yesql.se> — 2023-03-23T22:00:24Z

    > On 23 Mar 2023, at 22:54, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> Daniel Gustafsson <daniel@yesql.se> writes:
>> On 23 Mar 2023, at 19:31, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> I wonder if we should rethink that and have these operations strip
>>> insignificant trailing zeroes.
> 
>> Any app relying on insignificant trailing zeroes seems broken, but the inverse
>> can be argued as well.  It's however quite easy to argue for the stripped
>> output being more readable though.
> 
> It's more readable for sure, and it duplicates what you got in pre-v14
> versions, at least textually.  I'm not sure I'd propose back-patching
> this, but it feels like a good idea for HEAD.

Agreed, I don't think this is material for backpatching.

--
Daniel Gustafsson

  6. Re: BUG #17866: behavior does not match documentation

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-04-10T17:11:31Z

    Daniel Gustafsson <daniel@yesql.se> writes:
> On 23 Mar 2023, at 22:54, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> It's more readable for sure, and it duplicates what you got in pre-v14
>> versions, at least textually.  I'm not sure I'd propose back-patching
>> this, but it feels like a good idea for HEAD.

> Agreed, I don't think this is material for backpatching.

Since we didn't get anything done about that for v16, I concluded
that adjusting the docs is a better idea.  If anyone takes this
idea up in v17 or beyond, they can always revert fbbd7edca at that
time.

			regards, tom lane

  7. Re: BUG #17866: behavior does not match documentation

    Daniel Gustafsson <daniel@yesql.se> — 2023-04-10T18:33:45Z

    > On 10 Apr 2023, at 19:11, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> Daniel Gustafsson <daniel@yesql.se> writes:
>> On 23 Mar 2023, at 22:54, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> It's more readable for sure, and it duplicates what you got in pre-v14
>>> versions, at least textually.  I'm not sure I'd propose back-patching
>>> this, but it feels like a good idea for HEAD.
> 
>> Agreed, I don't think this is material for backpatching.
> 
> Since we didn't get anything done about that for v16, I concluded
> that adjusting the docs is a better idea.  If anyone takes this
> idea up in v17 or beyond, they can always revert fbbd7edca at that
> time.

The more I think about it the more I lean towards not changing anything
(besides the docs which you did).  It's been in two releases already and hardly
any complaints, so it seems to be have been quite well accepted.

--
Daniel Gustafsson