[PostgreSQL] New charts throw an error about FROM_UNIXTIME

RESOLVED FIXED in Bugzilla 2.22

Status

()

Bugzilla
Database
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: Frédéric Buclin, Assigned: Frédéric Buclin)

Tracking

2.21
Bugzilla 2.22
Dependency tree / graph
Bug Flags:
approval +

Details

Attachments

(1 attachment, 2 obsolete attachments)

2.45 KB, patch
Max Kanat-Alexander
: review+
Details | Diff | Splinter Review
(Assignee)

Description

12 years ago
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

12 years ago
Yeah, I think we can just use sql_date_format or format_time to fix that.
Summary: New charts do not work with PostgreSQL → [PostgreSQL] New charts throw an error about FROM_UNIXTIME

Updated

12 years ago
Assignee: database → mkanat

Comment 2

12 years ago
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

12 years ago
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.
Attachment #191352 - Flags: review-

Comment 4

12 years ago
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 ))

Updated

12 years ago
No longer blocks: 98304
(Assignee)

Updated

12 years ago
Blocks: 314490
(Assignee)

Comment 5

12 years ago
Created attachment 201871 [details] [diff] [review]
patch, v1
Assignee: mkanat → LpSolit
Attachment #191352 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #201871 - Flags: review?(mkanat)
(Assignee)

Comment 6

12 years ago
Comment on attachment 201871 [details] [diff] [review]
patch, v1

karl, could you test this patch? It's about new charts.
Attachment #201871 - Flags: review?(karl)

Comment 7

12 years ago
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.
Attachment #201871 - Flags: review?(karl) → review+
(Assignee)

Updated

12 years ago
Target Milestone: --- → Bugzilla 2.22

Comment 8

12 years ago
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.
Attachment #201871 - Flags: review?(mkanat) → review-
(Assignee)

Comment 9

12 years ago
(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

12 years ago
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.
(Assignee)

Comment 11

12 years ago
(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.
(Assignee)

Comment 12

12 years ago
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.
Attachment #201871 - Attachment is obsolete: true
Attachment #202438 - Flags: review?(mkanat)
(Assignee)

Updated

12 years ago
Depends on: 302599

Comment 13

12 years ago
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.
Attachment #202438 - Flags: review?(mkanat) → review+

Comment 14

12 years ago
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.
No longer depends on: 302599
(Assignee)

Comment 15

12 years ago
(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?
Flags: approval?

Comment 16

12 years ago
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.
Whiteboard: [hold approval until blocker is checked-in]
(Assignee)

Comment 17

12 years ago
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

12 years ago
(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.
(Assignee)

Comment 19

12 years ago
(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

12 years ago
*** Bug 302643 has been marked as a duplicate of this bug. ***
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...
(Assignee)

Updated

12 years ago
Depends on: 302599
(Assignee)

Comment 22

12 years ago
The blocker has a+ on it, this one can land too.
Whiteboard: [hold approval until blocker is checked-in]
Flags: approval? → approval+
(Assignee)

Comment 23

12 years ago
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
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.