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)

2.21
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 2.22

People

(Reporter: lance.larsh, Assigned: lance.larsh)

References

Details

Attachments

(1 file, 1 obsolete file)

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
Depends on: bz-oracle
Version: unspecified → 2.21
Blocks: bz-oracle
No longer depends on: bz-oracle
Assignee: database → lance.larsh
Status: UNCONFIRMED → NEW
Ever confirmed: true
(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
Attached patch Patch to add CASE WHEN clauses (obsolete) (deleted) — Splinter Review
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)
Does this have any significant performance impact on the other databses? Should it be a $dbh->boolean() operation?
(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.
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 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+
Attached patch Patch v2 (deleted) — Splinter Review
This patch reformats long lines, otherwise it's the same as Attachment 200298 [details] [diff]
Attachment #200298 - Attachment is obsolete: true
Flags: approval?
Flags: approval? → approval+
Target Milestone: --- → Bugzilla 2.22
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.

Attachment

General

Creator:
Created:
Updated:
Size: