[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: A possible collation issue for perl-5.12-transition.

user debian-perl@lists.debian.org
usertags 615882 - perl-5.12-transition
tag 615882 patch

On Fri, Apr 15, 2011 at 11:58:00PM +0200, gregor herrmann wrote:
> On Fri, 15 Apr 2011 20:18:47 +0200, Mats Erik Andersson wrote:

> > there is a perl-5.12-transition issue filed [1] against
> > libcache-historical-perl, 

> I guess this is #615882, cc'ing he bug (and keeping the full quote).

> > The failed test, a single test out of nineteen, concerns
> > a list returned based on content in an SQLite3 database.
> > The list should nominally be implicitly sorted by date,
> > stored in fields not reproduced by the printout. However,
> > this is the important point, the delivered strings differs
> > from the expected string by a single cyclic permutation,
> > since the presented/stored values are really value pairs:
> > an integer and a measurement.

It's a real bug in the module, only spotted by the test suite in rare
cases of suitable timing. See the description of the attached proposed

The perl version doesn't really affect it, it just happened to trigger in
Dominic's test rebuild. I was able to reproduce the failure on 5.10.1,
and it mostly works for me on 5.12.3. Also, the CPAN test reports show
it happening every now and then for others as well.
Niko Tyni   ntyni@debian.org
>From 0db8d46e47239773409760d65c53d55434df1aad Mon Sep 17 00:00:00 2001
From: Niko Tyni <ntyni@debian.org>
Date: Mon, 18 Apr 2011 21:06:04 +0300
Subject: [PATCH] Make values() really sort as documented

The values() method used to sort the returned list by the update time
(newest first), contrary to the documentation.

This was mostly missed by the test suite, except on rare occasions,
because the time stamps don't use sub-second precision. Only when the
threshold between full seconds happened in the middle of the data
insertion statements, the values() test in t/001Basic.t failed.

Fix the method, change the data insertion statement order, and add a
sleep statement in the middle to make sure the bug stays fixed.
 Historical.pm |    2 +-
 t/001Basic.t  |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Historical.pm b/Historical.pm
index e080642..b88b06a 100644
--- a/Historical.pm
+++ b/Historical.pm
@@ -178,7 +178,7 @@ sub values {
     my $values = Cache::Historical::Val::Manager->get_vals(
         query => [ @key ],
-        sort_by => ['upd_time DESC'],
+        sort_by => ['date'],
     for(@$values) {
diff --git a/t/001Basic.t b/t/001Basic.t
index 67fac46..fee8145 100755
--- a/t/001Basic.t
+++ b/t/001Basic.t
@@ -17,8 +17,9 @@ my $c = Cache::Historical->new(
 my $fmt = DateTime::Format::Strptime->new(
               pattern => "%Y-%m-%d");
-$c->set( $fmt->parse_datetime("2008-01-02"), "msft", 35.22 );
 $c->set( $fmt->parse_datetime("2008-01-03"), "msft", 35.37 );
+$c->set( $fmt->parse_datetime("2008-01-02"), "msft", 35.22 );
+sleep 1; # so the update time is never the same for all the rows
 $c->set( $fmt->parse_datetime("2008-01-04"), "msft", 34.38 );
 $c->set( $fmt->parse_datetime("2008-01-07"), "msft", 34.61 );

Reply to: