User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.2) Gecko/20040820 Debian/1.7.2-4 Build Identifier: As discussed in bug 237862, the SQL DATE_FORMAT function is not DB agnostic, and the best solution to cope with it is to replace it with perl code. Reproducible: Always Steps to Reproduce:

Here is a patch replacing DATE_FORMAT with perl. It turns out that in most cases, the only use of the date/time is passing it to the html template, which in turn calls Buzilla::Util::format_time() function. So as long as format_time() understands the format the DB is returning, we should be ok.

For the record, I would much rather continue to use DATE_FORMAT, but make it a wrapper function in DBCompat.pm, and make the wrapper function always return the same format. Some databases (Sybase and Oracle in particular?) "dumb down" the datestamp a little too much trying to make it human readable if you don't specify a more specific format, and we lose the "seconds" portion of the timestamp that way. In fact, IIRC (although it was Oracle 8 where I experienced this, and it may work better in newer versions), selecting a datestamp field without specifying a TO_CHAR() around it will cause an SQL error because it can't implicitly cast it to a string to return the value.

I am more than happy with keeping the solution implemented by DBcompat.pm module, just that when I was going through all the comments there (mainly https://bugzilla.mozilla.org/show_bug.cgi?id=237862#c14) in my attempt to update it and re-submit, I got the impression THIS is the preffered way. And IMHO, this approach allows us to easily change DB date type to whatewer we decide to go with (if we decide to change it), string, number... and we need to change only Util::format_time() and any code dealing with dates directly (currently only whine.pl, if I recall correctly). If we go for DBcompat approach, then we are changing it now from DATE_FORMAT SQL to perl function call, and if we decide to change the type later, we will be changing all the queries again to remove it... Or would you prefer to change the type (to lets say integer) as well in this patch, in one shot?

Well, as there are no other comments, I am marking this as invalid and pulling the code back to DBcompat.pm.