Thread: text -> time cast problem

text -> time cast problem

From
Brent Verner
Date:
I noticed an incorrect example in doc/src/sgml/func.sgml...

brent=# SELECT EXTRACT(SECOND FROM TIME '17:12:28.5');date_part 
-----------       28
(1 row)

The documentation says this should return 28.5.  Digging a bit, I
noticed the following (discrepancy?).  Is this desired behavior?

brent=# select "time"('12:00:12.5');   time     
-------------12:00:12.50
(1 row)

brent=# select '12:00:12.5'::time;  time   
----------12:00:12
(1 row)

IMO, one of these needs to be fixed before RC1 is rolled.


On a similar note, it would be neat if there was an sgml tag like <examplequery> SELECT EXTRACT(SECOND FROM TIME
'17:12:28.5');</examplequery>
 
that would dynamically execute the query and insert the result in
the sgml file...  This would ensure the docs always agree with current
behavior ;-)

cheers. brent

-- 
"Develop your talent, man, and leave the world something. Records are 
really gifts from people. To think that an artist would love you enough
to share his music with anyone is a beautiful thing."  -- Duane Allman


Re: text -> time cast problem

From
Tom Lane
Date:
Brent Verner <brent@rcfile.org> writes:
> I noticed an incorrect example in doc/src/sgml/func.sgml...
> brent=# SELECT EXTRACT(SECOND FROM TIME '17:12:28.5');
>  date_part 
> -----------
>         28
> (1 row)

> The documentation says this should return 28.5.

Historically we've made EXTRACT(SECOND) return integral seconds, with
MILLISECOND/MICROSECOND field names for the fractional seconds.  So the
docs are incorrect with respect to the actual code behavior.

But ...

The SQL92 spec appears to intend that EXTRACT(SECOND) should return
seconds *and* fractional seconds.  In 6.6 syntax rule 4,
        4) If <extract expression> is specified, then
           Case:
           a) If <datetime field> does not specify SECOND, then the data             type of the result is exact
numericwith implementation-             defined precision and scale 0.
 
           b) Otherwise, the data type of the result is exact numeric             with implementation-defined precision
andscale. The             implementation-defined scale shall not be less than the spec-             ified or implied
<timefractional seconds precision> or <in-             terval fractional seconds precision>, as appropriate, of the
       SECOND <datetime field> of the <extract source>.
 

It looks to me like 4b *requires* the fractional part of the seconds
field to be returned.  (Of course, we're blithely ignoring the aspect
of this that requires an exact numeric result type, since our version
of EXTRACT returns float8, but let's not worry about that fine point
at the moment.)

Don't think I want to change this behavior for 7.2, but it ought to be
on the TODO list to fix it for 7.3.


> Digging a bit, I
> noticed the following (discrepancy?).  Is this desired behavior?

> brent=# select "time"('12:00:12.5');
>     time     
> -------------
>  12:00:12.50
> (1 row)

> brent=# select '12:00:12.5'::time;
>    time   
> ----------
>  12:00:12
> (1 row)

> IMO, one of these needs to be fixed before RC1 is rolled.

I'm not convinced that's broken.  You're missing an important point
(forgivable, because Thomas hasn't yet committed any documentation
about it): TIME now implies a precision specification, and the default
is TIME(0), ie no fractional digits.  Observe:

regression=# select '12:00:12.6'::time(0);  time
----------12:00:13
(1 row)

regression=# select '12:00:12.6'::time(2);   time
-------------12:00:12.60
(1 row)

In the pseudo-function-call case, there is no implicit precision
specification and thus the value does not get rounded.

BTW, this means that

SELECT EXTRACT(SECOND FROM TIME '17:12:28.5');

*should* return 28, because the TIME literal is implicitly TIME(0).
But if it were written TIME(1) '17:12:28.5' or more precision, then
I believe SQL92 requires the EXTRACT result to include the fraction.
        regards, tom lane


Re: text -> time cast problem

From
Tom Lane
Date:
Peter Eisentraut <peter_e@gmx.net> writes:
> Nope, the docs represent the behavior of the code at the time the docs
> were written.  The code is now in error with respect to the documented
> behaviour.  A quick check shows that PostgreSQL 7.0.2 agrees with
> including the fractional part.

[ checks ... ] As does 7.1.  You're right, that is how it used to behave.

> For details, I refer you to my Oct 5 message "Unhappiness with forced
> precision conversion for timestamp", where we already discussed
> essentially the same issue, but apparently we never did anything about it.

I think the rest of us were waiting on Lockhart to opine about it ...
not to mention do something about it ...
        regards, tom lane


Re: text -> time cast problem

From
Peter Eisentraut
Date:
Tom Lane writes:

> Brent Verner <brent@rcfile.org> writes:
> > I noticed an incorrect example in doc/src/sgml/func.sgml...
> > brent=# SELECT EXTRACT(SECOND FROM TIME '17:12:28.5');
> >  date_part
> > -----------
> >         28
> > (1 row)
>
> > The documentation says this should return 28.5.
>
> Historically we've made EXTRACT(SECOND) return integral seconds, with
> MILLISECOND/MICROSECOND field names for the fractional seconds.  So the
> docs are incorrect with respect to the actual code behavior.

Nope, the docs represent the behavior of the code at the time the docs
were written.  The code is now in error with respect to the documented
behaviour.  A quick check shows that PostgreSQL 7.0.2 agrees with
including the fractional part.  Probably this was broken as part of the
time/timestamp precision changes.  Definitely looks like a show-stopper to
me.

> BTW, this means that
>
> SELECT EXTRACT(SECOND FROM TIME '17:12:28.5');
>
> *should* return 28, because the TIME literal is implicitly TIME(0).
> But if it were written TIME(1) '17:12:28.5' or more precision, then

That appears to be what it does, but it's not correct.  I point you to
SQL92:
        16)The data type of a <time literal> that does not specify <time           zone interval> is TIME(P), where P
isthe number of digits in           <seconds fraction>, if specified, and 0 otherwise. The data           type of a
<timeliteral> that specifies <time zone interval>           is TIME(P) WITH TIME ZONE, where P is the number of digits
in          <seconds fraction>, if specified, and 0 otherwise.
 

In this "time literal" context, TIME does not take a precision value at
all.  The new code certainly has this wrong.

For details, I refer you to my Oct 5 message "Unhappiness with forced
precision conversion for timestamp", where we already discussed
essentially the same issue, but apparently we never did anything about it.

-- 
Peter Eisentraut   peter_e@gmx.net



Re: text -> time cast problem

From
Tom Lane
Date:
Peter Eisentraut <peter_e@gmx.net> writes:
> That appears to be what it does, but it's not correct.  I point you to
> SQL92:

>          16)The data type of a <time literal> that does not specify <time
>             zone interval> is TIME(P), where P is the number of digits in
>             <seconds fraction>, if specified, and 0 otherwise. The data
>             type of a <time literal> that specifies <time zone interval>
>             is TIME(P) WITH TIME ZONE, where P is the number of digits in
>             <seconds fraction>, if specified, and 0 otherwise.

> In this "time literal" context, TIME does not take a precision value at
> all.  The new code certainly has this wrong.

I believe it is a reasonable extension for us to accept
    time(2) '17:12:28.123'

as producing '17:12:28.12'.  This accords with our general extension to
accept <any-type-name> <string-literal> as a typed constant, whereas I
believe that SQL92 only envisions certain specific type names being used
in this way.

But you are definitely right that
    time '17:12:28.123'

should not strip the fractional digits.  From this it is a small step
to asserting that
    '17:12:28.123'::time

shouldn't either; in general we'd like TYPE 'LIT' and 'LIT'::TYPE to
produce the same answers.

> For details, I refer you to my Oct 5 message "Unhappiness with forced
> precision conversion for timestamp", where we already discussed
> essentially the same issue, but apparently we never did anything about it.

I think you have put your finger on the heart of the problem.  Some
further research shows that it's not EXTRACT(SECOND) that is refusing
to produce a fractional part; the problem is with the time literal.

As an experiment, I made the attached patch to gram.y, which implements
the change I originally proposed in the older thread: time/timestamp
type names that don't explicitly specify a precision should get typmod
-1, which will mean no coercion to a specific precision.  This does not
follow SQL92's notion of having specific default precisions for these
types, but it does agree with our current handling of NUMERIC (no forced
default precision there either).  I make the following observations:

1. All the regression tests still pass.

2. The case I was unhappy about in October works nicely now:

regression=# select '2001-10-04 13:52:42.845985-04'::timestamp;         timestamptz
-------------------------------2001-10-04 13:52:42.845985-04
(1 row)

3. The cases Brent is unhappy about all pass:

regression=# SELECT EXTRACT(SECOND FROM TIME '17:12:28.5');date_part
-----------     28.5
(1 row)

regression=# select "time"('12:00:12.5');   time
-------------12:00:12.50
(1 row)

regression=# select '12:00:12.5'::time;   time
-------------12:00:12.50
(1 row)


This needs further thought and testing before I'd dare call it a
solution, but it does seem to suggest the direction we should pursue.
        regards, tom lane

*** src/backend/parser/gram.y.orig    Thu Nov 15 23:08:33 2001
--- src/backend/parser/gram.y    Tue Dec  4 17:52:10 2001
***************
*** 4058,4064 ****                 {                     $$ = $1;                     if ($2 != -1)
!                         $$->typmod = ((($2 & 0x7FFF) << 16) | 0xFFFF);                 }         | ConstInterval '('
Iconst')' opt_interval                 {
 
--- 4058,4064 ----                 {                     $$ = $1;                     if ($2 != -1)
!                         $$->typmod = (($2 << 16) | 0xFFFF);                 }         | ConstInterval '(' Iconst ')'
opt_interval                {
 
***************
*** 4328,4337 ****                      * - thomas 2001-09-06                      */                     $$->timezone
=$2;
 
!                     /* SQL99 specified a default precision of six.
!                      * - thomas 2001-09-30
!                      */
!                     $$->typmod = 6;                 }         | TIME '(' Iconst ')' opt_timezone                 {
--- 4328,4334 ----                      * - thomas 2001-09-06                      */                     $$->timezone
=$2;
 
!                     $$->typmod = -1;                 }         | TIME '(' Iconst ')' opt_timezone                 {
***************
*** 4352,4361 ****                         $$->name = xlateSqlType("timetz");                     else
      $$->name = xlateSqlType("time");
 
!                     /* SQL99 specified a default precision of zero.
!                      * - thomas 2001-09-30
!                      */
!                     $$->typmod = 0;                 }         ; 
--- 4349,4355 ----                         $$->name = xlateSqlType("timetz");                     else
      $$->name = xlateSqlType("time");
 
!                     $$->typmod = -1;                 }         ; 
***************
*** 5603,5609 ****                     n->val.val.str = $2;                     /* precision is not specified, but
fieldsmay be... */                     if ($3 != -1)
 
!                         n->typename->typmod = ((($3 & 0x7FFF) << 16) | 0xFFFF);                     $$ = (Node *)n;
             }         | ConstInterval '(' Iconst ')' Sconst opt_interval
 
--- 5597,5603 ----                     n->val.val.str = $2;                     /* precision is not specified, but
fieldsmay be... */                     if ($3 != -1)
 
!                         n->typename->typmod = (($3 << 16) | 0xFFFF);                     $$ = (Node *)n;
  }         | ConstInterval '(' Iconst ')' Sconst opt_interval 

Re: text -> time cast problem

From
Brent Verner
Date:
[2001-12-04 18:15] Tom Lane said:
| Peter Eisentraut <peter_e@gmx.net> writes:
| > That appears to be what it does, but it's not correct.  I point you to
| > SQL92:
| 
| >          16)The data type of a <time literal> that does not specify <time
| >             zone interval> is TIME(P), where P is the number of digits in
| >             <seconds fraction>, if specified, and 0 otherwise. The data
| >             type of a <time literal> that specifies <time zone interval>
| >             is TIME(P) WITH TIME ZONE, where P is the number of digits in
| >             <seconds fraction>, if specified, and 0 otherwise.
| 
| > In this "time literal" context, TIME does not take a precision value at
| > all.  The new code certainly has this wrong.

The current handling of <time literal> and <timestamp literal> appear 
to be correct from my reading of the sql standards.

| But you are definitely right that
| 
|         time '17:12:28.123'

sql-99 seems to contradict this assertion.
page 160 (Syntax Rules, 6.1 <data type>)  30) If <time precision> is not specified, then 0 (zero) is implicit.     If
<timestampprecision> is not specified, then 6 is implicit.
 

meaning (to me) that "TIME" should be equivalent "TIME(0)".
 [snip]

| in general we'd like TYPE 'LIT' and 'LIT'::TYPE to
| produce the same answers.

I agree wholly with this statement.
 [snip]

To get back to my original problem report... 
 I believe the proper solution would be to update the documentation 
to reflect the fact that "TIME 'hh:mm:ss.ff'" will correctly drop 
the '.ff' seconds fraction.
 That said, how should "time"('hh:mm:ss.ff') behave?  How could
<time precision> be specified in this syntax?  If there is no way
to specify <time precision>, I believe we should drop the seconds
fraction from <time string>.  Is there any reason we couldn't drop
this typename-as-a-function-call syntax for types like "time" and
"timestamp"?

cheers. brent

p.s. sorry for not replying sooner...

-- 
"Develop your talent, man, and leave the world something. Records are 
really gifts from people. To think that an artist would love you enough
to share his music with anyone is a beautiful thing."  -- Duane Allman


Re: text -> time cast problem

From
Tom Lane
Date:
Brent Verner <brent@rcfile.org> writes:
> [2001-12-04 18:15] Tom Lane said:
> | But you are definitely right that
> | 
> |         time '17:12:28.123'

> sql-99 seems to contradict this assertion.
> page 160 (Syntax Rules, 6.1 <data type>) 
>   30) If <time precision> is not specified, then 0 (zero) is implicit.
>       If <timestamp precision> is not specified, then 6 is implicit.

But that's <data type>, ie, they're specifying the implied width of
a table column that's declared "foo time".  The rules for <time literal>
are different.

Our problem is that we want to generalize the notion of <time literal>
to be <datatype> <string literal>, and this makes it hard to have
datatype-specific rules that differ from the rules that apply in the
column-datatype context.

My thought is that we should resolve this conflict by rejecting the
part of the spec that assigns fixed default precisions to time
and timestamp columns, the same as we have done for type numeric.
There's no benefit to the user in that requirement; it's only a
crutch for implementations that cannot cope with variable-width
columns effectively.  If people want a column that rounds fractional
inputs to integral seconds, let 'em say "TIME(0)".  But I don't
think that "TIME" should do so, especially when the spec provides
no alternative way to get the effect of "time with no particular
precision restriction".  It's the old "text vs varchar(N)" game all
over again.

>   I believe the proper solution would be to update the documentation 
> to reflect the fact that "TIME 'hh:mm:ss.ff'" will correctly drop 
> the '.ff' seconds fraction.

No, because that behavior is *not* correct, neither per spec nor per
our historical behavior.
        regards, tom lane


Re: text -> time cast problem

From
Brent Verner
Date:
[2001-12-06 13:00] Tom Lane said:
| Brent Verner <brent@rcfile.org> writes:
| > [2001-12-04 18:15] Tom Lane said:
| > | But you are definitely right that
| > | 
| > |         time '17:12:28.123'
| 
| > sql-99 seems to contradict this assertion.
| > page 160 (Syntax Rules, 6.1 <data type>)
|   
| >   30) If <time precision> is not specified, then 0 (zero) is implicit.
| >       If <timestamp precision> is not specified, then 6 is implicit.
| 
| But that's <data type>, ie, they're specifying the implied width of
| a table column that's declared "foo time".  The rules for <time literal>
| are different.

Understood now.

I'd  misunderstood the meaning of "if specified" in the <time literal>
definition;  specifically, I interpreted it as "if specified by 
<time precision>" as opposed to the intended meaning of "if specified 
in <time string>"... so off I went seeking additional definitions
to support treating "TIME <time string>" as "TIME(0) <time string>"...

Thanks for swinging the clue stick my way :-)

| Our problem is that we want to generalize the notion of <time literal>
| to be <datatype> <string literal>, and this makes it hard to have
| datatype-specific rules that differ from the rules that apply in the
| column-datatype context.
| 
| My thought is that we should resolve this conflict by rejecting the
| part of the spec that assigns fixed default precisions to time
| and timestamp columns, the same as we have done for type numeric.
| There's no benefit to the user in that requirement; it's only a
| crutch for implementations that cannot cope with variable-width
| columns effectively.  If people want a column that rounds fractional
| inputs to integral seconds, let 'em say "TIME(0)".  But I don't
| think that "TIME" should do so, especially when the spec provides
| no alternative way to get the effect of "time with no particular
| precision restriction".  It's the old "text vs varchar(N)" game all
| over again.

This seems fair.  Would this approach imply that CURRENT_TIME and 
CURRENT_TIMESTAMP should not apply default precision to their return 
values?  Right now, "CURRENT_TIME" is equivalent to "CURRENT_TIME(0)" 
and "CURRENT_TIMESTAMP" eq to "CURRENT_TIMESTAMP(6)".

cheers. brent

-- 
"Develop your talent, man, and leave the world something. Records are 
really gifts from people. To think that an artist would love you enough
to share his music with anyone is a beautiful thing."  -- Duane Allman


Re: text -> time cast problem

From
Tom Lane
Date:
Brent Verner <brent@rcfile.org> writes:
> This seems fair.  Would this approach imply that CURRENT_TIME and 
> CURRENT_TIMESTAMP should not apply default precision to their return 
> values?  Right now, "CURRENT_TIME" is equivalent to "CURRENT_TIME(0)" 
> and "CURRENT_TIMESTAMP" eq to "CURRENT_TIMESTAMP(6)".

Yes, I had been thinking that myself, but hadn't got round to mentioning
it to the list yet.  (Even if you do accept default precisions for time
& timestamp columns, I can see nothing in the spec that justifies
applying those default precisions to CURRENT_TIME/TIMESTAMP.  AFAICS,
the precision of their results when they are given no argument is
just plain not specified.)
        regards, tom lane


Re: text -> time cast problem

From
Thomas Lockhart
Date:
> > This seems fair.  Would this approach imply that CURRENT_TIME and
> > CURRENT_TIMESTAMP should not apply default precision to their return
> > values?  Right now, "CURRENT_TIME" is equivalent to "CURRENT_TIME(0)"
> > and "CURRENT_TIMESTAMP" eq to "CURRENT_TIMESTAMP(6)".
> Yes, I had been thinking that myself, but hadn't got round to mentioning
> it to the list yet.  (Even if you do accept default precisions for time
> & timestamp columns, I can see nothing in the spec that justifies
> applying those default precisions to CURRENT_TIME/TIMESTAMP.  AFAICS,
> the precision of their results when they are given no argument is
> just plain not specified.)

I'll shift the default precisions of CURRENT_TIME to match that of
CURRENT_TIMESTAMP, which is currently six (6). As you might know, 7.2
has sub-second system time available, which was not true in previous
releases. But that time is only good to microseconds, so the six digits
of precision is a good match for that.
                     - Thomas


Re: text -> time cast problem

From
"Christopher Kings-Lynne"
Date:
Just a quick question for 7.1.3:

I notice that where as you can do this:

insert into table values ('now');

To insert a current timestamp, if you try using the more standard syntax:

insert into table values ('CURRENT_TIMESTAMP');

You get the word 'current' inserted.  However if you do this:

insert into table values (CURRENT_TIMESTAMP);

It works as expected.

Is there anything wrong with any of these behaviours, and has any of it been
changed for 7.2?  I kind think that the quoted CURRENT_TIMESTAMP should work
just like the unquoted...

Chris

> -----Original Message-----
> From: pgsql-hackers-owner@postgresql.org
> [mailto:pgsql-hackers-owner@postgresql.org]On Behalf Of Tom Lane
> Sent: Friday, 7 December 2001 10:21 PM
> To: Brent Verner
> Cc: Peter Eisentraut; PostgreSQL Development; Thomas Lockhart
> Subject: Re: [HACKERS] text -> time cast problem
>
>
> Brent Verner <brent@rcfile.org> writes:
> > This seems fair.  Would this approach imply that CURRENT_TIME and
> > CURRENT_TIMESTAMP should not apply default precision to their return
> > values?  Right now, "CURRENT_TIME" is equivalent to "CURRENT_TIME(0)"
> > and "CURRENT_TIMESTAMP" eq to "CURRENT_TIMESTAMP(6)".
>
> Yes, I had been thinking that myself, but hadn't got round to mentioning
> it to the list yet.  (Even if you do accept default precisions for time
> & timestamp columns, I can see nothing in the spec that justifies
> applying those default precisions to CURRENT_TIME/TIMESTAMP.  AFAICS,
> the precision of their results when they are given no argument is
> just plain not specified.)
>
>             regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
>     (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
>



Re: text -> time cast problem

From
Thomas Lockhart
Date:
...
> Is there anything wrong with any of these behaviours, and has any of it been
> changed for 7.2?  I kind think that the quoted CURRENT_TIMESTAMP should work
> just like the unquoted...

No, it is pretty much the same as before. Quoted 'current_timestamp' has
nothing to do with the SQL9x standard, while unquoted CURRENT_TIMESTAMP
does. For 7.2, we are dropping the somewhat interesting 'current' value,
which was evaluated at the time a math operation was performed, at which
time it became equivalent to 'now'.
                  - Thomas


Re: text -> time cast problem

From
Bruce Momjian
Date:
> > > This seems fair.  Would this approach imply that CURRENT_TIME and
> > > CURRENT_TIMESTAMP should not apply default precision to their return
> > > values?  Right now, "CURRENT_TIME" is equivalent to "CURRENT_TIME(0)"
> > > and "CURRENT_TIMESTAMP" eq to "CURRENT_TIMESTAMP(6)".
> > Yes, I had been thinking that myself, but hadn't got round to mentioning
> > it to the list yet.  (Even if you do accept default precisions for time
> > & timestamp columns, I can see nothing in the spec that justifies
> > applying those default precisions to CURRENT_TIME/TIMESTAMP.  AFAICS,
> > the precision of their results when they are given no argument is
> > just plain not specified.)
> 
> I'll shift the default precisions of CURRENT_TIME to match that of
> CURRENT_TIMESTAMP, which is currently six (6). As you might know, 7.2
> has sub-second system time available, which was not true in previous
> releases. But that time is only good to microseconds, so the six digits
> of precision is a good match for that.

Is this all resolved?

--  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,
Pennsylvania19026
 


Re: text -> time cast problem

From
Thomas Lockhart
Date:
...
> Is this all resolved?

Some time ago, yes. (Pun intended ;)
                - Thomas


Re: text -> time cast problem

From
Bruce Momjian
Date:
> ...
> > Is this all resolved?
> 
> Some time ago, yes. (Pun intended ;)

Thanks.  Sometimes I can not tell from the email messages.

--  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,
Pennsylvania19026