Open Bug 283076 Opened 17 years ago Updated 8 years ago

Dates and times must be cross-database compatible and consistent in all of Bugzilla

Categories

(Bugzilla :: Database, enhancement, P4)

enhancement

Tracking

()

People

(Reporter: Tomas.Kopal, Unassigned)

References

(Blocks 1 open bug, )

Details

Dates/times in bugzilla are coming from different sources (perl, DB), and can
have different format, especially when we support different databases. E.g.
Postgres is returning times including fractions of seconds, MySQL does not. This
causes problems when doing date/time calculations and comparisons (see comments
at bug 280500).
We need to investigate and fix this, e.g. by introducing some formatting,
changing the code to do all date/time operations on one place (preferably in
SQL) or with some other solution.
(In reply to bug 
>>> One solution comes to my mind and that is swithing from native SQL dates to
>>> something general. Someone suggested previously on some other Pg bug to 
>>> switch to strings (problems with sorting) or to integers. But that's 
>>> definitely a much bigger change...
>> 
>>   Yeah, we decided against it because we would lose indexes on date 
>> comparisons in the SQL, which could kill performance.
>
> I am afraid I don't uderstand. If you switch to integers (e.g. seconds since
> epoch, unix format), you should be able to do comparisons as you wish, you
> should be able to index etc.

  That makes it slightly more difficult to do timezones eventually, but that's
not a huge issue... There might be some problem with switching to seconds
globally, but I can't think of it right now.

  Can anybody think of why switching to seconds would cause any problems?
Summary: Date/time format is not enforced → Dates and times must be cross-database compatible and consistent in all of Bugzilla
(In reply to comment #1)
>   Can anybody think of why switching to seconds would cause any problems?

The only problem I can think of is that you'll need an 8-byte integer to store seconds from the Unix 
epoch for dates greater than ~2038 (or at least something bigger than a 4-byte integer anyway). Do 
MySQL and PostgreSQL (and Oracle and Sybase) currently support an 8-byte integer type?

Integer seconds are a good choice, I think. In another bug somewhere I suggested using Modified Julian 
Dates stored in floats. FWIW, that's what astronomers use (see http://tycho.usno.navy.mil/mjd.html), 
but integer seconds is probably a better choice for Bugzilla.

I presume the idea is to "inflate" the database values to a DateTime Perl object (http://search.cpan.org/
~drolsky/DateTime-0.27/lib/DateTime.pm) that can then be formatted or compared very easily.
(In reply to comment #2)
> The only problem I can think of is that you'll need an 8-byte integer to store
seconds from the Unix 
> epoch for dates greater than ~2038 (or at least something bigger than a 4-byte
integer anyway). Do 
> MySQL and PostgreSQL (and Oracle and Sybase) currently support an 8-byte
integer type?

Both MySQL and Postgres has BIGINT type, which is 8 bytes. Will that get through
perl unafected? I have read the warnings at the epoch() method of the
DateTime.pm module and I am not sure how it works internally.

> Integer seconds are a good choice, I think. In another bug somewhere I
suggested using Modified Julian 
> Dates stored in floats. FWIW, that's what astronomers use (see
http://tycho.usno.navy.mil/mjd.html), 
> but integer seconds is probably a better choice for Bugzilla.

Yes, your post was my inspiration :-). I am still not sure whether Julian/float
would not be better, but I generally do not like floating point for anything
"exact" :-). We could hit the same problems with comparisons we are just trying
to solve.

> I presume the idea is to "inflate" the database values to a DateTime Perl
object (http://search.cpan.org/~drolsky/DateTime-0.27/lib/DateTime.pm) that can
then be formatted or compared very easily.

So far, it was more a rhetorical question than idea, but as I am looking at the
module you mentioned I am starting to like it :-).
(In reply to comment #2)
> The only problem I can think of is that you'll need an 8-byte integer to store
seconds from the Unix 
> epoch for dates greater than ~2038 (or at least something bigger than a 4-byte
integer anyway).

Also, we can maybe "hack in" some offset to start say at year 1990, to get
another twenty years :-).
Don't worry about using 64-bit integers at this point -- I'm not sure that perl
can handle them on a 32-bit system.

And yeah, we're definitely not using the MJD. :-) Seconds should be perfect for us.

  Is there any way we can avoid adding another module dependency? I thought we
already had something in our modules or in base perl that could do what we need
-- like Date::Format or Date::Parse.
(In reply to comment #5)
> Don't worry about using 64-bit integers at this point -- I'm not sure that perl
> can handle them on a 32-bit system.

OK, let's leave it 32bit for now, we have over 30years to fix it :-).

>   Is there any way we can avoid adding another module dependency? I thought we
> already had something in our modules or in base perl that could do what we need
> -- like Date::Format or Date::Parse.

These modules do only conversions between string and numeric representation. The
DateTime.pm can do date math (replacing e.g. DiffDate() in buglist.cgi), can
format date for output (replacing Date::Format) etc. It does not seem to be able
to parse dates (thus Date::Parse will remain).

It all depends on how much functionality we need, maybe having just
parsing/formatting will be enough?
(In reply to comment #6)
> (In reply to comment #5)
> > Don't worry about using 64-bit integers at this point -- I'm not sure that perl
> > can handle them on a 32-bit system.
> 
> OK, let's leave it 32bit for now, we have over 30years to fix it :-).

Isn't that what the COBOL programmers said back in the 1970s? ;-) Anyway, I'm sure 32 bits will be 
fine.

> >   Is there any way we can avoid adding another module dependency? I thought we
> > already had something in our modules or in base perl that could do what we need
> > -- like Date::Format or Date::Parse.
> 
> These modules do only conversions between string and numeric representation. The
> DateTime.pm can do date math (replacing e.g. DiffDate() in buglist.cgi), can
> format date for output (replacing Date::Format) etc. It does not seem to be able
> to parse dates (thus Date::Parse will remain).
> 
> It all depends on how much functionality we need, maybe having just
> parsing/formatting will be enough?

I think it would be foolish to use anything other than DateTime. There's no point in reinventing the 
wheel and it will be the de facto standard for Perl dates and times, if it isn't already. For parsing needs, 
there's DateTime::Format::Builder (http://search.cpan.org/dist/DateTime-Format-Builder/).
Two more things:

1. Check out DateTime::Format::DBI (http://search.cpan.org/dist/DateTime-Format-DBI/).
2. Read the extensive DateTime FAQ: http://datetime.perl.org/faq.html
(In reply to comment #7)
> I think it would be foolish to use anything other than DateTime. There's no
point in reinventing the 
> wheel and it will be the de facto standard for Perl dates and times, if it
isn't already.

Hmmm, it really looks very good and I can easily imagine converting bugzilla to
use this class for all date/time manipulations. It would probably clean up a lot
of code. But it's a HUGE change...

(In reply to comment #8)
> 1. Check out DateTime::Format::DBI
(http://search.cpan.org/dist/DateTime-Format-DBI/).

With this, is it still practical to think about switching to integers? Can we
hit some problems if we leave DB date/time types with the precision the DB is
offering (different for every DB)? I can't think of anything, and using native
type for what it was created is usually the best solution (and we would avoid
the mistake COBOL programmers did ;-) ).
Disadvantage is that we are introducing another module dependency (but isn't
code reuse exactly about this?).
(In reply to comment #9)
> > 1. Check out DateTime::Format::DBI
> (http://search.cpan.org/dist/DateTime-Format-DBI/).
> 
> With this, is it still practical to think about switching to integers? Can we
> hit some problems if we leave DB date/time types with the precision the DB is
> offering (different for every DB)? I can't think of anything, and using native
> type for what it was created is usually the best solution (and we would avoid
> the mistake COBOL programmers did ;-) ).
> Disadvantage is that we are introducing another module dependency (but isn't
> code reuse exactly about this?).

That's a very good point. At the very least, DateTime::Format::DBI would allow you
to do this in two separate steps/bugs: First, convert Bugzilla to use DateTime and
then change the Bugzilla database schema to store dates in integer fields.

It's quite possible that integer seconds may be desired for some database platforms,
but not others. I would suggest adding a get_datetime_parser() method to Bugzilla::DB.
The default implementation would invoke DateTime::Format::DBI, at least initially,
but DB-specific subclasses could override it to invoke a different parser based on
how dates/times are stored in the DB-specific schema.
When all dates are integers, we won't need to do date math. We just need to be
able to convert a date to an integer, and convert an integer to a date. I can't
believe that Date::Format and Date::Parse don't accept epoch seconds... is that
really true?
perldoc -f time says:

"Returns the number of non-leap seconds since whatever time the system considers
to be the epoch (that's 00:00:00, January 1, 1904 for Mac OS, and 00:00:00 UTC,
January 1, 1970 for most other systems). Suitable for feeding to gmtime and
localtime."

Does that mean Mac OS Classic (which we don't support), or also Mac OS X? I'd
imagine it only means Classic, but if it means X we might have a problem.
(In reply to comment #12)
> Does that mean Mac OS Classic (which we don't support), or also Mac OS X? I'd
> imagine it only means Classic, but if it means X we might have a problem.

It means Mac OS Classic. time() works as expected on Mac OS X. Verified by
direct experimentation.
http://www.postgresql.org/docs/7.3/interactive/datatype-datetime.html

"time, timestamp, and interval accept an optional precision value p which
specifies the number of fractional digits retained in the seconds field. By
default, there is no explicit bound on precision. The allowed range of p is from
0 to 6 for the timestamp and interval types."

Set it to 0 in the Schema. Solved, for now.

Also:

<justdave> the solution we did with the Sybase port was ANY time a time is being
returned from the database it gets wrapped in a formatting function on the
database side
<justdave> if it's being used as part of a comparison within the SQL or copying
from one column to another, you leave it alone
<justdave> if it's being returned to the app you wrap it in DATE_FORMAT() or
TO_CHAR() or whatever.
<justdave> and make sure every driver always returns the same format
(In reply to comment #14)
> <justdave> if it's being returned to the app you wrap it in DATE_FORMAT() or
> TO_CHAR() or whatever.
> <justdave> and make sure every driver always returns the same format

That'll work, but it's not nearly as "cool" as an object-oriented approach. It
puts the burden on the SQL engine to do formatting, which I feel is less than
optimal and can be somewhat clumsy.
(In reply to comment #15)
> (In reply to comment #14)
> > <justdave> if it's being returned to the app you wrap it in DATE_FORMAT() or
> > TO_CHAR() or whatever.
> > <justdave> and make sure every driver always returns the same format
> 
> That'll work, but it's not nearly as "cool" as an object-oriented approach. It
> puts the burden on the SQL engine to do formatting, which I feel is less than
> optimal and can be somewhat clumsy.

  Yeah. But for now we don't have to worry about this, because we can just use
the schema fix that I posted in comment 14 for PostgreSQL support.
(In reply to comment #14)
> http://www.postgresql.org/docs/7.3/interactive/datatype-datetime.html
> 
> "time, timestamp, and interval accept an optional precision value p which
> specifies the number of fractional digits retained in the seconds field. By
> default, there is no explicit bound on precision. The allowed range of p is from
> 0 to 6 for the timestamp and interval types."
> 
> Set it to 0 in the Schema. Solved, for now.

Coool, I must have missed this one, bugger. OK, so the sum it up, we'll leave
this bug open for now (it will still be nice to clean up the date handling by
using a class), and I will remove the regex fix from the patch at bug 280500.
Agreed?

> 
> Also:
> 
> <justdave> the solution we did with the Sybase port was ANY time a time is being
> returned from the database it gets wrapped in a formatting function on the
> database side
> <justdave> if it's being used as part of a comparison within the SQL or copying
> from one column to another, you leave it alone
> <justdave> if it's being returned to the app you wrap it in DATE_FORMAT() or
> TO_CHAR() or whatever.
> <justdave> and make sure every driver always returns the same format

That's basically what I did by the regex patch, just instead of SQL, its done in
perl. Although I agree I haven't verified (yet) that all DB->perl is done using
this function (it should though).
Severity: normal → enhancement
We handled this for PostgreSQL, so I'm removing the blocking.
No longer blocks: bz-postgres
Blocks: 182238
In Bugzilla 2.20.1 the change mentioned in comment 14 has been made. This breaks bugzilla for postgres users, specifically it causes bugzilla to randomly refuse to send emails on a ticket change.

See bug 330580 for more information.

Ciao!
Assignee: general → database
Component: Bugzilla-General → Database
xiaoou says this bug doesn't block Oracle support. So removing the dependency.
No longer blocks: bz-oracle
Priority: -- → P4
Duplicate of this bug: 759763
(In reply to Frédéric Buclin from comment #22)
> Duplicate of this bug: 759763
You took my new-ish small bug report that had a chance of getting fixed, and lumped it in with this 7+ years old meta bug that hasn't been touched and doesn't look likely to ever get addressed.  My hopes of getting a patch submitted and approved just went out the window. ;-)  Seriously though, this doesn't seem like the best place to address the "system localtime" vs "database now()" change.


My report was to avoid use of "localtime" in the 8 places where it is used.  Here is how to get the current epoch from the various databases supported.

ORACLE: SELECT (SYSDATE - to_date('01-JAN-1970','DD-MON-YYYY')) * (86400) as dt FROM dual;  **untested, I'm not familiar with Oracle at all, and don't have it available to test on**

POSTGRE: SELECT FLOOR(EXTRACT(EPOCH FROM LOCALTIMESTAMP);

MYSQL: SELECT UNIX_TIMESTAMP();

Obviously the best place to put these types of things is in Bugzilla/DB/*.pm, and allow an optional timestamp to be provided that could be converted to the epoch.
You need to log in before you can comment on or make changes to this bug.