189446 - Cannot change product of a bug

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Jussi Sirpoma]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3b) Gecko/20030116
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3b) Gecko/20030116

When trying to change the product of a bug. Bugzilla gives a short Bug processed
message but does not change the product. I could reproduce this in ladfill.

Reproducible: Always

Steps to Reproduce:
1. Go to http://landfill.bugzilla.org/bugzilla-tip/ 
2. Change the product of a bug
3. Commit

Actual Results:  
Landfill gives Bug processed message (Not in the box but just a text message
below the banner). the product has not been changed and nothing is inserted to
the bug activity log.

In my own site I get following diagnostics:
Software error:

DBD::mysql::st fetchrow_array failed: fetch() without execute() at
Bugzilla/DB.pm line 100
	Bugzilla::DB::FetchSQLData() called at globals.pl line 497
	main::AnyDefaultGroups() called at process_bug.cgi line 297

This could be a regression from bug 163290.

Comment 1

[Jussi Sirpoma]

Attachment: Trace from a failed attempt

Trace for bbaetz.

Comment 2

[Bradley Baetz (:bbaetz)]

Oh, bleh. the code in queries gets the result of MoreSQLData, and then calls
FetchSQLData anyway.

This is needed to flush the fetchahead buffer. However, if MoreSQLData returns
false, then there is no fetchahead buffer, so FetchSQLData calls ->fetch, which
fails because the statement is already completed.

The old code never tested for errors, so we never noticed. Bleh.

I still can't repro this locally, probably because my query does return a result
for this case.

This idiom is all over because of the silly fetchahead stuff. In this case, we
don't need to do this because of the Pop which destroys the fetchahead buffer
anyway, but there are other plces we do, and auditing them will be painful.

I think just ignoring errors from that line would be the best way to go. We've
been doing it for ages, and theres not much we could do in the case of an error
such as disk failure anyway, esp without transactions. Thoughts?

Comment 3

[Jussi Sirpoma]

When I tested this in landfill, I used bug 632 and tried to change the product
to MyOwnBadSelf.

bbaetz:
Changing the last line of pre-DB.pm version of FetchSQLData to
return $::currentquery->fetchrow_array || die "Error: " . $DBI::errstr;

Resulted in premature end of script headers in every page I tried. Propably
because the cgi's died before producing enough output.

Comment 4

[Bradley Baetz (:bbaetz)]

Yeah, same bug.

Comment 5

[Bradley Baetz (:bbaetz)]

Attachment: evil hack

Its evil, its ugly, but it works..

Comment 6

[Dave Miller [:justdave]]

Does this record a warning?  We should make it do a non-fatal warning so we get
them in log files and people start fixing them.  (does a die become a warn
inside an eval or does it just not output anything?)

Comment 7

[Dave Miller [:justdave]]

see also bug 190197, patch included, which fixes one of these occurances from
the caller instead of hiding the error...

Comment 8

[Dave Miller [:justdave]]

Comment on attachment 112273 [details] [diff] [review]
evil hack

we can worry about warnings later, this makes it all work again at least.

Comment 9

[Bradley Baetz (:bbaetz)]

Yeah, but this isn't the only case - edit* does this too, and I am _not_
cleaning that up.

Theres no warning, and I'm not sure that we want one, really, since this
idiam(sp?) is scattered arround. New code using the DBI directly won't be
protected by this.

Comment 10

[Bradley Baetz (:bbaetz)]

Fixed.