Closed
Bug 301141
Opened 19 years ago
Closed 19 years ago
[PostgreSQL] New charts throw an error about FROM_UNIXTIME
Categories
(Bugzilla :: Database, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.22
People
(Reporter: LpSolit, Assigned: LpSolit)
References
Details
Attachments
(1 file, 2 obsolete files)
2.45 KB,
patch
|
mkanat
:
review+
|
Details | Diff | Splinter Review |
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•19 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•19 years ago
|
Assignee: database → mkanat
Comment 2•19 years ago
|
||
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•19 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•19 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•19 years ago
|
No longer blocks: bz-postgres
Assignee | ||
Comment 5•19 years ago
|
||
Assignee: mkanat → LpSolit
Attachment #191352 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #201871 -
Flags: review?(mkanat)
Assignee | ||
Comment 6•19 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•19 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•19 years ago
|
Target Milestone: --- → Bugzilla 2.22
Comment 8•19 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•19 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•19 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•19 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•19 years ago
|
||
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)
Comment 13•19 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•19 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•19 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•19 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•19 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•19 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•19 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•19 years ago
|
||
*** Bug 302643 has been marked as a duplicate of this bug. ***
Comment 21•19 years ago
|
||
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 | ||
Comment 22•19 years ago
|
||
The blocker has a+ on it, this one can land too.
Whiteboard: [hold approval until blocker is checked-in]
Updated•19 years ago
|
Flags: approval? → approval+
Assignee | ||
Comment 23•19 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
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•