Обсуждение: Small addition to PGInterval

Hi all,

please find attached a small extension to PGInterval:
- getTimeInMillis   - returns the number of milliseconds in the interval
- copying constructor PGInterval(PGInterval) for type-safe, cast-free
cloning
- now implements Comaparable<PGInterval>

--
Hartmut Benz,
TI-WMC, http://www.ti-wmc.nl

On Thu, 29 Mar 2007, Hartmut Benz wrote:

> please find attached a small extension to PGInterval:
> - getTimeInMillis   - returns the number of milliseconds in the interval

getLengthInMillis doesn't account for months.

ONE_YEAR is seconds in a year instead of milliseconds.

Shouldn't we use the server's definition instead of google's for a year's
length in seconds:
select extract(epoch from '1 year'::interval);
  date_part
-----------
   31557600
(1 row)


> - copying constructor PGInterval(PGInterval) for type-safe, cast-free cloning
> - now implements Comaparable<PGInterval>

We can't use generics in this class because it must be buildable with
older JDK versions.

Also context diffs (-c) are preferred.

Kris Jurka

Thanks for pointing this out. I corrected the bug (months) and use the
server definition for both year and month. It is a bit problematic to
determine the 'correct' number of seconds in these kind of intervals
because there are so many of them :-)

Hartmut Benz


Kris Jurka wrote:
>
>
> On Thu, 29 Mar 2007, Hartmut Benz wrote:
>
>> please find attached a small extension to PGInterval:
>> - getTimeInMillis   - returns the number of milliseconds in the interval
>
> getLengthInMillis doesn't account for months.
>
> ONE_YEAR is seconds in a year instead of milliseconds.
>
> Shouldn't we use the server's definition instead of google's for a
> year's length in seconds:
> select extract(epoch from '1 year'::interval);
>  date_part
> -----------
>   31557600
> (1 row)
>
>
>> - copying constructor PGInterval(PGInterval) for type-safe, cast-free
>> cloning
>> - now implements Comaparable<PGInterval>
>
> We can't use generics in this class because it must be buildable with
> older JDK versions.
>
> Also context diffs (-c) are preferred.
>
> Kris Jurka
>

On Wed, 11 Apr 2007, Hartmut Benz wrote:

> Thanks for pointing this out. I corrected the bug (months) and use the server
> definition for both year and month. It is a bit problematic to determine the
> 'correct' number of seconds in these kind of intervals because there are so
> many of them :-)

Does getLengthInMillis have any external value or is it solely useful for
implementing compareTo?  I fear that people will try to do something like:

Date d = new Date();
PGInterval i = new PGInterval("1 year");
d.setTime(d.getTime() + i.getLengthInMillis());

instead of using the exising i.add(d) method.  As you mention it's not
entirely clear what value it should be returning, so I'd prefer not to
expose it unless you have a more useful use case.

Kris Jurka

In my application I have a value stored as interval in the DB that I use
in my application for various checks against the system time
(System.currentTimeMillis()) and other times. My alternatives were:
- store the interval as ms in the DB making it less readable
- implement a conversion routine myself converting from PGInterval
values to ms in my code
- perform this conversion in PGInterval itself

I found the third to be the most attractive. It puts the code to the
data it belongs to. The basis "ms" is the basic 'coinage' most program
work with. It exposes (implicitly), what postgres thinks how long a year is.

Independent of which number of ms per year/month is the correct one, I
think it is more important to have identical values in the postgres
server and the client. Therefore, your suggestion to use the server
value instead of some other value was the right one.

By the way, I do not see such a problem with your code example. Wherever
I see programs working with timers I see similar constructions. See, for
instance, javax.management.timer.Timer: it defines millisecond-based
constants for second, minute, hour, day, and week to be used very
similar to your code fragment.

The biggest problem I see is that the class now takes a position
regarding how many ms are in an 'average' month and year. But postgres
does that already in the server.


Hartmut Benz


Kris Jurka wrote:
>
>
> On Wed, 11 Apr 2007, Hartmut Benz wrote:
>
>> Thanks for pointing this out. I corrected the bug (months) and use
>> the server definition for both year and month. It is a bit
>> problematic to determine the 'correct' number of seconds in these
>> kind of intervals because there are so many of them :-)
>
> Does getLengthInMillis have any external value or is it solely useful
> for implementing compareTo?  I fear that people will try to do
> something like:
>
> Date d = new Date();
> PGInterval i = new PGInterval("1 year");
> d.setTime(d.getTime() + i.getLengthInMillis());
>
> instead of using the exising i.add(d) method.  As you mention it's not
> entirely clear what value it should be returning, so I'd prefer not to
> expose it unless you have a more useful use case.
>
> Kris Jurka
>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: In versions below 8.0, the planner will ignore your desire to
>       choose an index scan if your joining column's datatypes do not
>       match
>

On Wed, 11 Apr 2007, Hartmut Benz wrote:

> In my application I have a value stored as interval in the DB that I use in
> my application for various checks against the system time
> (System.currentTimeMillis()) and other times. My alternatives were:
> - store the interval as ms in the DB making it less readable
> - implement a conversion routine myself converting from PGInterval values to
> ms in my code
> - perform this conversion in PGInterval itself

You've left out the option of using existing functionality that handles
months and years (more) correctly.

Date d = new Date(System.currentTimeMillis());
PGInterval i = getIntervalFromSomewhere();
i.add(d);
long millis = d.getTime();

> I found the third to be the most attractive. It puts the code to the data it
> belongs to. The basis "ms" is the basic 'coinage' most program work with. It
> exposes (implicitly), what postgres thinks how long a year is.
>

Do you have intervals that have month/year components?  If you don't then
it doesn't matter, but if we're advertising it as a general purpose
function then I think we need to handle it as accurately as possible.

> By the way, I do not see such a problem with your code example. Wherever I
> see programs working with timers I see similar constructions. See, for
> instance, javax.management.timer.Timer: it defines millisecond-based
> constants for second, minute, hour, day, and week to be used very similar to
> your code fragment.

Yes, but it doesn't define month/year where the problems are so it doesn't
seem relevent.

Kris Jurka