Last Comment Bug 559539 - [Oracle] whine.pl sets run_next incorrectly due to CURRENT_DATE
: [Oracle] whine.pl sets run_next incorrectly due to CURRENT_DATE
Status: RESOLVED FIXED
:
Product: Bugzilla
Classification: Server Software
Component: Database (show other bugs)
: 3.6
: All All
: -- minor (vote)
: Bugzilla 4.2
Assigned To: David Taylor
: default-qa
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-04-15 04:16 PDT by David Taylor
Modified: 2012-08-25 18:22 PDT (History)
3 users (show)
LpSolit: approval+
LpSolit: approval4.2+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
wrap CURRENT_DATE in TRUNC() (2.14 KB, patch)
2010-04-15 04:16 PDT, David Taylor
LpSolit: review-
Details | Diff | Review
wrap CURRENT_DATE in TRUNC() for Oracle only (1.18 KB, patch)
2010-04-20 05:51 PDT, David Taylor
LpSolit: review+
Details | Diff | Review
patch for 4.4, v1 (743 bytes, patch)
2012-08-25 18:03 PDT, Frédéric Buclin
no flags Details | Diff | Review
patch for 4.4, v1.1 (831 bytes, patch)
2012-08-25 18:08 PDT, Frédéric Buclin
LpSolit: review+
Details | Diff | Review

Description David Taylor 2010-04-15 04:16:13 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3
Build Identifier: Bugzilla 3.6.0

whine.pl assumes that CURRENT_DATE returns, well, the current date.  Oracle's DATE datatype is actually a date+time.

As a result when whine.pl runs at 2010-04-15 08:04:30 and performs the calculation (pseudo-SQL) 'CURRENT_DATE + 11 hours', rather than getting the answer 2010-04-15 11:00:00, it instead obtains 2010-04-15 19:04:30.

As a result whines run at completely the wrong time.  The simple fix is to use TRUNC(CURRENT_DATE) to ensure the datetime is truncated to simply the date.

Reproducible: Always

Steps to Reproduce:
1. Install bugzilla using an Oracle DB
2. At 10 am, configure a whine to run at 13:00
3. Run whine.pl
Actual Results:  
run_next will be set to 10am + 13 hours = 23:00

Expected Results:  
run_next should be set to 13:00
Comment 1 David Taylor 2010-04-15 04:16:44 PDT
Created attachment 439207 [details] [diff] [review]
wrap CURRENT_DATE in TRUNC()
Comment 2 Frédéric Buclin 2010-04-15 04:40:02 PDT
Comment on attachment 439207 [details] [diff] [review]
wrap CURRENT_DATE in TRUNC()

TRUNC doesn't exist on MySQL. It must be an Oracle thing only, if required.
Comment 3 David Taylor 2010-04-20 05:51:26 PDT
Created attachment 440196 [details] [diff] [review]
wrap CURRENT_DATE in TRUNC() for Oracle only

Oops.  It seems Bugzilla/DB/Oracle.pm is already translating CURRENT_DATE() into CURRENT_DATE.  The attached patch will now translate them both into TRUNC(CURRENT_DATE).
Comment 4 Max Kanat-Alexander 2010-04-20 13:08:22 PDT
Hey David. Thanks for the patch! If you'd like to get it reviewed, see our development process here for details on how:

  http://wiki.mozilla.org/Bugzilla:Developers
Comment 5 Frédéric Buclin 2011-08-28 16:15:08 PDT
Comment on attachment 440196 [details] [diff] [review]
wrap CURRENT_DATE in TRUNC() for Oracle only

This code has been removed, see bug 675603.
Comment 6 David Taylor 2011-08-30 12:32:50 PDT
As noted in bug 675603 comment 11, CURRENT_DATE is still used in whine.pl.

The fix for that bug has had no effect on whine.pl and this bug remains.
Comment 7 Frédéric Buclin 2012-08-25 18:03:23 PDT
Created attachment 655383 [details] [diff] [review]
patch for 4.4, v1

Tested with current 4.3.2+ code.
Comment 8 Frédéric Buclin 2012-08-25 18:08:22 PDT
Created attachment 655384 [details] [diff] [review]
patch for 4.4, v1.1

Oops, there was an incorrect indentation in my previous patch. Actually, I'm going to r+ this patch myself, because it's mostly a port of David's patch to 4.4.
Comment 9 Frédéric Buclin 2012-08-25 18:17:19 PDT
Comment on attachment 440196 [details] [diff] [review]
wrap CURRENT_DATE in TRUNC() for Oracle only

>-    $part =~ s/\bCURRENT_DATE\b\(\)/CURRENT_DATE/io;
>+    $part =~ s/\bCURRENT_DATE(?:\(\)|\b)/TRUNC(CURRENT_DATE)/io;

Ok, this change is fine. I'm going to write \bCURRENT_DATE\b(?:\(\))? though, which seems cleaner to me. r=LpSolit for 4.2.3.
Comment 10 Frédéric Buclin 2012-08-25 18:22:56 PDT
Thanks for your patch, David! :)

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/DB/Oracle.pm
Committed revision 8357.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.2/
modified Bugzilla/DB/Oracle.pm
Committed revision 8122.

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