Closed
Bug 313209
Opened 19 years ago
Closed 19 years ago
Oracle requires "CASE WHEN" around boolean expressions in the SELECT column list
Categories
(Bugzilla :: Database, enhancement)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.22
People
(Reporter: lance.larsh, Assigned: lance.larsh)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.3) Gecko/20041119
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.3) Gecko/20041119
Oracle doesn't allow boolean expressions directly in the column list of a SELECT
statement. Queries will fail on Oracle if written as:
SELECT col IS NULL FROM t;
To make this query work on Oracle, it must be written as:
SELECT CASE WHEN col IS NULL THEN 1 ELSE 0 END FROM t;
Most queries already have the CASE WHEN clause, but there are a handful that don't.
Reproducible: Always
Steps to Reproduce:
Affected queries:
Bugzilla/Bug.pm:549
Bugzilla/User.pm:492
Bugzilla/User.pm:510
chart.cgi:210
editproducts.cgi:197
editproducts.cgi:834
editproducts.cgi:868
globals.pl:758
process_bug.cgi:1668
Assignee | ||
Updated•19 years ago
|
Updated•19 years ago
|
Assignee: database → lance.larsh
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 1•19 years ago
|
||
(In reply to comment #0)
Hmm, the code has changed quite a bit since my last pull a few days ago. One of
the offending queries was removed altogether, and the others changed line #s
(and hopefully no new ones have been introduced). The list of queries is now:
Bugzilla/Bug.pm:549
Bugzilla/User.pm:512
Bugzilla/User.pm:530
chart.cgi:210
editproducts.cgi:628
editproducts.cgi:660
globals.pl:737
process_bug.cgi:1723
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•19 years ago
|
||
One of the diffs in this patch (i.e., the diff in process_bug.cgi) may fail to
apply depending on whether the patch for Bug 312349 (Attachment 199595 [details] [diff]) has
been checked in or not, since both patches touch that file about 2-3 lines away
from each other. This patch is based on today's CVS source, which does not yet
have 312349 applied.
This patch addresses all the occurrences I found. I manually checked all 563
(yikes!) SELECT statements that existed in the Bugzilla code as of a few days
ago, and these were the only ones that were affected at that time.
Attachment #200298 -
Flags: review?(mkanat)
Comment 3•19 years ago
|
||
Does this have any significant performance impact on the other databses? Should
it be a $dbh->boolean() operation?
Assignee | ||
Comment 4•19 years ago
|
||
(In reply to comment #3)
> Does this have any significant performance impact on the other databses?
Hmm, I really don't know. But it does appear that using "CASE WHEN ... THEN 1
ELSE 0 END" is already done elsewhere in Bugzilla. In fact, some of the
queries my patch touches already have a boolean CASE WHEN in other fields of
the SELECT list.
> Should it be a $dbh->boolean() operation?
That's not my call, but I see 19 places where "THEN 1 ELSE 0 END" already
exists. Is there a reason those 19 places needed the CASE WHEN for the other
DBs while the other 8 spots I found didn't? I'm guessing no and that it was
just an oversight, but I'm no expert on the other DBs.
Comment 5•19 years ago
|
||
I think that, as long as this is only in the fields returned and not in any
criteria, we should be OK. That seems to be the case in all of the cases you
have in the patch. If any of these become "AS alias" fields that feed into a
WHERE condition or get used in a subselect, it could be more of a question.
Comment 6•19 years ago
|
||
Comment on attachment 200298 [details] [diff] [review]
Patch to add CASE WHEN clauses
This looks fine to me.
HOWEVER: There are several lines that are longer than 80 characters, which
violates our style guide. Those need to be fixed before you check this in.
Request approval once you have that fixed.
Attachment #200298 -
Flags: review?(mkanat) → review+
Assignee | ||
Comment 7•19 years ago
|
||
This patch reformats long lines, otherwise it's the same as Attachment 200298 [details] [diff]
Attachment #200298 -
Attachment is obsolete: true
Assignee | ||
Updated•19 years ago
|
Flags: approval?
Updated•19 years ago
|
Flags: approval? → approval+
Target Milestone: --- → Bugzilla 2.22
Comment 8•19 years ago
|
||
Checking in chart.cgi;
/cvsroot/mozilla/webtools/bugzilla/chart.cgi,v <-- chart.cgi
new revision: 1.15; previous revision: 1.14
done
Checking in editproducts.cgi;
/cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v <-- editproducts.cgi
new revision: 1.103; previous revision: 1.102
done
Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v <-- globals.pl
new revision: 1.344; previous revision: 1.343
done
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v <-- process_bug.cgi
new revision: 1.291; previous revision: 1.290
done
Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v <-- Bug.pm
new revision: 1.99; previous revision: 1.98
done
Checking in Bugzilla/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v <-- User.pm
new revision: 1.90; previous revision: 1.89
done
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•