Last Comment Bug 301141 - [PostgreSQL] New charts throw an error about FROM_UNIXTIME
: [PostgreSQL] New charts throw an error about FROM_UNIXTIME
Status: RESOLVED FIXED
:
Product: Bugzilla
Classification: Server Software
Component: Database (show other bugs)
: 2.21
: All All
: -- normal (vote)
: Bugzilla 2.22
Assigned To: Frédéric Buclin
: default-qa
Mentors:
: 302643 (view as bug list)
Depends on: 302599
Blocks: meta-pg
  Show dependency treegraph
 
Reported: 2005-07-17 15:36 PDT by Frédéric Buclin
Modified: 2005-11-15 01:53 PST (History)
6 users (show)
justdave: approval+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
sql_from_epoch (3.85 KB, patch)
2005-08-02 09:20 PDT, Shin-ichi Nagamura
mkanat: review-
Details | Diff | Splinter Review
patch, v1 (3.76 KB, patch)
2005-11-04 12:26 PST, Frédéric Buclin
mkanat: review-
karl: review+
Details | Diff | Splinter Review
patch, v2 (2.45 KB, patch)
2005-11-09 16:23 PST, Frédéric Buclin
mkanat: review+
Details | Diff | Splinter Review

Description Frédéric Buclin 2005-07-17 15:36:19 PDT
Create a CSV file using new charts on Pg:

      undef error - DBD::Pg::st execute failed: ERROR:  function
from_unixtime(integer) does not exist [for Statement "SELECT
TO_CHAR(series_date, 'J')::int - TO_CHAR(FROM_UNIXTIME(961138800), 'J')::int,
series_value FROM series_data WHERE series_id = ? AND series_date >=
FROM_UNIXTIME(961138800) AND series_date <= FROM_UNIXTIME(1120287600)"]
at Bugzilla/Chart.pm line 259
	Bugzilla::Chart::readData('Bugzilla::Chart=HASH(0x9ae44b0)') called at
Bugzilla/Chart.pm line 200
	Bugzilla::Chart::data('Bugzilla::Chart=HASH(0x9ae44b0)') called at
data/template/template/en/default/reports/chart.csv.tmpl line 19
	eval {...} called at data/template/template/en/default/reports/chart.csv.tmpl
line 19
	eval {...} called at data/template/template/en/default/reports/chart.csv.tmpl
line 16
	Template::Provider::__ANON__('Template::Context=HASH(0x9aadda0)') called at
/usr/lib/perl5/site_perl/5.8.5/i386-linux-thread-multi/Template/Document.pm line 144
	eval {...} called at
/usr/lib/perl5/site_perl/5.8.5/i386-linux-thread-multi/Template/Document.pm line 142
	Template::Document::process('Template::Document=HASH(0x9bc7854)',
'Template::Context=HASH(0x9aadda0)') called at
/usr/lib/perl5/site_perl/5.8.5/i386-linux-thread-multi/Template/Context.pm line 315
	eval {...} called at
/usr/lib/perl5/site_perl/5.8.5/i386-linux-thread-multi/Template/Context.pm line 295
	Template::Context::process('Template::Context=HASH(0x9aadda0)',
'Template::Document=HASH(0x9bc7854)') called at
/usr/lib/perl5/site_perl/5.8.5/i386-linux-thread-multi/Template/Service.pm line 90
	eval {...} called at
/usr/lib/perl5/site_perl/5.8.5/i386-linux-thread-multi/Template/Service.pm line 88
	Template::Service::process('Template::Service=HASH(0x9a84b90)',
'reports/chart.csv.tmpl', 'HASH(0x9a0f1cc)') called at
/usr/lib/perl5/site_perl/5.8.5/i386-linux-thread-multi/Template.pm line 59
	Template::process('Bugzilla::Template=HASH(0x9a42ff0)',
'reports/chart.csv.tmpl', 'HASH(0x9a0f1cc)') called at
/var/www/html/bugzilla-tip-pg/chart.cgi line 276
	main::plot() called at /var/www/html/bugzilla-tip-pg/chart.cgi line 119


I can reproduce the problem on both my local installation using PostgreSQL 8.0.1
and on landfill (Pg 7.x IIRC). WFM using MySQL 4.1.
Comment 1 Max Kanat-Alexander 2005-07-17 20:03:34 PDT
Yeah, I think we can just use sql_date_format or format_time to fix that.
Comment 2 Shin-ichi Nagamura 2005-08-02 09:20:25 PDT
Created attachment 191352 [details] [diff] [review]
sql_from_epoch

This bug is because postgresql doesn't understand FROM_UNIXTIME() function.
Then I make a abstract method for same function.

This method given a time from epoch, then return a time format string.
Comment 3 Max Kanat-Alexander 2005-08-02 10:01:46 PDT
Comment on attachment 191352 [details] [diff] [review]
sql_from_epoch

I think this code can be restructured to not need sql_from_epoch. I don't
really want to create another sql_ function when it's only going to be used
here.
Comment 4 Shin-ichi Nagamura 2005-08-02 11:17:32 PDT
OK, but I don't know same function for unix epoch time in mysql and postgresql.
I have a idea and it is use strftime() in perl code. 

For example.

use POSIX 'strftime' ;

#$dbh->sql_to_days("FROM_UNIXTIME($datefrom)") #original code
$dbh->sql_to_days( strftime( "%Y-%m-%d %H:%M:%S", $datefrom ))
Comment 5 Frédéric Buclin 2005-11-04 12:26:43 PST
Created attachment 201871 [details] [diff] [review]
patch, v1
Comment 6 Frédéric Buclin 2005-11-04 12:27:48 PST
Comment on attachment 201871 [details] [diff] [review]
patch, v1

karl, could you test this patch? It's about new charts.
Comment 7 A. Karl Kornel 2005-11-09 09:57:27 PST
Comment on attachment 201871 [details] [diff] [review]
patch, v1

(In reply to comment #6)
> karl, could you test this patch? It's about new charts.

Looks OK to me.
Comment 8 Max Kanat-Alexander 2005-11-09 13:59:58 PST
Comment on attachment 201871 [details] [diff] [review]
patch, v1

I really don't like that '::date' argument.

The times when it needs to be cast to a ::date is when you pass in a string-literal date instead of a field name.

So, instead of adding a parameter, just add "::date" whenever the argument is a string literal.
Comment 9 Frédéric Buclin 2005-11-09 14:21:27 PST
(In reply to comment #8)
> So, instead of adding a parameter, just add "::date" whenever the argument is a
> string literal.

Huh?? How do I do that???
Comment 10 Max Kanat-Alexander 2005-11-09 15:00:54 PST
A string literal will always be passed in like q{'1970-01-01'}. Thus, it will always have single-quotes around it. Check for the single-quotes at the edges of the string.
Comment 11 Frédéric Buclin 2005-11-09 16:10:23 PST
(In reply to comment #10)
> A string literal will always be passed in like q{'1970-01-01'}. Thus, it will
> always have single-quotes around it. Check for the single-quotes at the edges
> of the string.


Hum... this doesn't work, because $dbh->sql_to_days() gets a '?', not a string. So you have no idea what will be passed to the placeholder. Am I missing something?
Moreover, I don't think that quotes are part of the string.
Comment 12 Frédéric Buclin 2005-11-09 16:23:35 PST
Created attachment 202438 [details] [diff] [review]
patch, v2

I think this is definitely the right fix, i.e. cast the date in all cases. But we have to remove

AddFDef("(" . $dbh->sql_to_days('NOW()') . " - " .
              $dbh->sql_to_days('bugs.delta_ts') . ")",
        "Days since bug changed", 0);

from checksetup.pl, see bug 302599. This is the single place which causes error.
Comment 13 Max Kanat-Alexander 2005-11-10 12:36:07 PST
Comment on attachment 202438 [details] [diff] [review]
patch, v2

(In reply to comment #12)
> I think this is definitely the right fix, i.e. cast the date in all cases. But
> we have to remove

  We don't have to remove it, we just have to make the field longer. The error is that the contents are too long.

  But yes, otherwise I like this solution much better.
Comment 14 Max Kanat-Alexander 2005-11-10 12:39:12 PST
The listed bug is not actually the blocker. It's true, Dave has suggested that we switch to using a special "name" with a separate "definition" for all fielddefs, but we're not going to do that during the 2.22 freeze. (It would be a large change.)

What we need to do instead is enlarge the name varchar before we run the AddFDef functions, which can be done in this bug.
Comment 15 Frédéric Buclin 2005-11-10 12:54:30 PST
(In reply to comment #14)
> What we need to do instead is enlarge the name varchar before we run the
> AddFDef functions, which can be done in this bug.

This is another bug (which should block this one), and the fix you are suggesting is a workaround only. But if we haven't any better solution in the short term, let's do it. :( Suggesting 128 characters instead of 64?
Comment 16 Max Kanat-Alexander 2005-11-10 13:00:47 PST
You're right, it would be another bug. Yeah, 128 sounds fine to me.

You probably shouldn't request approval on this one until we've actually checked-in the other one.
Comment 17 Frédéric Buclin 2005-11-10 13:31:25 PST
Hum... we still have a problem. My DB already has the field in question and fixing $dbh->sql_to_days() introduce a second one:

bugs_pg=> select name, length(name) as longi from fielddefs order by longi desc;
                                   name                                    | longi
---------------------------------------------------------------------------+-------
 (TO_CHAR(NOW()::date, 'J')::int - TO_CHAR(bugs.delta_ts::date, 'J')::int) |    73
 (TO_CHAR(NOW(), 'J')::int - TO_CHAR(bugs.delta_ts, 'J')::int)             |    61
 attachments.description                                                   |    23

If we have to rename the old field, we could as well rename it correctly, i.e. a real name, not a SQL fragment.
Comment 18 Max Kanat-Alexander 2005-11-10 14:38:03 PST
(In reply to comment #17)
> Hum... we still have a problem. My DB already has the field in question and
> fixing $dbh->sql_to_days() introduce a second one:

  Ah, yeah, I knew about that. :-) I'd forgotten until you mentioned it just now, though. You're going to have to special-case this particular AddFDef, for now, and check for the exact old text if it's Pg. Fun, I know.
Comment 19 Frédéric Buclin 2005-11-10 14:41:53 PST
(In reply to comment #18)
>   Ah, yeah, I knew about that. :-) I'd forgotten until you mentioned it just
> now, though. You're going to have to special-case this particular AddFDef, for
> now, and check for the exact old text if it's Pg. Fun, I know.

I'm on it. Saved searches have to be fixed too. I will post my patch in bug 302599. If everything goes well, my patch should be ready in less than an hour.
Comment 20 Max Kanat-Alexander 2005-11-11 17:24:39 PST
*** Bug 302643 has been marked as a duplicate of this bug. ***
Comment 21 Dave Miller [:justdave] (justdave@bugzilla.org) 2005-11-11 18:22:33 PST
ok, the status whiteboard says "hold approval until blocker is checked-in" but the only thing in the dependency fields is a metabug that depends on this, doesn't block it.  What am I missing?  Is it bug 302599?  That's mentioned in passing, but nobody actually says it blocks this one anywhere...
Comment 22 Frédéric Buclin 2005-11-14 23:37:10 PST
The blocker has a+ on it, this one can land too.
Comment 23 Frédéric Buclin 2005-11-15 01:53:27 PST
Checking in Bugzilla/Chart.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Chart.pm,v  <--  Chart.pm
new revision: 1.9; previous revision: 1.8
done
Checking in Bugzilla/DB/Pg.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Pg.pm,v  <--  Pg.pm
new revision: 1.17; previous revision: 1.16
done

Note You need to log in before you can comment on or make changes to this bug.