Last Comment Bug 340210 - bug creation timestamp is different than the first long description associated with that bug
: bug creation timestamp is different than the first long description associate...
Status: RESOLVED WORKSFORME
:
Product: Bugzilla
Classification: Server Software
Component: Creating/Changing Bugs (show other bugs)
: 2.18
: All All
-- minor (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
: default-qa
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-06-02 22:35 PDT by Leo Davis
Modified: 2006-06-07 09:53 PDT (History)
7 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch, v1: Don't create timestamps back in time when importing (1.21 KB, patch)
2006-06-03 08:44 PDT, Vlad Dascalu
LpSolit: review-
Details | Diff | Splinter Review
searches for/fixes descriptions that have this problem (1.15 KB, text/plain)
2006-06-05 12:41 PDT, Leo Davis
no flags Details

Description User image Leo Davis 2006-06-02 22:35:56 PDT
User-Agent:       Mozilla/5.0 (compatible; Konqueror/3.5; Linux) KHTML/3.5.2 (like Gecko)
Build Identifier: 

I noticed that one of the bugs in my database has a different creation timestamp (bugs.creation_ts) than the first long description (longdescs.bug_when) (by one second) associated with that bug.

I also noticed in post_bug.cgi that indeed there are two different timestamps used when these values are put into their respective tables: $timestamp (for bugs.creation_ts) and $sql_timestamp (for longdescs.bug_when, amongst others).  Perhaps they are not always equal.


Reproducible: Didn't try




I don't know exactly which version of Bugzilla created that bug, but I'm guessing it was 2.18 or 2.20.  I'm on 2.22 now.

I'm using MySQL 5.0, but the bug in question was probably created using MySQL 4.1.  I'm not sure what version of Perl/DBI etc., but probably 5.6 or 5.7.
Comment 1 User image Max Kanat-Alexander 2006-06-03 01:31:46 PDT
Yeah, that's actually intentional. Bugs are created without a creation timestamp at first, so that they don't show up in searches until they're appropriately locked to the correct security groups. The creation timestamp is added at the end of bug creation.

Thanks for the report, though!
Comment 2 User image Vlad Dascalu 2006-06-03 02:13:15 PDT
Regarding comment #1: we use the same timestamp value fetched from a single "SELECT NOW()" statement.
Comment 3 User image Vlad Dascalu 2006-06-03 02:21:57 PDT
Leo: I've looked at the code on trunk, 2.22, 2.20 and 2.18, although you might be using a not-up-to-date branch code as well.

Except 2.20, we're using a single $timestamp variable, and in 2.20, the $sql_timestamp thing is simply a quoted value of $timestamp. So I can't imagine how you would end up with 2 different values.

The only remaining option is the scenario where we have in the DB schema a timestamp column that auto-updates to NOW() automatically each time we do an INSERT or UPDATE on that table. I've looked at the current DB schema ( http://www.ravenbrook.com/project/p4dti/tool/cgi/bugzilla-schema/ ) and the two fields that you mentioned as having different values, and they look ok.

It might be possible to have had different DB schema or different code in the past, but now it looks ok. Please reopen if something new appears.
Comment 4 User image Olav Vitters 2006-06-03 02:26:40 PDT
This could still happen when you use importxml.pl.
Comment 5 User image Vlad Dascalu 2006-06-03 03:52:32 PDT
Reopening per Olav's comment -- we should fix importxml.pl (we should have the same timestamp in both places).
Comment 6 User image Max Kanat-Alexander 2006-06-03 04:07:28 PDT
(In reply to comment #0)
> I noticed that one of the bugs in my database has a different creation
> timestamp (bugs.creation_ts) than the first long description
> (longdescs.bug_when) (by one second) associated with that bug.
> 
> I also noticed in post_bug.cgi that indeed there are two different timestamps
> used when these values are put into their respective tables: $timestamp (for
> bugs.creation_ts) and $sql_timestamp (for longdescs.bug_when, amongst others). 
> Perhaps they are not always equal.
> 
> 
> Reproducible: Didn't try
> 
> 
> 
> 
> I don't know exactly which version of Bugzilla created that bug, but I'm
> guessing it was 2.18 or 2.20.  I'm on 2.22 now.
> 
> I'm using MySQL 5.0, but the bug in question was probably created using MySQL
> 4.1.  I'm not sure what version of Perl/DBI etc., but probably 5.6 or 5.7.
> 

Comment 7 User image Max Kanat-Alexander 2006-06-03 04:08:05 PDT
Oops. Didn't mean to quote the original description there. :-) I had actually meant to leave no comment at all. :-) My apologies.
Comment 8 User image Frédéric Buclin 2006-06-03 06:27:41 PDT
This bug has been fixed almost a year ago as part of bug 292544. The fix has been checked in in Bugzilla 2.18.2 and higher.
Comment 9 User image Vlad Dascalu 2006-06-03 06:40:48 PDT
LpSolit: I can reproduce the summary by using importxml.pl on the trunk. Reopening.
Comment 10 User image Vlad Dascalu 2006-06-03 06:41:27 PDT
Forgot to add LpSolit to CC (see previous comment)
Comment 11 User image Frédéric Buclin 2006-06-03 06:51:29 PDT
grr.... stop reopening this bug! importxml.pl works as expected:

    # Timestamps
    push( @query, "creation_ts" );
    push( @values,
        format_time( $bug_fields{'creation_ts'}, "%Y-%m-%d %X" )
          || $timestamp );

We use the original creation_ts date and time, if any. Else we use $timestamp.


    $dbh->do("INSERT INTO longdescs 
                     (bug_id, who, bug_when, work_time, isprivate, thetext) 
                     VALUES (?,?,?,?,?,?)", undef,
        $id, $exporterid, $timestamp, $worktime, $private, $long_description
    );

All original comments of the bug are concatenated into a single comment. In this case, their original date and time cannot be used and we use $timestamp again. So if the original bug has no creation_ts field specified, both use the same timestamp. Else not. That's so by design.

Do not reopen.
Comment 12 User image Vlad Dascalu 2006-06-03 08:06:39 PDT
Yes, I've looked at the code.

importxml.pl should always preserve DB consistency, and use the same timestamp for both the bugs table and the longdesc table.

If the original bug has the creation_ts field specified, then this should be used in both places. Currently it's used only in one place, and this leads to DB consistency failure. We need to fix that.

Reopening.
Comment 13 User image Vlad Dascalu 2006-06-03 08:12:08 PDT
(In reply to comment #12)
> If the original bug has the creation_ts field specified, then this should be
> used in both places.

Alternatively (since we're concatenating all the posts in one), we could *always* use in both places NOW() (or the timestamp of the last comment).
Comment 14 User image Frédéric Buclin 2006-06-03 08:21:40 PDT
CC'ing ghendricks who rewrote a large part of importxml.pl.

Another problem could be: when importing a bug X and using its original creation date Tx, we may have Tx < Ty where bug Y is an existing bug of the target DB, with Y < X. Isn't that inconsistent too? In this case, we should use $timestamp (NOW) in both cases, as suggested by vladd in comment 13.
Comment 15 User image Vlad Dascalu 2006-06-03 08:30:17 PDT
I've thought a little bit more about this, trying to aim for the big picture. I think we have here two possible models, and the controversy is the fact that the current importxml.pl implements a 3rd model (which is a combination of those two).

Model 1
-------

In this model we regard importxml.pl's abilities as similar to those of a normal Bugzilla user. In this case, importxml.pl should not be able to create bugs or comments in the Bugzilla DB that have timestamps older than the NOW() value. Granted, importxml should be able in this model to fetch via XML remote bugs and to import them in the current Bugzilla DB, with the posts concatenated, but all the timestamps should be set to NOW(), just like it would happen in case any other user that decides to manually import a remote bug manually. Importxml, in this model, simply automates the task of manually importing a bug, inside the abilities of the specified username exporter.

Model 2
-------

In this model, importxml.pl has the abilities of a "special" user, which is able to import bugs with timestamps in the past. In this case, bugs should really be moved together with associated comments and users. It's really tricky, and I don't see how we should be able to copy via XML the comments, without having previously replicated usernames from the remote DB.


We're trying Model 3, which tries to set a timestamp in the past, in the name of an exporter DB account. Normally, the exporter would not be able to perform such an operation, so importxml.pl offers an ability that can not be replicated via the traditional web-based interface.


I think the best solution is to use in both cases a one-time fetched NOW() value, and to specify in comment #0 the creation time of the initial bug, if it's available (follow Model 1 and comment #13 that is).
Comment 16 User image Vlad Dascalu 2006-06-03 08:40:44 PDT
Actually the importxml.pl script does:

   $long_description .= "$sorted_descs[$z]->{'bug_when'}";

so mentioning creation_ts when available would simply be redundant.
Comment 17 User image Vlad Dascalu 2006-06-03 08:44:12 PDT
Created attachment 224313 [details] [diff] [review]
Patch, v1: Don't create timestamps back in time when importing
Comment 18 User image Frédéric Buclin 2006-06-03 08:48:39 PDT
I don't like loosing all these timestamp information, for bugs as well as for attachments. That's something I like in ghendricks's rewrite: we loose as few information as possible.
Comment 19 User image Vlad Dascalu 2006-06-03 09:20:49 PDT
See comment #16, we don't loose bug creation because it's identical with the timestamp of the first longdesc, which is dumped in the concatenated log.

The only thing that we loose is the timestamp for the attachments. We can dump that in the concatenated log as well, but it seems overkill.
Comment 20 User image Frédéric Buclin 2006-06-03 09:47:08 PDT
What about queries based on dates? Adding timestamps into a comment is not enough IMO. I definitely don't understand what the problem is with the actual behavior. Who said it was inconsistent to have a different timestamp for the bug creation date and the description when importing a bug? My vote for a statu quo here.
Comment 21 User image Vlad Dascalu 2006-06-03 13:40:05 PDT
>> I definitely don't understand what the problem is with the actual
>> behavior.


1) It's a hack. There's no logic in having the first comment made at a different timestamp than the timestamp of bug creation.

2) Trust. You're basically altering "history" in the database by relying on a timestamp provided by a 3rd party server. You're giving the ability to introduce events and you-name-what, by taking as a good reference a timestamp provided in a 3rd party xml file, and integrating that timestamp in your DB historical flow.

3) Not a doable thing via the web interface. I'm reluctant in providing and maintaining an "import" tool that does things that you'd be unable to do via the web.

2 is most important, followed by 1 and 3 in my opinion.
Comment 22 User image Max Kanat-Alexander 2006-06-03 14:24:32 PDT
importxml.pl should duplicate exactly what came from the target DB.

There's no functional bug created by having the first comment's timestamp be different from the bug-creation timestamp, as long as the bug creation timestamp is the same as it was in the DB it came from.

Please don't go around fixing bugs that don't cause people problems.
Comment 23 User image Frédéric Buclin 2006-06-03 14:30:18 PDT
Comment on attachment 224313 [details] [diff] [review]
Patch, v1: Don't create timestamps back in time when importing

Importing bugs from another DB *is* something different than opening a new bug. An administrator who is moving bugs from one DB to another wants to keep as much original data as possible, explaining why we keep the original creation date when we have it. I'm with mkanat on this.
Comment 24 User image Leo Davis 2006-06-03 20:56:44 PDT
> Except 2.20, we're using a single $timestamp variable, and in 2.20, the
> $sql_timestamp thing is simply a quoted value of $timestamp. So I can't imagine
> how you would end up with 2 different values.

I can't imagine it either looking at the 2.22 code.  But it did happen.

I was just creating my own extension to the bug list and wondered why that one bug never showed up...

It's good that this bug has been fixed, so I'll be fixing that description timestamp (and looking for others to fix).
Comment 25 User image Vlad Dascalu 2006-06-04 00:35:37 PDT
> Please don't go around fixing bugs that don't cause people problems.

Mkanat, that's off-topic in regard to this specific bug. Please email me such statements, or contact justdave if you're not getting from me the reply that you expect. Thanks.
Comment 26 User image Leo Davis 2006-06-05 12:41:44 PDT
Created attachment 224466 [details]
searches for/fixes descriptions that have this problem 

This will search for/fix descriptions having this problem.  I've tried it on MySQL 5.0/Bugzilla 2.22.
Comment 27 User image Max Kanat-Alexander 2006-06-05 18:25:36 PDT
WFM per all above comments. The script could be useful to anybody who was bitten by the bug before 2.18.2.
Comment 28 User image Greg Hendricks 2006-06-07 09:53:00 PDT
I really don't have much of an opinion on this.
I do think that no matter what we should keep the original timestamp of the bug. This way I preserve the ability to search and sort on bug age from when it was orignially filed in the original system.

I can see the argument for making sure the first comment matches timestamps. But if it works as is, lets let it be.

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