Since nobody has commented on this patch does that mean everybody agrees that it's a good idea? :)
-J
--
On Mon, Jul 25, 2005 at 04:10:53PM -1000, Joshua Hoblitt wrote:
> Hi Folks,
>
> I stumbled across a limited precision issue while working on
> DateTime::Format::DateParse.
>
> One of 'epoch' test values used in Date::Parse is 1045469647.198189,
> which is just beyond the precision of an IEEE 754 64bit value (a C
> double). So internally (on all IEEE 754 compliant platforms, YMMV) this
> gets represent as 1045469647.198189020... . Since
> DateTime->from_epoch() treats the epoch parameter as numeric, it
> currently creates an object from that number set to 198_189_020
> nanoseconds. I believe that this is surprising behavior for most users;
> in general people don't think about using an epsilon when comparing
> integer values.
>
> Possible solutions:
>
> a) do nothing... nobody else seems to have noticed
> b) document the limited precision issue
> c) change the API to some awful Fortranish a part + b part to preserve
> precision
> d) turn the epoch parameter into a Math::BigFloat so a high resolution
> 'string' can be passed in and document the behavior.
>
> It's not clear to me what the right solution is although a patch to
> implement option d sans the required documentation changes is attached.
>
> Cheers,
>
> -J
>
> --
> ? .swp
> ? DateTime-0.2901-from_epoch_precision-fix.patch
> ? DateTime.bs
> ? DateTime.c
> ? Makefile
> ? blib
> ? leap_seconds.h
> ? pm_to_blib
> ? tmp.pl
> ? t/zz_00load.t
> ? t/zz_01sanity.t
> ? t/zz_02last_day.t
> ? t/zz_03components.t
> ? t/zz_04epoch.t
> ? t/zz_05set.t
> ? t/zz_06add.t
> ? t/zz_07compare.t
> ? t/zz_09greg.t
> ? t/zz_10subtract.t
> ? t/zz_11duration.t
> ? t/zz_12week.t
> ? t/zz_13strftime.t
> ? t/zz_14locale.t
> ? t/zz_15jd.t
> ? t/zz_16truncate.t
> ? t/zz_17set_return.t
> ? t/zz_18today.t
> ? t/zz_19leap_second.t
> ? t/zz_20infinite.t
> ? t/zz_21bad_params.t
> ? t/zz_22from_doy.t
> ? t/zz_23storable.t
> ? t/zz_24from_object.t
> ? t/zz_25add_subtract.t
> ? t/zz_27delta.t
> ? t/zz_28dow.t
> ? t/zz_29overload.t
> ? t/zz_30future_tz.t
> ? t/zz_31formatter.t
> ? t/zz_32leap_second2.t
> ? t/zz_33seconds_offset.t
> ? t/zz_34set_tz.t
> ? t/zz_35rd_values.t
> ? t/zz_36invalid_local.t
> Index: Makefile.PL
> ===================================================================
> RCS file: /cvsroot/perl-date-time/modules/DateTime.pm/Makefile.PL,v
> retrieving revision 1.47
> diff -u -r1.47 Makefile.PL
> --- Makefile.PL 27 Feb 2005 03:27:25 -0000 1.47
> +++ Makefile.PL 26 Jul 2005 01:41:35 -0000
> @@ -83,6 +83,7 @@
>
> PREREQ_PM => { 'DateTime::Locale' => 0.21,
> 'DateTime::TimeZone' => 0.26,
> + 'Math::BigFloat' => 0,
> 'Params::Validate' => 0.76,
> 'Pod::Man' => 1.14,
> 'Test::More' => 0.34,
> Index: lib/DateTime.pm
> ===================================================================
> RCS file: /cvsroot/perl-date-time/modules/DateTime.pm/lib/DateTime.pm,v
> retrieving revision 1.304
> diff -u -r1.304 DateTime.pm
> --- lib/DateTime.pm 7 Jul 2005 23:18:21 -0000 1.304
> +++ lib/DateTime.pm 26 Jul 2005 01:41:38 -0000
> @@ -48,6 +48,7 @@
> use DateTime::TimeZone;
> use Params::Validate qw( validate validate_pos SCALAR BOOLEAN HASHREF OBJECT
> );
> use Time::Local ();
> +use Math::BigFloat;
>
> # for some reason, overloading doesn't work unless fallback is listed
> # early.
> @@ -435,8 +436,8 @@
> my %args;
>
> # Because epoch may come from Time::HiRes
> - my $fraction = $p{epoch} - int( $p{epoch} );
> - $args{nanosecond} = int( $fraction * MAX_NANOSECONDS )
> + my $fraction = Math::BigFloat->new( $p{epoch} ) - int( $p{epoch} );
> + $args{nanosecond} = int ( $fraction * MAX_NANOSECONDS )->as_int->bstr
> if $fraction;
>
> # Note, for very large negative values this may give a blatantly
> Index: t/04epoch.t
> ===================================================================
> RCS file: /cvsroot/perl-date-time/modules/DateTime.pm/t/04epoch.t,v
> retrieving revision 1.18
> diff -u -r1.18 04epoch.t
> --- t/04epoch.t 13 Nov 2004 21:22:36 -0000 1.18
> +++ t/04epoch.t 26 Jul 2005 01:41:38 -0000
> @@ -2,7 +2,7 @@
>
> use strict;
>
> -use Test::More tests => 32;
> +use Test::More tests => 34;
>
> use DateTime;
>
> @@ -126,3 +126,13 @@
> my $dt = DateTime->from_epoch( epoch => 0.1234567891 );
> is( $dt->nanosecond, 123_456_789, 'nanosecond should be an integer ' );
> }
> +
> +{
> + my $dt = DateTime->from_epoch( epoch => "123456789.1234567891" );
> + is( $dt->nanosecond, 123_456_789, 'nanosecond should be an integer ' );
> +}
> +
> +{
> + my $dt = DateTime->from_epoch( epoch => "1045469647.198189" );
> + is( $dt->nanosecond, 198_189_000, 'nanosecond should be an integer ' );
> +}
pgphQzDt5zeaD.pgp
Description: PGP signature
