BUG #1517: SQL interval syntax is accepted by the parser, but the interpretation is bogus

Started by Roy Badamiabout 21 years ago53 messagesbugs
Jump to latest
#1Roy Badami
roy@gnomon.org.uk

The following bug has been logged online:

Bug reference: 1517
Logged by: Roy Badami
Email address: roy@gnomon.org.uk
PostgreSQL version: 8.0.1
Operating system: Solaris 9
Description: SQL interval syntax is accepted by the parser, but the
interpretation is bogus
Details:

The parser accepts SQL interval syntax, but then silently ignores it,
treating it as a zero interval.

radius=# select date '2005-01-01' + interval '1' month;
?column?
---------------------
2005-01-01 00:00:00
(1 row)

radius=# select timestamp '2005-01-1 00:00:00' + interval '1' minute;
?column?
---------------------
2005-01-01 00:00:00
(1 row)

radius=#

#2Bruce Momjian
bruce@momjian.us
In reply to: Roy Badami (#1)
Re: BUG #1517: SQL interval syntax is accepted by the parser,

Roy Badami wrote:

The following bug has been logged online:

Bug reference: 1517
Logged by: Roy Badami
Email address: roy@gnomon.org.uk
PostgreSQL version: 8.0.1
Operating system: Solaris 9
Description: SQL interval syntax is accepted by the parser, but the
interpretation is bogus
Details:

The parser accepts SQL interval syntax, but then silently ignores it,
treating it as a zero interval.

radius=# select date '2005-01-01' + interval '1' month;
?column?
---------------------
2005-01-01 00:00:00
(1 row)

radius=# select timestamp '2005-01-1 00:00:00' + interval '1' minute;
?column?
---------------------
2005-01-01 00:00:00
(1 row)

Well, that certainly belongs in the 'bizarre' category. It should not
accept that syntax. It should require the 'month' or 'minute' to be in
single quotes. This is wrong:

test=> select date '2005-01-01' + interval '1' month;
?column?
---------------------
2005-01-01 00:00:00
(1 row)

This is right:

test=> select date '2005-01-01' + interval '1 month';
?column?
---------------------
2005-02-01 00:00:00
(1 row)

In fact when the 'month' is outside the quotes, it modifies the
'interval', like this:

test=> select date '2005-01-01' + interval '1 year' year to month;
?column?
---------------------
2006-01-01 00:00:00
(1 row)

and in fact the '1' is taken to be 1 second:

test=> select date '2005-01-01' + interval '1';
?column?
---------------------
2005-01-01 00:00:01
(1 row)

So, in fact these work just fine:

test=> select date '2005-01-01' + interval '1' second;
?column?
---------------------
2005-01-01 00:00:01
(1 row)

test=> select date '2005-01-01' + interval '1' hour to second;
?column?
---------------------
2005-01-01 00:00:01
(1 row)

Do we need help in this area? Yes. Where? I don't know.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#2)
Re: BUG #1517: SQL interval syntax is accepted by the parser,

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Well, that certainly belongs in the 'bizarre' category. It should not
accept that syntax. It should require the 'month' or 'minute' to be in
single quotes.

No, it shouldn't; read the SQL spec. AFAICS the syntax
select interval '1' month
is perfectly spec-compliant. The variant
select interval '1 month'
is *not* per-spec, it is a Postgres-ism.

Tom Lockhart was working on this stuff shortly before he decided that
raising horses was a more interesting use of his spare time. It doesn't
look like he ever quite finished. I tried several back versions of
Postgres to see if it had ever operated correctly and the answer seems
to be "no" :-( ... although we have managed to fail in more than one
way over the years ...

regards, tom lane

#4Roy Badami
roy@gnomon.org.uk
In reply to: Tom Lane (#3)
Re: BUG #1517: SQL interval syntax is accepted by the parser,

Tom> No, it shouldn't; read the SQL spec. AFAICS the syntax
Tom> select interval '1' month is perfectly spec-compliant. The
Tom> variant select interval '1 month' is *not* per-spec, it is a
Tom> Postgres-ism.

That is my understanding, though I don't have a copy of the spec (my
reference is Date & Darwen's "A guide to the SQL standard")

However, it may be better if the PostgreSQL parser rejected the
syntax. The current behaviour would seem to be a smoking gun for
people porting ANSI-compliant SQL applications (assuming such things
exist :) to PostgreSQL.

-roy

#5Roy Badami
roy@gnomon.org.uk
In reply to: Tom Lane (#3)
Re: BUG #1517: SQL interval syntax is accepted by the parser,

Tom> AFAICS the syntax
Tom> select interval '1' month
Tom> is perfectly spec-compliant.

Well, it's not _perfectly_ spec compliant, because AIUI SELECTs
without FROM clauses are a postgres-ism, too. But I'm just
nitpicking...

-roy

#6Bruce Momjian
bruce@momjian.us
In reply to: Roy Badami (#4)
Re: BUG #1517: SQL interval syntax is accepted by the parser,

Roy Badami wrote:

Tom> No, it shouldn't; read the SQL spec. AFAICS the syntax
Tom> select interval '1' month is perfectly spec-compliant. The
Tom> variant select interval '1 month' is *not* per-spec, it is a
Tom> Postgres-ism.

That is my understanding, though I don't have a copy of the spec (my
reference is Date & Darwen's "A guide to the SQL standard")

We have links to the spec in the developer's FAQ.

However, it may be better if the PostgreSQL parser rejected the
syntax. The current behaviour would seem to be a smoking gun for
people porting ANSI-compliant SQL applications (assuming such things
exist :) to PostgreSQL.

So, we have a few major problems with intervals. Let me think a little
and I will summarize.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#7Roy Badami
roy@gnomon.org.uk
In reply to: Bruce Momjian (#6)
Re: BUG #1517: SQL interval syntax is accepted by the parser,

Bruce> So, we have a few major problems with intervals. Let me
Bruce> think a little and I will summarize.

FWIW, AFAICT the problems I reported in bug 1517 and 1518 all relate
to undocumented features of PostgreSQL.

All the documented interval functionality works fine. The
undocumented support for ANSI SQL interval data types and litereals
doesn't :-/

-roy

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Roy Badami (#7)
Re: BUG #1517: SQL interval syntax is accepted by the parser,

Roy Badami <roy@gnomon.org.uk> writes:

All the documented interval functionality works fine. The
undocumented support for ANSI SQL interval data types and litereals
doesn't :-/

I think the reason it's not documented is precisely that Tom never
finished it. It may not be very far away though --- seeing that the
grammar support exists, I suspect the only missing piece is that
interval_in isn't paying attention to the typmod info, as it should
do to disambiguate input like '1'. Or maybe that support is partially
there but doesn't quite work. Feel like hacking the code?

regards, tom lane

#9Roy Badami
roy@gnomon.org.uk
In reply to: Tom Lane (#8)
Re: BUG #1517: SQL interval syntax is accepted by the parser,

Tom> Feel like hacking the code?

Hmm, in principle I might take a look some time; in reality it's
unlikely I'll have time any time soon...

There are some design issues involved, though. If you have the type
modifier, do you isnist on SQL syntax in the string?

ie do you accept

interval '1 day 1 hour' day to second

Personally I think it would be a bad idea to allow hybrid SQL/postgres
syntax like this.

IMHO, you should either write

interval '1 day 1 hour'

(postgres style), or

interval '1 1:00:00' day to second

(SQL style.)

Hmm, except writing the above has just raised another question. Is
that what the postgres-ism really means (I think it does) or does it
mean

interval '1 1' day to hour

Once you start distinguishing your interval types, does this become
important? Actually, I can't immediately see a case where it would
matter, but that doesn't mean there isn't one...

-roy

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Roy Badami (#9)
Re: BUG #1517: SQL interval syntax is accepted by the parser,

Roy Badami <roy@gnomon.org.uk> writes:

ie do you accept
interval '1 day 1 hour' day to second

I think we have to, and the reason is that this isn't different under
the hood from reading the external value '1 day 1 hour' and storing
it into a column that has the DAY TO SECOND typmod. If we reject
the above we'd be breaking existing dump files. Furthermore this
would imply that dump output from a constrained interval column
would *have to* not have any decoration; ie we could only output
'1 1' and not '1 day 1 hour'. Regardless of what the spec says,
I find the former dangerously ambiguous.

I'm happy to see our code upgraded to accept the spec's syntax.
I won't be happy to see it changed to reject input that we used
to accept, especially when the only argument for doing so is a
narrow-minded insistence that we can't accept anything beyond
what the spec says.

regards, tom lane

#11Roy Badami
roy@gnomon.org.uk
In reply to: Tom Lane (#10)
Re: BUG #1517: SQL interval syntax is accepted by the parser,

"Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:

ie do you accept interval '1 day 1 hour' day to second

Tom> I think we have to, and the reason is that this isn't
Tom> different under the hood from reading the external value '1
Tom> day 1 hour' and storing it into a column that has the DAY TO
Tom> SECOND typmod.

I don't know anything about the postgres internals, but I don't see it
has to be this way.

INTERVAL '1 day 1 hour' DAY TO SECOND

won't occur in any existing dump file. But if it's important to treat
this the same as casting the string '1 day 1 hour' to type INTERVAL
DAY TO SECOND then yes, you'll have to accept it.

But this is just syntax; I don't see why you have to interpret it that
way...

But on refelction if you want to treat

INTERVAL 'postgres-interval' ansi-interval-type

as equivalent to

CAST (INTERVAL 'postgres-interval' AS INTERVAL ansi-interval-type)

that's probably not unreasonable. Though it creates an inconsistency
with the current (undocumented) postgresism of treating

INTERVAL '1'

as

INTERVAL '1 second'

since clearly you can't treat the ANSI interval

INTERVAL '1' HOUR

as
CAST (INTERVAL '1 second' AS INTERVAL HOUR)

-roy

#12Roy Badami
roy@gnomon.org.uk
In reply to: Roy Badami (#11)
Re: BUG #1517: SQL interval syntax is accepted by the parser,

Similary the undocumented postgresism of interpreting

INTERVAL '1:02'

as 1 hour 2 minutes is consistent with the ANSI

INTERVAL '1:02' HOUR TO MINUTE

but not with the ANSI

INTERVAL '1:02' MINUTE TO SECOND

which of course means 1 minute 2 seconds.

The fact is that ANSI interval syntax is very different from postgres
interval syntax. In ANSI interval syntax the literal string can only
be interpreted in the context of the interval type; in postgres
interval syntax the literal string has a well defined meaning in and
of itself, and no interval type is explicitly declared.

So I think I'm back to where I started. Attempting to define
semantics for a hybrid format, where you have an ANSI interval type
but the literal string formatted in postgres interval format is
unnecessarity confusing and complicated.

-roy

#13Tom Lane
tgl@sss.pgh.pa.us
In reply to: Roy Badami (#12)
Re: BUG #1517: SQL interval syntax is accepted by the parser,

Roy Badami <roy@gnomon.org.uk> writes:

Similary the undocumented postgresism of interpreting
INTERVAL '1:02'
as 1 hour 2 minutes is consistent with the ANSI
INTERVAL '1:02' HOUR TO MINUTE
but not with the ANSI
INTERVAL '1:02' MINUTE TO SECOND
which of course means 1 minute 2 seconds.

Well, that's an annoying case but I don't think it means we should throw
up our hands and reject cases that are (a) perfectly unambiguous and
(b) accepted by the present and past code.

We have to be able to support casts from undecorated INTERVAL to
INTERVALs with typmods, so most of these issues *have* to be dealt with
anyway; we can't arbitrarily reject them. What I am thinking is that
(a) if the input string is undecorated or ambiguous, use the typmod
to help resolve it --- in particular this should cover all of the
spec-mandated cases.
(b) if it is unambiguous Postgres-style syntax, read it that way and
then perform a cast to the restricted interval type.

regards, tom lane

#14Bruce Momjian
bruce@momjian.us
In reply to: Roy Badami (#9)
Re: BUG #1517: SQL interval syntax is accepted by the parser,

Roy Badami wrote:

Tom> Feel like hacking the code?

Hmm, in principle I might take a look some time; in reality it's
unlikely I'll have time any time soon...

There are some design issues involved, though. If you have the type
modifier, do you isnist on SQL syntax in the string?

ie do you accept

select interval '1 day 1 hour' day to second

Personally I think it would be a bad idea to allow hybrid SQL/postgres
syntax like this.

I am wondering why we allow the 'interval' data type specification to be
after the string. It seems this should be written as:

select interval day to second '1 day 1 hour'

However, we don't support that syntax, only the one with the
specification after.

Timestamp does it logically with 'time zone':

test=> select timestamp with time zone '2004-01-01';
timestamptz
------------------------
2004-01-01 00:00:00-05
(1 row)

test=> select timestamp '2004-01-01' with time zone;
ERROR: syntax error at or near "with" at character 31

So, I am thinking we should allow something like this:

interval day to second '1' hour

where the 'day to second' is the interval data type specification and
'hour' is the data value specification.

I realize this might break backward compatibility but we never
documented that syntax and we need to use it to be consistent with the
SQL standard.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#15Roy Badami
roy@gnomon.org.uk
In reply to: Bruce Momjian (#14)
Re: BUG #1517: SQL interval syntax is accepted by the parser,

Bruce> select interval day to second '1 day 1 hour'

Bruce> However, we don't support that syntax, only the one with
Bruce> the specification after.

Is that valid ANSI SQL?

-roy

#16Bruce Momjian
bruce@momjian.us
In reply to: Roy Badami (#15)
Re: BUG #1517: SQL interval syntax is accepted by the parser,

Roy Badami wrote:

Bruce> select interval day to second '1 day 1 hour'

Bruce> However, we don't support that syntax, only the one with
Bruce> the specification after.

Is that valid ANSI SQL?

No idea. It just seemed like the data type specification and the data
value specification has to be split apart somehow. Right now we use the
clause after the string as the date type specification, and I see you
saying that the data value specification has to after the string. Is
that correct?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#17Roy Badami
roy@gnomon.org.uk
In reply to: Bruce Momjian (#16)
Re: BUG #1517: SQL interval syntax is accepted by the parser,

Bruce> somehow. Right now we use the clause after the string as
Bruce> the date type specification, and I see you saying that the
Bruce> data value specification has to after the string. Is that
Bruce> correct?

Well, that's what 'A guide to the SQL standard' gives as the syntax
for interval literals. Note too that this isn't just some optional
qualifier, it's presence is mandatory in ANSI SQL, since without it
you can't interpret the string.

I had a brief look at the standard, but I don't know my way around it
and couldn't immeidately find where this is specified...

-roy

#18Roy Badami
roy@gnomon.org.uk
In reply to: Bruce Momjian (#14)
Re: BUG #1517: SQL interval syntax is accepted by the parser,

Bruce> test=> select timestamp with time zone '2004-01-01';

Also, FWIW, according to the postgres doc this is a postgresism. The
'with time zone' clause never occurs in an ANSI timestamp literal;
whether it is a timestamp or a timestamp with time zone depends on
whether a time zone specification is included in the literal string.

-roy

#19Bruce Momjian
bruce@momjian.us
In reply to: Roy Badami (#15)
Re: BUG #1517: SQL interval syntax is accepted by the parser,

Roy Badami wrote:

Bruce> select interval day to second '1 day 1 hour'

Bruce> However, we don't support that syntax, only the one with
Bruce> the specification after.

Is that valid ANSI SQL?

I guess my point is that we should allow:

select interval '1' day '1' hour

as SQL standard and equavalent to:

select interval '1 day 1 hour'

and if we need to specify the data type we would add it before
the data value:

select interval day to second '1' day '1' hour

I see no way to support the SQL syntax and allow the data type
specification after the data value, and it doesn't make any sense to do
so because it isn't logical and probably not used by anyone, though we
might be able to support it as a specialized case.

We could accept:

select interval year to month '1' month
select interval year to month '1 month'

and as a special case:

select interval '1 month' year to month

but not:

select interval '1' year to month
select interval year to month '1'

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#20Roy Badami
roy@gnomon.org.uk
In reply to: Bruce Momjian (#19)
Re: BUG #1517: SQL interval syntax is accepted by the parser,

Bruce> I guess my point is that we should allow:

Bruce> select interval '1' day '1' hour

Bruce> as SQL standard and equavalent to:

Ah, I think you're misunderstanding what the SQL standard interval
literal syntax looks like.

It would be

INTERVAL '1 1' DAY TO HOUR

Essentially the full syntax for a day-time interval is

INTERVAL '1 2:03:04' DAY TO SECOND

and the full syntax of a year-month interval is

INTERVA: '1-2' YEAR TO MONTH

but if you use a more restricted interval type you omit the fields
that aren't present in your interval type.

#21Roy Badami
roy@gnomon.org.uk
In reply to: Roy Badami (#20)
#22Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#14)
#23Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#19)
#24Tom Lane
tgl@sss.pgh.pa.us
In reply to: Roy Badami (#21)
#25Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#13)
#26Roy Badami
roy@gnomon.org.uk
In reply to: Bruce Momjian (#25)
#27Bruce Momjian
bruce@momjian.us
In reply to: Roy Badami (#26)
#28Roy Badami
roy@gnomon.org.uk
In reply to: Bruce Momjian (#27)
#29Bruce Momjian
bruce@momjian.us
In reply to: Roy Badami (#28)
#30Roy Badami
roy@gnomon.org.uk
In reply to: Bruce Momjian (#29)
#31Bruce Momjian
bruce@momjian.us
In reply to: Roy Badami (#30)
#32Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#29)
#33Roy Badami
roy@gnomon.org.uk
In reply to: Tom Lane (#32)
#34Roy Badami
roy@gnomon.org.uk
In reply to: Roy Badami (#33)
#35Roy Badami
roy@gnomon.org.uk
In reply to: Roy Badami (#34)
#36Roy Badami
roy@gnomon.org.uk
In reply to: Tom Lane (#32)
#37Tom Lane
tgl@sss.pgh.pa.us
In reply to: Roy Badami (#34)
#38Tom Lane
tgl@sss.pgh.pa.us
In reply to: Roy Badami (#36)
#39Roy Badami
roy@gnomon.org.uk
In reply to: Tom Lane (#37)
#40Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#38)
#41Tom Lane
tgl@sss.pgh.pa.us
In reply to: Roy Badami (#39)
#42Roy Badami
roy@gnomon.org.uk
In reply to: Tom Lane (#41)
#43Roy Badami
roy@gnomon.org.uk
In reply to: Tom Lane (#41)
#44Tom Lane
tgl@sss.pgh.pa.us
In reply to: Roy Badami (#42)
#45Tom Lane
tgl@sss.pgh.pa.us
In reply to: Roy Badami (#43)
#46Roy Badami
roy@gnomon.org.uk
In reply to: Tom Lane (#45)
#47Ron Mayer
rm_pg@cheapcomplexdevices.com
In reply to: Bruce Momjian (#29)
#48Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#38)
#49Bruce Momjian
bruce@momjian.us
In reply to: Roy Badami (#34)
#50Roy Badami
roy@gnomon.org.uk
In reply to: Bruce Momjian (#49)
#51Tom Lane
tgl@sss.pgh.pa.us
In reply to: Roy Badami (#50)
#52Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#51)
#53Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#52)