Re: Issue in Behavior of Interval Datatype

Lists: pgsql-hackers
From: Amit Kapila <amit(dot)kapila(at)huawei(dot)com>
To: "'PG Hackers'" <pgsql-hackers(at)postgresql(dot)org>
Subject: Issue in Behavior of Interval Datatype
Date: 2012-08-02 13:08:31
Message-ID: 003201cd70af$eaf058a0$c0d109e0$@kapila@huawei.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

select (interval '56:48' minute to second);
result:00:56:48
select (interval '-56:48' minute to second);
result:-56:48:00
select (interval '+56:48' minute to second);
result:56:48:00

When user uses ‘+’ or ‘-‘ symbol, then minute to second range is getting
ignored.

I have checked the code and found that in function DecodeInterval(), for
timezone case (DTK_TZ) it uses INTERVAL_FULL_RANGE irrespective of range
passed by user.

However if use the range passed as argument in function DecodeInterval(),
the result of using ‘+’ or ‘-‘ is same as without using it.

Is there any particular reason for ignoring the range for DTK_TZ case in
DecodeInterval() function?

With Regards,

Amit Kapila


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Amit Kapila <amit(dot)kapila(at)huawei(dot)com>
Cc: "'PG Hackers'" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Issue in Behavior of Interval Datatype
Date: 2012-08-03 20:18:03
Message-ID: 10515.1344025083@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Amit Kapila <amit(dot)kapila(at)huawei(dot)com> writes:
> select (interval '56:48' minute to second);
> result$B!'(B00:56:48
> select (interval '-56:48' minute to second);
> result$B!'(B-56:48:00
> select (interval '+56:48' minute to second);
> result$B!'(B56:48:00

> I have checked the code and found that in function DecodeInterval(), for
> timezone case (DTK_TZ) it uses INTERVAL_FULL_RANGE irrespective of range
> passed by user.

> However if use the range passed as argument in function DecodeInterval(),
> the result of using $B!F(B+$B!G(B or $B!F(B-$B!F(B is same as without using it.

> Is there any particular reason for ignoring the range for DTK_TZ case in
> DecodeInterval() function?

I think you are right; this if-block should be exactly like the DTK_TIME
case except for handling the prepended sign. That also raises the
question why it is changing the tmask value returned by DecodeTime.
It seems to be doing exactly the wrong thing there. Test case:

regression=# select (interval '56:48 56:48' );
ERROR: invalid input syntax for type interval: "56:48 56:48"
LINE 1: select (interval '56:48 56:48' );
^
regression=# select (interval '56:48 +56:48' );
interval
----------
56:48:00
(1 row)

The second one fails to fail because an inappropriate tmask value got
included into fmask.

Will fix.

regards, tom lane


From: Amit Kapila <amit(dot)kapila(at)huawei(dot)com>
To: "'Tom Lane'" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "'PG Hackers'" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Issue in Behavior of Interval Datatype
Date: 2012-08-04 06:18:33
Message-ID: 001f01cd7208$fa440900$eecc1b00$@kapila@huawei.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

From: Tom Lane [mailto:tgl(at)sss(dot)pgh(dot)pa(dot)us]
Sent: Saturday, August 04, 2012 1:48 AM
Amit Kapila <amit(dot)kapila(at)huawei(dot)com> writes:
> select (interval '56:48' minute to second);
> result$B!'(B00:56:48
> select (interval '-56:48' minute to second);
> result$B!'(B-56:48:00
> select (interval '+56:48' minute to second);
> result$B!'(B56:48:00

> I have checked the code and found that in function DecodeInterval(), for
> timezone case (DTK_TZ) it uses INTERVAL_FULL_RANGE irrespective of range
> passed by user.

> However if use the range passed as argument in function DecodeInterval(),
> the result of using $B!F(B+$B!G(B or $B!F(B-$B!F(B is same as
without using it.

> Is there any particular reason for ignoring the range for DTK_TZ case in
> DecodeInterval() function?

> I think you are right; this if-block should be exactly like the DTK_TIME
> case except for handling the prepended sign. That also raises the
> question why it is changing the tmask value returned by DecodeTime.
> It seems to be doing exactly the wrong thing there. Test case:

> regression=# select (interval '56:48 56:48' );
> ERROR: invalid input syntax for type interval: "56:48 56:48"
> LINE 1: select (interval '56:48 56:48' );
^
> regression=# select (interval '56:48 +56:48' );
> interval
> ----------
> 56:48:00
> (1 row)

> The second one fails to fail because an inappropriate tmask value got
>included into fmask.

Yes, this is right that tmask need not be changed, also one more thing I
have noticed that
in file Interval.c also there is a function DecodeInterval() which is
currently little different
from DecodeInterval() in datetime.c for DTK_TZ case.
For example Assert check is commented.
Why the Assert check is commented out there?
May be due to some defect, but I am not able to think the reason for same.

Shouldn't we make this change (don't change tmask in case of DTK_TZ) in
Interval.c?

>Will fix.

Thank you very much.

With Regards,
Amit Kapila.


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Amit Kapila <amit(dot)kapila(at)huawei(dot)com>
Cc: "'PG Hackers'" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Issue in Behavior of Interval Datatype
Date: 2012-08-04 16:11:56
Message-ID: 4694.1344096716@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Amit Kapila <amit(dot)kapila(at)huawei(dot)com> writes:
> Yes, this is right that tmask need not be changed, also one more thing I
> have noticed that
> in file Interval.c also there is a function DecodeInterval() which is
> currently little different
> from DecodeInterval() in datetime.c for DTK_TZ case.
> For example Assert check is commented.
> Why the Assert check is commented out there?

Assert doesn't work in client-side code.

More generally, nobody is maintaining ecpg's copy of the datetime code,
and that's been true for a very long time. I'm not personally
interested in trying to re-sync that copy. It would be more useful to
figure out a way to get rid of it. There have been past discussions of
how we could make a single copy of the code work in both backend and
frontend contexts, but the frontend environment is so impoverished by
comparison (no Assert, no elog, no palloc) that it hasn't looked like an
attractive idea. It's also fairly unclear whether anyone is actually
using ecpg's client-side datetime support, which means there's little
motivation to put a lot of work into it.

regards, tom lane


From: Michael Meskes <meskes(at)postgresql(dot)org>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Amit Kapila <amit(dot)kapila(at)huawei(dot)com>, 'PG Hackers' <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Issue in Behavior of Interval Datatype
Date: 2012-08-11 09:59:02
Message-ID: 20120811095902.GA14079@feivel.credativ.lan
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Sat, Aug 04, 2012 at 12:11:56PM -0400, Tom Lane wrote:
> More generally, nobody is maintaining ecpg's copy of the datetime code,
> and that's been true for a very long time. I'm not personally

Well, maintaining as in fixing bugs I do as I find time. Maintaining as in
keeping in sync with the backend nobody does, you're right. However, as long as
the code works and the internal data representation isn't changed or used,
that's not really a problem.

> interested in trying to re-sync that copy. It would be more useful to
> figure out a way to get rid of it. There have been past discussions of

+1

> how we could make a single copy of the code work in both backend and
> frontend contexts, but the frontend environment is so impoverished by
> comparison (no Assert, no elog, no palloc) that it hasn't looked like an
> attractive idea. It's also fairly unclear whether anyone is actually
> using ecpg's client-side datetime support, which means there's little
> motivation to put a lot of work into it.

This is an interesting question. I used to think that even ecpg is not used
very often anymore but I keep running into large scale usages from time to
time. And at least some do use pgtypeslib, too.

Michael

--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at googlemail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL