558817 - [MSSQL] Change sql_interval to sql_date_math

Status: RESOLVED DUPLICATE of bug 602165

(Bugzilla :: Database, enhancement, P3)

Description

[Michael Thomas (Mockodin)]

I propose to add a third parameter to sql_interval, the date object we are effecting, eg

current method:
$dbh->prepare("UPDATE whine_schedules " .
              "SET run_next = CURRENT_DATE + " .
              $dbh->sql_interval('?', 'HOUR') .
              " WHERE id = ?");

proposed method:
$dbh->prepare("UPDATE whine_schedules " .
              "SET run_next = " .
              $dbh->sql_interval('CURRENT_DATE','?', 'HOUR') .
              " WHERE id = ?");

Why?:
MSSQL has no idea what to do with the INTERVAL ? DAY syntax, Oracle as well. So currently we are using regex to alter sql on the fly eg:
CURRENT_DATE + /* 60 DAY */

For mssql that means both the date reference 'CURRENT_DATE' and the place holder '/* 60 DAY */' get altered via regex.

By passing in the date/field reference we make the statement more database customizable without having to regex replace on the fly. The regex becomes increasing more complex when you begin chaining sql_interval calls

$dbh->prepare("UPDATE whine_schedules " .
              "SET run_next = (CURRENT_DATE + " .
              $dbh->sql_interval('?', 'DAY') . ") + " .
              $dbh->sql_interval('?', 'HOUR')
              " WHERE id = ?")

To address the issue of + or - modifiers in the interval I would propose that simply checking for a leading - would suffice. Some databases are probably fine always saying + with a following negative number. Point being that it could be handled easily on a per db basis. 

It doesn't look like it would take much to make this change.

the following files would be effected:
whine.pl                              (8 lines)
Bugzilla/Search.pm                    (1 line)
Bugzilla/Token.pm                     (2 lines)
Bugzilla/User.pm                      (1 line)
Bugzilla/Auth/Persist/Cookie.pm       (1 Line)
Bugzilla/contrib/sendunsentbugmail.pl (1 line)

Plus the associated functions themselves:
Bugzilla/DB/Mysql.pm
Bugzilla/DB/Oracle.pm
Bugzilla/DB/Pg.pm
Bugzilla/DB/Mssql.pm

Comment 1

[Michael Thomas (Mockodin)]

Should not, that Oracle is not using regex, so ignore that comment, the issue is specific to Mssql. However the concept I'm proposing still allows for flexibility for any database.

Comment 2

[Michael Thomas (Mockodin)]

A quick google search seems to indicate prefixing the value with a - is supported by MySQL, MSSQL, Oracle and Pg

Comment 3

[Max Kanat-Alexander]

Actually, you need to add two arguments--the other item you're doing math with, and the math operator you're using.

I don't want to have to do this just for MS-SQL, when INTERVAL is an ANSI-standard type that all our other databases support.

Comment 4

[Michael Thomas (Mockodin)]

(In reply to comment #3)
>I don't want to have to do this just for MS-SQL, when INTERVAL is an
>ANSI-standard type that all our other databases support.
So your saying you will not accept a patch if I generate one?

The sad thing really is that mssql can get you part of the way.

SELECT GETDATE() - 1   #eg 2010-01-02 12:52:00 - 1 = 2010-01-01 12:52:00

You can use decimals to make smaller changes, however its impossible to be accurate once you take into account time increments greater than an exact number of days eg INTERVAL 1 MONTH

> Actually, you need to add two arguments--the other item you're doing math with,
> and the math operator you're using.

I suppose, though as mentioned it should be perfectly fine doing the following in mysql for example:

SELECT NOW(), NOW() + INTERVAL -5 DAY
/*2010-04-13 12:59:20	2010-04-08 12:59:20 */

So the third would not truly be required, however given the basis of the request itself its probably better to include it as well.

Comment 5

[Max Kanat-Alexander]

Oh, oops, I filed a duplicate of this bug that now has a patch.